I messed up and missed that we were carrying a diff on x86_64-mont5.pl. This
was accidentally dropped in https://boringssl-review.googlesource.com/6616.
To confirm the merge is good now, check out at this revision and run:
git diff e701f16bd69b6f251ed537e40364c281e85a63b2^ crypto/bn/asm/x86_64-mont5.pl > /tmp/A
Then in OpenSSL's repository:
git diff d73cc256c8e256c32ed959456101b73ba9842f72^ d73cc256c8e256c32ed959456101b73ba9842f72 crypto/bn/asm/x86_64-mont5.pl > /tmp/B
And confirm the diffs vary in only metadata:
diff -u /tmp/A /tmp/B
--- /tmp/A 2015-12-03 11:53:23.127034998 -0500
+++ /tmp/B 2015-12-03 11:53:53.099314287 -0500
@@ -1,8 +1,8 @@
diff --git a/crypto/bn/asm/x86_64-mont5.pl b/crypto/bn/asm/x86_64-mont5.pl
-index 38def07..3c5a8fc 100644
+index 388e3c6..64e668f 100755
--- a/crypto/bn/asm/x86_64-mont5.pl
+++ b/crypto/bn/asm/x86_64-mont5.pl
-@@ -1770,6 +1770,15 @@ sqr8x_reduction:
+@@ -1784,6 +1784,15 @@ sqr8x_reduction:
.align 32
.L8x_tail_done:
add (%rdx),%r8 # can this overflow?
@@ -18,7 +18,7 @@
xor %rax,%rax
neg $carry
-@@ -3116,6 +3125,15 @@ sqrx8x_reduction:
+@@ -3130,6 +3139,15 @@ sqrx8x_reduction:
.align 32
.Lsqrx8x_tail_done:
add 24+8(%rsp),%r8 # can this overflow?
@@ -34,7 +34,7 @@
mov $carry,%rax # xor %rax,%rax
sub 16+8(%rsp),$carry # mov 16(%rsp),%cf
-@@ -3159,13 +3177,11 @@ my ($rptr,$nptr)=("%rdx","%rbp");
+@@ -3173,13 +3191,11 @@ my ($rptr,$nptr)=("%rdx","%rbp");
my @ri=map("%r$_",(10..13));
my @ni=map("%r$_",(14..15));
$code.=<<___;
Change-Id: I3fb5253783ed82e4831f5bffde75273bd9609c23
Reviewed-on: https://boringssl-review.googlesource.com/6618
Reviewed-by: Adam Langley <agl@google.com>
BN_LLONG is only ever used in #ifdefs. The actual type is BN_ULLONG. Switch the
ifdefs to check on BN_ULLONG and remove BN_LLONG. Also fix signedness of all
the constants (potentially avoiding undefined behavior in some operations).
Change-Id: I3e7739bbe14c50ea7db04fc507a034a8cb315a5f
Reviewed-on: https://boringssl-review.googlesource.com/6518
Reviewed-by: Adam Langley <agl@google.com>
The |ri| field was only used in |BN_MONT_CTX_set|, so make it a local
variable of that function.
Change-Id: Id8c3d44ac2e30e3961311a7b1a6731fe2c33a0eb
Reviewed-on: https://boringssl-review.googlesource.com/6526
Reviewed-by: Adam Langley <agl@google.com>
The comment in |BN_mod_inverse_ex| makes it clear that |BN_BITS2| was
intended. Besides fixing the code to match the comment, remove
the now-unused |BN_BITS| and the already-unused |BN_MASK| to prevent
future confusion of this sort.
On MSVC builds there seems to be very little difference in performance
between the two code paths according to |bssl speed|.
Change-Id: I765b7b3d464e2057b1d7952af25b6deb2724976a
Reviewed-on: https://boringssl-review.googlesource.com/6525
Reviewed-by: Adam Langley <agl@google.com>
Previously, both crypto/dh and crypto/ec defined |TOBN| macros that did
the same thing, but which took their arguments in the opposite order.
This change makes the code consistently use the same macro. It also
makes |STATIC_BIGNUM| available for internal use outside of crypto/bn.
Change-Id: Ide57f6a5b74ea95b3585724c7e1a630c82a864d9
Reviewed-on: https://boringssl-review.googlesource.com/6528
Reviewed-by: Adam Langley <agl@google.com>
Relevant code was removed in 5d5e39f5d2.
Change-Id: I198844064030c04f88e5541f2bbaa29ae13d14bb
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/6521
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Chromium's toolchains may now assume C++11 library support, so we may freely
use C++11 features. (Chromium's still in the process of deciding what to allow,
but we use Google's style guide directly, toolchain limitations aside.)
Change-Id: I1c7feb92b7f5f51d9091a4c686649fb574ac138d
Reviewed-on: https://boringssl-review.googlesource.com/6465
Reviewed-by: Adam Langley <agl@google.com>
The file armv8-mont.pl is taken from upstream. The speed ups are fairly
modest (~30%) but seem worthwhile.
Before:
Did 231 RSA 2048 signing operations in 1008671us (229.0 ops/sec)
Did 11208 RSA 2048 verify operations in 1036997us (10808.1 ops/sec)
Did 342 RSA 2048 (3 prime, e=3) signing operations in 1021545us (334.8 ops/sec)
Did 32000 RSA 2048 (3 prime, e=3) verify operations in 1016162us (31491.0 ops/sec)
Did 45 RSA 4096 signing operations in 1039805us (43.3 ops/sec)
Did 3608 RSA 4096 verify operations in 1060283us (3402.9 ops/sec)
After:
Did 300 RSA 2048 signing operations in 1009772us (297.1 ops/sec)
Did 12740 RSA 2048 verify operations in 1075413us (11846.6 ops/sec)
Did 408 RSA 2048 (3 prime, e=3) signing operations in 1016139us (401.5 ops/sec)
Did 33000 RSA 2048 (3 prime, e=3) verify operations in 1017510us (32432.1 ops/sec)
Did 52 RSA 4096 signing operations in 1067678us (48.7 ops/sec)
Did 3408 RSA 4096 verify operations in 1062863us (3206.4 ops/sec)
Change-Id: Ife74fac784067fce3668b5c87f51d481732ff855
Reviewed-on: https://boringssl-review.googlesource.com/6444
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
When building in OPENSSL_NO_ASM mode, MSVC complains about unreachable
code. The redundant initialization of |i| is the main problem. The
skipping of the first test of the condition |i < num| with |goto| was
also confusing.
It turns out that |bn_mul_mont| is only called when assembly language
optimizations are available, but in that case the assmebly language
versions will always be used instead. Although this code will be
compiled in |OPENSSL_NO_ASM| builds, it is never called in
|OPENSSL_NO_ASM| builds. Thus, it can just be removed.
Change-Id: Id551899b2602824978edc1a1cb0703b76516808d
Reviewed-on: https://boringssl-review.googlesource.com/5550
Reviewed-by: Adam Langley <agl@google.com>
Most functions can take this in as const. Note this changes an
RSA_METHOD hook, though one I would not expect anyone to override.
Change-Id: Ib70ae65e5876b01169bdc594e465e3e3c4319a8b
Reviewed-on: https://boringssl-review.googlesource.com/6419
Reviewed-by: Adam Langley <agl@google.com>
BN_mod_exp_mont_consttime does not modify its |BN_MONT_CTX| so that
value should be const.
Change-Id: Ie74e48eec8061899fd056fbd99dcca2a86b02cad
Reviewed-on: https://boringssl-review.googlesource.com/6403
Reviewed-by: Adam Langley <agl@google.com>
Since we pre-generate our perlasm, having the output of these files be
sensitive to the environment the run in is unhelpful. It would be bad to
suddenly change what features we do or don't compile in whenever workstations'
toolchains change or if developers do or don't have CC variables set.
Previously, all compiler-version-gated features were turned on in
https://boringssl-review.googlesource.com/6260, but this broke the build. I
also wasn't thorough enough in gathering performance numbers. So, flip them all
to off instead. I'll enable them one-by-one as they're tested.
This should result in no change to generated assembly.
Change-Id: Ib4259b3f97adc4939cb0557c5580e8def120d5bc
Reviewed-on: https://boringssl-review.googlesource.com/6383
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit b9c26014de.
The win64 bot seems unhappy. Will sniff at it tomorrow. In
the meantime, get the tree green again.
Change-Id: I058ddb3ec549beee7eabb2f3f72feb0a4a5143b2
Reviewed-on: https://boringssl-review.googlesource.com/6353
Reviewed-by: Adam Langley <alangley@gmail.com>
It's very annoying having to remember the right incant every time I want
to switch around between my build, build-release, build-asan, etc.,
output directories.
Unfortunately, this target is pretty unfriendly without CMake 3.2+ (and
Ninja 1.5+). This combination gives a USES_TERMINAL flag to
add_custom_target which uses Ninja's "console" pool, otherwise the
output buffering gets in the way. Ubuntu LTS is still on an older CMake,
so do a version check in the meantime.
CMake also has its own test mechanism (CTest), but this doesn't use it.
It seems to prefer knowing what all the tests are and then tries to do
its own output management and parallelizing and such. We already have
our own runners. all_tests.go could actually be converted tidily, but
generate_build_files.py also needs to read it, and runner.go has very
specific needs.
Naming the target ninja -C build test would be nice, but CTest squats
that name and CMake grumps when you use a reserved name, so I've gone
with run_tests.
Change-Id: Ibd20ebd50febe1b4e91bb19921f3bbbd9fbcf66c
Reviewed-on: https://boringssl-review.googlesource.com/6270
Reviewed-by: Adam Langley <alangley@gmail.com>
Since we pre-generate our perlasm, having the output of these files be
sensitive to the environment the run in is unhelpful. It would be bad to
suddenly change what features we do or don't compile in whenever workstations'
toolchains change.
Enable all compiler-version-gated features as they should all be runtime-gated
anyway. This should align with what upstream's files would have produced on
modern toolschains. We should assume our assemblers can take whatever we'd like
to throw at them. (If it turns out some can't, we'd rather find out and
probably switch the problematic instructions to explicit byte sequences.)
This actually results in a fairly significant change to the assembly we
generate. I'm guessing upstream's buildsystem sets the CC environment variable,
while ours doesn't and so the version checks were all coming out conservative.
diffstat of generated files:
linux-x86/crypto/sha/sha1-586.S | 1176 ++++++++++++
linux-x86/crypto/sha/sha256-586.S | 2248 ++++++++++++++++++++++++
linux-x86_64/crypto/bn/rsaz-avx2.S | 1644 +++++++++++++++++
linux-x86_64/crypto/bn/rsaz-x86_64.S | 638 ++++++
linux-x86_64/crypto/bn/x86_64-mont.S | 332 +++
linux-x86_64/crypto/bn/x86_64-mont5.S | 1130 ++++++++++++
linux-x86_64/crypto/modes/aesni-gcm-x86_64.S | 754 ++++++++
linux-x86_64/crypto/modes/ghash-x86_64.S | 475 +++++
linux-x86_64/crypto/sha/sha1-x86_64.S | 1121 ++++++++++++
linux-x86_64/crypto/sha/sha256-x86_64.S | 1062 +++++++++++
linux-x86_64/crypto/sha/sha512-x86_64.S | 2241 ++++++++++++++++++++++++
mac-x86/crypto/sha/sha1-586.S | 1174 ++++++++++++
mac-x86/crypto/sha/sha256-586.S | 2248 ++++++++++++++++++++++++
mac-x86_64/crypto/bn/rsaz-avx2.S | 1637 +++++++++++++++++
mac-x86_64/crypto/bn/rsaz-x86_64.S | 638 ++++++
mac-x86_64/crypto/bn/x86_64-mont.S | 331 +++
mac-x86_64/crypto/bn/x86_64-mont5.S | 1130 ++++++++++++
mac-x86_64/crypto/modes/aesni-gcm-x86_64.S | 750 ++++++++
mac-x86_64/crypto/modes/ghash-x86_64.S | 475 +++++
mac-x86_64/crypto/sha/sha1-x86_64.S | 1121 ++++++++++++
mac-x86_64/crypto/sha/sha256-x86_64.S | 1062 +++++++++++
mac-x86_64/crypto/sha/sha512-x86_64.S | 2241 ++++++++++++++++++++++++
win-x86/crypto/sha/sha1-586.asm | 1173 ++++++++++++
win-x86/crypto/sha/sha256-586.asm | 2248 ++++++++++++++++++++++++
win-x86_64/crypto/bn/rsaz-avx2.asm | 1858 +++++++++++++++++++-
win-x86_64/crypto/bn/rsaz-x86_64.asm | 638 ++++++
win-x86_64/crypto/bn/x86_64-mont.asm | 352 +++
win-x86_64/crypto/bn/x86_64-mont5.asm | 1184 ++++++++++++
win-x86_64/crypto/modes/aesni-gcm-x86_64.asm | 933 ++++++++++
win-x86_64/crypto/modes/ghash-x86_64.asm | 515 +++++
win-x86_64/crypto/sha/sha1-x86_64.asm | 1152 ++++++++++++
win-x86_64/crypto/sha/sha256-x86_64.asm | 1088 +++++++++++
win-x86_64/crypto/sha/sha512-x86_64.asm | 2499 ++++++
SHA* gets faster. RSA and AES-GCM seem to be more of a wash and even slower
sometimes! This is a little concerning. Though when I repeated the latter two,
it's definitely noisy (RSA in particular), so we may wish to repeat in a more
controlled environment. We could also flip some of these toggles to something
other than the highest setting if it seems some of the variants aren't
desirable. We just shouldn't have them enabled or disabled on accident. This
aligns us closer to upstream though.
$ /tmp/bssl.old speed SHA-
Did 5028000 SHA-1 (16 bytes) operations in 1000048us (5027758.7 ops/sec): 80.4 MB/s
Did 1708000 SHA-1 (256 bytes) operations in 1000257us (1707561.2 ops/sec): 437.1 MB/s
Did 73000 SHA-1 (8192 bytes) operations in 1008406us (72391.5 ops/sec): 593.0 MB/s
Did 3041000 SHA-256 (16 bytes) operations in 1000311us (3040054.5 ops/sec): 48.6 MB/s
Did 779000 SHA-256 (256 bytes) operations in 1000820us (778361.7 ops/sec): 199.3 MB/s
Did 26000 SHA-256 (8192 bytes) operations in 1009875us (25745.8 ops/sec): 210.9 MB/s
Did 1837000 SHA-512 (16 bytes) operations in 1000251us (1836539.0 ops/sec): 29.4 MB/s
Did 803000 SHA-512 (256 bytes) operations in 1000969us (802222.6 ops/sec): 205.4 MB/s
Did 41000 SHA-512 (8192 bytes) operations in 1016768us (40323.8 ops/sec): 330.3 MB/s
$ /tmp/bssl.new speed SHA-
Did 5354000 SHA-1 (16 bytes) operations in 1000104us (5353443.2 ops/sec): 85.7 MB/s
Did 1779000 SHA-1 (256 bytes) operations in 1000121us (1778784.8 ops/sec): 455.4 MB/s
Did 87000 SHA-1 (8192 bytes) operations in 1012641us (85914.0 ops/sec): 703.8 MB/s
Did 3517000 SHA-256 (16 bytes) operations in 1000114us (3516599.1 ops/sec): 56.3 MB/s
Did 935000 SHA-256 (256 bytes) operations in 1000096us (934910.2 ops/sec): 239.3 MB/s
Did 38000 SHA-256 (8192 bytes) operations in 1004476us (37830.7 ops/sec): 309.9 MB/s
Did 2930000 SHA-512 (16 bytes) operations in 1000259us (2929241.3 ops/sec): 46.9 MB/s
Did 1008000 SHA-512 (256 bytes) operations in 1000509us (1007487.2 ops/sec): 257.9 MB/s
Did 45000 SHA-512 (8192 bytes) operations in 1000593us (44973.3 ops/sec): 368.4 MB/s
$ /tmp/bssl.old speed RSA
Did 820 RSA 2048 signing operations in 1017008us (806.3 ops/sec)
Did 27000 RSA 2048 verify operations in 1015400us (26590.5 ops/sec)
Did 1292 RSA 2048 (3 prime, e=3) signing operations in 1008185us (1281.5 ops/sec)
Did 65000 RSA 2048 (3 prime, e=3) verify operations in 1011388us (64268.1 ops/sec)
Did 120 RSA 4096 signing operations in 1061027us (113.1 ops/sec)
Did 8208 RSA 4096 verify operations in 1002717us (8185.8 ops/sec)
$ /tmp/bssl.new speed RSA
Did 760 RSA 2048 signing operations in 1003351us (757.5 ops/sec)
Did 25900 RSA 2048 verify operations in 1028931us (25171.8 ops/sec)
Did 1320 RSA 2048 (3 prime, e=3) signing operations in 1040806us (1268.2 ops/sec)
Did 63000 RSA 2048 (3 prime, e=3) verify operations in 1016042us (62005.3 ops/sec)
Did 104 RSA 4096 signing operations in 1008718us (103.1 ops/sec)
Did 6875 RSA 4096 verify operations in 1093441us (6287.5 ops/sec)
$ /tmp/bssl.old speed GCM
Did 5316000 AES-128-GCM (16 bytes) seal operations in 1000082us (5315564.1 ops/sec): 85.0 MB/s
Did 712000 AES-128-GCM (1350 bytes) seal operations in 1000252us (711820.6 ops/sec): 961.0 MB/s
Did 149000 AES-128-GCM (8192 bytes) seal operations in 1003182us (148527.4 ops/sec): 1216.7 MB/s
Did 5919750 AES-256-GCM (16 bytes) seal operations in 1000016us (5919655.3 ops/sec): 94.7 MB/s
Did 800000 AES-256-GCM (1350 bytes) seal operations in 1000951us (799239.9 ops/sec): 1079.0 MB/s
Did 152000 AES-256-GCM (8192 bytes) seal operations in 1000765us (151883.8 ops/sec): 1244.2 MB/s
$ /tmp/bssl.new speed GCM
Did 5315000 AES-128-GCM (16 bytes) seal operations in 1000125us (5314335.7 ops/sec): 85.0 MB/s
Did 755000 AES-128-GCM (1350 bytes) seal operations in 1000878us (754337.7 ops/sec): 1018.4 MB/s
Did 151000 AES-128-GCM (8192 bytes) seal operations in 1005655us (150150.9 ops/sec): 1230.0 MB/s
Did 5913500 AES-256-GCM (16 bytes) seal operations in 1000041us (5913257.6 ops/sec): 94.6 MB/s
Did 782000 AES-256-GCM (1350 bytes) seal operations in 1001484us (780841.2 ops/sec): 1054.1 MB/s
Did 121000 AES-256-GCM (8192 bytes) seal operations in 1006389us (120231.8 ops/sec): 984.9 MB/s
Change-Id: I0efb32f896c597abc7d7e55c31d038528a5c72a1
Reviewed-on: https://boringssl-review.googlesource.com/6260
Reviewed-by: Adam Langley <alangley@gmail.com>
One less exported function. Nothing ever stack-allocates them, within BoringSSL
or in consumers. This avoids the slightly odd mechanism where BN_MONT_CTX_free
might or might not free the BN_MONT_CTX itself based on a flag.
(This is also consistent with OpenSSL 1.1.x which does away with the _init
variants of both this and BIGNUM so it shouldn't be a compatibility concern
long-term either.)
Change-Id: Id885ae35a26f75686cc68a8aa971e2ea6767ba88
Reviewed-on: https://boringssl-review.googlesource.com/6350
Reviewed-by: Adam Langley <alangley@gmail.com>
The warning is:
C4210: nonstandard extension used : function given file scope.
It is caused by function declarations that aren't at the top level in a
file.
Change-Id: Ib1c2ae64e15e66eb0a7255a29c0e560fbf55c2b2
Reviewed-on: https://boringssl-review.googlesource.com/6210
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The goto always jumps into the loop so the for's initialisation
expression can never be executed. Clang warns about this.
Change-Id: I3c3d4b8430754099e9ca6fd20101868c40165245
2ab24a2d40 added sections to ARM assembly
files. However, in cases where .align directives were not next to the
labels that they were intended to apply to, the section directives would
cause them to be ignored.
Change-Id: I32117f6747ff8545b80c70dd3b8effdc6e6f67e0
Reviewed-on: https://boringssl-review.googlesource.com/6050
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This change causes each global arm or aarch64 asm function to be put
into its own section by default. This matches the behaviour of the
-ffunction-sections option to GCC and allows the --gc-sections option to
the linker to discard unused asm functions on a function-by-function
basis.
Sometimes several asm functions will share the same data an, in that
situation, the data is put into the section of one of the functions and
the section of the other function is merged with the added
“.global_with_section” directive.
Change-Id: I12c9b844d48d104d28beb816764358551eac4456
Reviewed-on: https://boringssl-review.googlesource.com/6003
Reviewed-by: Adam Langley <agl@google.com>
Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.
This workaround will be removed in six months.
BUG=534766
Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>
Target date for removal of the workaround is 6 months.
BUG=532048
Change-Id: I402f75e46736936725575559cd8eb194115ab0df
Reviewed-on: https://boringssl-review.googlesource.com/5910
Reviewed-by: Adam Langley <agl@google.com>
Estonian IDs issued between September 2014 to September 2015 are broken and use
negative moduli. They last five years and are common enough that we need to
work around this bug.
Add parallel "buggy" versions of BN_cbs2unsigned and RSA_parse_public_key which
tolerate this mistake, to align with OpenSSL's previous behavior. This code is
currently hooked up to rsa_pub_decode in RSA_ASN1_METHOD so that d2i_X509 is
tolerant. (This isn't a huge deal as the rest of that stack still uses the
legacy ASN.1 code which is overly lenient in many other ways.)
In future, when Chromium isn't using crypto/x509 and has more unified
certificate handling code, we can put client certificates under a slightly
different codepath, so this needn't hold for all certificates forever. Then in
September 2019, when the broken Estonian certificates all expire, we can purge
this codepath altogether.
BUG=532048
Change-Id: Iadb245048c71dba2eec45dd066c4a6e077140751
Reviewed-on: https://boringssl-review.googlesource.com/5894
Reviewed-by: Adam Langley <agl@google.com>
History has shown there are bugs in not setting the error code
appropriately, which makes any decision making based on
|ERR_peek_last_error|, etc. suspect. Also, this call was interfering
with the link-time optimizer's ability to discard the implementations of
many functions in crypto/err during dead code elimination.
Change-Id: Iba9e553bf0a72a1370ceb17ff275f5a20fca31ec
Reviewed-on: https://boringssl-review.googlesource.com/5748
Reviewed-by: Adam Langley <agl@google.com>
Don't dereference |d| when |top| is zero. Also test that various BIGNUM
methods behave correctly on zero/even inputs.
(Imported from upstream's cf633fa00244e39eea2f2c0b623f7d5bbefa904e.)
We already had the BN_div and BN_MONT_CTX_set tests, but align them with
upstream's for consistency.
Change-Id: Ice5d04f559b4d5672e23c400637c07d8ee401727
Reviewed-on: https://boringssl-review.googlesource.com/5783
Reviewed-by: Adam Langley <agl@google.com>
BN_rand generates a single-word zero BIGNUM with quite a large
probability.
A zero BIGNUM in turn will end up having a NULL |d|-buffer, which we
shouldn't dereference without checking.
(Imported from upstream's 9c989aaa749d88b63bef5d5beeb3046eae62d836.)
Change-Id: Ic4d113e4fcf4ea4c0a4e905a1c4ba3fb758d9fc6
Reviewed-on: https://boringssl-review.googlesource.com/5782
Reviewed-by: Adam Langley <agl@google.com>
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.
It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.
Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
The function BN_MONT_CTX_set was assuming that the modulus was non-zero
and therefore that |mod->top| > 0. In an error situation that may not be
the case and could cause a seg fault.
This is a follow on from CVE-2015-1794.
(Imported from upstream's 512368c9ed4d53fb230000e83071eb81bf628b22.)
The CVE itself doesn't affect us as the bit strength check in the DHE logic
excludes zero.
Also add tests to bn_test for a couple of division by zero cases. (This and
BN_div.)
Change-Id: Ibd8ef98d6be48eb95110021c23cd8e278656764d
Reviewed-on: https://boringssl-review.googlesource.com/5690
Reviewed-by: Adam Langley <agl@google.com>
BN_bin2bn takes a size_t as it should, but it passes that into bn_wexpand which
takes unsigned. Switch bn_wexpand and bn_expand to take size_t before they
check bounds against INT_MAX.
BIGNUM itself still uses int everywhere and we may want to audit all the
arithmetic at some point. Although I suspect having bn_expand require that the
number of bits fit in an int is sufficient to make everything happy, unless
we're doing interesting arithmetic on the number of bits somewhere.
Change-Id: Id191a4a095adb7c938cde6f5a28bee56644720c6
Reviewed-on: https://boringssl-review.googlesource.com/5680
Reviewed-by: Adam Langley <agl@google.com>
Move the bn_expand call inside decode_hex; it's an implementation detail of
hex-decoding. decode_dec instead works with BN_mul_word and BN_add_word so it
can just rely on BN internally expanding things and check the return value.
Also clean up the decode_hex loop so it's somewhat more readable and check for
INT_MAX in bn_x2bn. It uses int over size_t rather pervasively, but while I'm
here at least make that function check overflow.
BUG=517474
Change-Id: I4f043973ee43071a02ea5d4313a8fdaf12404e84
Reviewed-on: https://boringssl-review.googlesource.com/5679
Reviewed-by: Adam Langley <agl@google.com>
Probably a remnant of ifdef soup somewhere.
Change-Id: I472f236a2db54a97490b22b0bbcc1701a2dba3b3
Reviewed-on: https://boringssl-review.googlesource.com/5623
Reviewed-by: Adam Langley <agl@google.com>
gyp doesn't allow two files to share the same name to avoid bugs in OS X
libtool, so asn1.c's need to all get a prefix.
Change-Id: I3593597912c49dd02655cae329fb253ed4f6f56d
Reviewed-on: https://boringssl-review.googlesource.com/5431
Reviewed-by: Adam Langley <agl@google.com>
RSA and ECDSA will both require being able to convert ASN.1 INTEGERs to
and from DER. Don't bother handling negative BIGNUMs for now. It doesn't
seem necessary and saves bothering with two's-complement vs
sign-and-magnitude.
BUG=499653
Change-Id: I1e80052067ed528809493af73b04f82539d564ff
Reviewed-on: https://boringssl-review.googlesource.com/5268
Reviewed-by: Adam Langley <agl@google.com>
BN_copy can fail on malloc failure. The case in crypto/rsa was causing the
malloc tests in all_tests.go to infinite loop.
Change-Id: Id5900512013fba9960444d78a8c056aa4314fb2d
Reviewed-on: https://boringssl-review.googlesource.com/5110
Reviewed-by: Adam Langley <agl@google.com>
bn_test's output is meant to be piped to bc, but this got broken somewhat:
- OpenSSL uses uppercase hex rather than BoringSSL's lowercase. bc only accepts
uppercase. Document that this needs some shell pipeline until we replace
them with better tests because this is all ridiculous.
- Some stderr outputs moved to stdout to avoid cluttering stdout. Just remove
them. The operations are fast enough to not need progress.
- To cut down on noise, only write the bc transcript given a command-line flag.
Also remove the -results flag since it's pointless. (It writes only the
results and not the inputs.)
Change-Id: I08f87cac1e03fab461f0dc40b9d4285bd877807d
Reviewed-on: https://boringssl-review.googlesource.com/4896
Reviewed-by: Adam Langley <agl@google.com>
If BN_rand is called with |bits| set to 1 and |top| set to 1 then a 1 byte
buffer overflow can occur.
See also upstream's efee575ad464bfb60bf72dcb73f9b51768f4b1a1. But rather than
making |BN_rand| fail, be consistent with the |bits| = 0 case and just don't
set the bits that don't exist. Add tests to ensure the degenerate cases behave.
Change-Id: I5e9fbe6fd8f7f7b2e011a680f2fbe6d7ed4dab65
Reviewed-on: https://boringssl-review.googlesource.com/4893
Reviewed-by: Adam Langley <agl@google.com>
The functions BN_rshift and BN_lshift shift their arguments to the right or
left by a specified number of bits. Unpredicatable results (including
crashes) can occur if a negative number is supplied for the shift value.
Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and Filip Palian
for discovering and reporting this issue.
(Imported from upstream's 7cc18d8158b5fc2676393d99b51c30c135502107.)
Change-Id: Ib9f5e410a46df3d7f02a61374807fba209612bd3
Reviewed-on: https://boringssl-review.googlesource.com/4892
Reviewed-by: Adam Langley <agl@google.com>
Currently far from passing and I haven't even tried with a leak checker yet.
Also bn_test is slow.
Change-Id: I4fe2783aa5f7897839ca846062ae7e4a367d2469
Reviewed-on: https://boringssl-review.googlesource.com/4794
Reviewed-by: Adam Langley <agl@google.com>
Unlike the standalone build, builds generated from util/generate_build_files.py
do not exclude x86_64-gcc.c. Match the consumer builds by making the standalone
build unconditionally include it. (This would have noticed the missing
preprocessor checks in the file.)
Change-Id: I8d20f269dea63776320ae636ee1e5339cb85fa30
Reviewed-on: https://boringssl-review.googlesource.com/4761
Reviewed-by: Adam Langley <agl@google.com>
