BUG=77
Change-Id: If568412655aae240b072c29d763a5b17bb5ca3f7
Reviewed-on: https://boringssl-review.googlesource.com/10840
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
This finally removes the last Android hack. Both Chromium and Android
end up needing this thing (Chromium needs it for WebCrypto but currently
uses the EVP_AEAD version and Android needs it by way of
wpa_supplicant).
On the Android side, the alternative is we finish upstream's
NEED_INTERNAL_AES_WRAP patch, but then it just uses its own key-wrap
implementation. This seems a little silly, considering we have a version
of key-wrap under a different API anyway.
It also doesn't make much sense to leave the EVP_AEAD API around if we
don't want people to use it and Chromium's the only consumer. Remove it
and I'll switch Chromium to the new---er, old--- APIs next roll.
Change-Id: I23a89cda25bddb6ac1033e4cd408165f393d1e6c
Reviewed-on: https://boringssl-review.googlesource.com/11410
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We have CBS_get_asn1 / CBS_get_asn1_element, but not the "any" variants
of them. Without this, a consumer walking a DER structure must manually
CBS_skip the header, which is a little annoying.
Change-Id: I7735c37eb9e5aaad2bde8407669bce5492e1ccf6
Reviewed-on: https://boringssl-review.googlesource.com/11404
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is just to reduce the diff with upstream's files so it's easier to
tell what's going on. Upstream's files have lots and lots of trailing
whitespace. We were also missing a comment.
Change-Id: Icfc3b52939823a046a3744fd8e619b5bd6160715
Reviewed-on: https://boringssl-review.googlesource.com/11408
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
If asn1_item_ex_combine_new fails in one of the ASN1_template_new calls
just before the ASN1_OP_NEW_POST call, ASN1_item_ex_free will free the
temporary object which ultimately calls ASN1_OP_FREE_POST. This means
that ASN1_OP_FREE_POST needs to account for zero-initialized objects.
Change-Id: I56fb63bd5c015d9dfe3961606449bc6f5b1259e3
Reviewed-on: https://boringssl-review.googlesource.com/11403
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Ia020ea08431859bf268d828b5d72715295de26e6
Reviewed-on: https://boringssl-review.googlesource.com/11401
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
They just need a different name for the real malloc implementations.
Change-Id: Iee1aac1133113d628fd3f9f1ed0335d66c6def24
Reviewed-on: https://boringssl-review.googlesource.com/11400
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In order to align ppc64le with the existing code, 4467e59b changed the
prefix for both the ARM and ppc64le AES assembly code to be “aes_hw_”.
However, it didn't update aes.c as well.
Change-Id: I8e3c7dea1c49ddad8a613369af274e25d572a8fd
Reviewed-on: https://boringssl-review.googlesource.com/11342
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This change adds AES and GHASH assembly from upstream, with the aim of
speeding up AES-GCM.
The PPC64LE assembly matches the interface of the ARMv8 assembly so I've
changed the prefix of both sets of asm functions to be the same
("aes_hw_").
Otherwise, the new assmebly files and Perlasm match exactly those from
upstream's c536b6be1a (from their master branch).
Before:
Did 1879000 AES-128-GCM (16 bytes) seal operations in 1000428us (1878196.1 ops/sec): 30.1 MB/s
Did 61000 AES-128-GCM (1350 bytes) seal operations in 1006660us (60596.4 ops/sec): 81.8 MB/s
Did 11000 AES-128-GCM (8192 bytes) seal operations in 1072649us (10255.0 ops/sec): 84.0 MB/s
Did 1665000 AES-256-GCM (16 bytes) seal operations in 1000591us (1664016.6 ops/sec): 26.6 MB/s
Did 52000 AES-256-GCM (1350 bytes) seal operations in 1006971us (51640.0 ops/sec): 69.7 MB/s
Did 8840 AES-256-GCM (8192 bytes) seal operations in 1013294us (8724.0 ops/sec): 71.5 MB/s
After:
Did 4994000 AES-128-GCM (16 bytes) seal operations in 1000017us (4993915.1 ops/sec): 79.9 MB/s
Did 1389000 AES-128-GCM (1350 bytes) seal operations in 1000073us (1388898.6 ops/sec): 1875.0 MB/s
Did 319000 AES-128-GCM (8192 bytes) seal operations in 1000101us (318967.8 ops/sec): 2613.0 MB/s
Did 4668000 AES-256-GCM (16 bytes) seal operations in 1000149us (4667304.6 ops/sec): 74.7 MB/s
Did 1202000 AES-256-GCM (1350 bytes) seal operations in 1000646us (1201224.0 ops/sec): 1621.7 MB/s
Did 269000 AES-256-GCM (8192 bytes) seal operations in 1002804us (268247.8 ops/sec): 2197.5 MB/s
Change-Id: Id848562bd4e1aa79a4683012501dfa5e6c08cfcc
Reviewed-on: https://boringssl-review.googlesource.com/11262
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
One of Chromium's toolchains can't handle this for some reason. See also
empty_crls and empty in TestVerify.
Change-Id: I5e6a849f3042288da2e406882ae5cfec249a86ae
Reviewed-on: https://boringssl-review.googlesource.com/11340
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We used upstream's reformat script, but they had stuck hyphens
everywhere to tell indent to leave a comment alone. Fix this one since
it was especially hard to read.
Change-Id: I9f43bd57dbcf66b79b775ad10ee67867d815ed33
Reviewed-on: https://boringssl-review.googlesource.com/11243
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We'd previously been assuming we'd want to predict P-256 and X25519 but,
on reflection, that's nonsense. Although, today, P-256 is widespread and
X25519 is less so, that's not the right question to ask. Those servers
are all 1.2.
The right question is whether we believe enough servers will get to TLS
1.3 before X25519 to justify wasting 64 bytes on all other connections.
Given that OpenSSL has already shipped X25519 and Microsoft was doing
interop testing on X25519 around when we were shipping it, I think the
answer is no.
Moreover, if we are wrong, it will be easier to go from predicting one
group to two rather than the inverse (provided we send a fake one with
GREASE). I anticipate prediction-miss HelloRetryRequest logic across the
TLS/TCP ecosystem will be largely untested (no one wants to pay an RTT),
so taking a group out of the predicted set will likely be a risky
operation.
Only predicting one group also makes things a bit simpler. I haven't
done this here, but we'll be able to fold the 1.2 and 1.3 ecdh_ctx's
together, even.
Change-Id: Ie7e42d3105aca48eb9d97e2e05a16c5379aa66a3
Reviewed-on: https://boringssl-review.googlesource.com/10960
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Found by libFuzzer and then one more mistake caught by valgrind. Add a
test for this case.
Change-Id: I92773bc1231bafe5fc069e8568d93ac0df4c8acb
Reviewed-on: https://boringssl-review.googlesource.com/11129
Reviewed-by: David Benjamin <davidben@google.com>
The MinGW setup on Android already defines this stat macro.
Change-Id: Ia8e89195c06ec01d4b5a2fa7357fb8d2d500aa06
Reviewed-on: https://boringssl-review.googlesource.com/11124
Reviewed-by: Kenny Root <kroot@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Android uses MinGW for some host tools on Windows. That toolchain
doesn't support the #pragma tricks we use for thread-local destructors,
but does appear to support pthreads.
This also lets us remove the INIT_ONCE workaround, although that's
removable anyway since Android's MinGW is now new enough.
Change-Id: I8d1573923fdaac880a50d84acbebbf87461c50d2
Reviewed-on: https://boringssl-review.googlesource.com/11125
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Kenny Root <kroot@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
C99 decided that, like PRI* macros, UINT64_C and friends should be
conditioned on __STDC_CONSTANT_MACROS in C++. C++11 then decided this
was ridiculous and overruled this decision. However, Android's headers
in older NDKs mistakenly followed the C99 rules for C++, so work around
this.
This fixes the android_arm bots.
Change-Id: I3b49e8dfc20190ebfa78876909bd0dccd3e210ea
Reviewed-on: https://boringssl-review.googlesource.com/11089
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Android currently implements this manually (see NativeBN_putULongInt) by
reaching into BIGNUM's internals. BN_ULONG is a somewhat unfortunate API
anyway as the size is platform-dependent, so add a platform-independent
way to do this.
The other things Android needs are going to need more work, but this
one's easy.
BUG=97
Change-Id: I4af4dc29f9845bdce0f0663c379b4b5d3e1dc46e
Reviewed-on: https://boringssl-review.googlesource.com/11088
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Code acting generically on an EVP_AEAD_CTX may wish to get at the
underlying EVP_AEAD.
Change-Id: I9cc905522ba76402bda4c255aa1488158323b02c
Reviewed-on: https://boringssl-review.googlesource.com/11085
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This withdraws support for -DBORINGSSL_ENABLE_RC4_TLS, and removes the
RC4 AEADs.
Change-Id: I1321b76bfe047d180743fa46d1b81c5d70c64e81
Reviewed-on: https://boringssl-review.googlesource.com/10940
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Since gcm_test now contains variable decls in for loops it needs
-std=c11. However, tests are compiled with C++ test_support files in
Bazel, which doesn't work with -std=c11.
Change-Id: Ife18c2d80b01448bb3b7ee2728412289bf749bd9
Reviewed-on: https://boringssl-review.googlesource.com/11041
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This function (actually a macro in OpenSSL) is used by several projects
(e.g. OpenResty, OpenVPN, ...) so it can useuful to provide it for
compatibility.
However, depending on the semantics of the BIO type (e.g. BIO_pair), the
return value can be meaningless, which might explain why it was removed.
Change-Id: I0e432c92222c267eb994d32b0bc28e999c4b40a7
Reviewed-on: https://boringssl-review.googlesource.com/11020
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
The stuff around i being reused for |len| rounded to a number of blocks
is a little weird.
Change-Id: I6f07a82fe84d077062e5b34ce75cc68250be8a4a
Reviewed-on: https://boringssl-review.googlesource.com/10802
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I hear our character set includes such novel symbols as '+'.
Change-Id: I96591a563317e71299748a948d68a849e15b5d60
Reviewed-on: https://boringssl-review.googlesource.com/11009
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This was done just by grepping for 'size_t i;' and 'size_t j;'. I left
everything in crypto/x509 and friends alone.
There's some instances in gcm.c that are non-trivial and pulled into a
separate CL for ease of review.
Change-Id: I6515804e3097f7e90855f1e7610868ee87117223
Reviewed-on: https://boringssl-review.googlesource.com/10801
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Really the only thing we should be doing with these ciphers is hastening
their demise, but it was the weekend and this seemed like fun.
EVP_tls_cbc_copy_mac needs to rotate a buffer by a secret amount. (It
extracts the MAC, but rotated.) We have two codepaths for this. If
CBC_MAC_ROTATE_IN_PLACE is defined (always on), we make some assumptions
abuot cache lines, play games with volatile, and hope that doesn't leak
anything. Otherwise, we do O(N^2) work to constant-time select the
rotation incidences.
But we can do O(N lg N). Rotate by powers of two and constant-time
select by the offset's bit positions. (Handwaivy lower-bound: an array
position has N possible values, so, armed with only a constant-time
select, we need O(lg N) work to resolve it. There's N array positions,
so O(N lg N).)
A microbenchmark of EVP_tls_cbc_copy_mac shows this is 27% faster than
the old one, but still 32% slower than the in-place version.
in-place:
Did 15724000 CopyFromMAC operations in 20000744us (786170.8 ops/sec)
N^2:
Did 8443000 CopyFromMAC operations in 20001582us (422116.6 ops/sec)
N lg N:
Did 10718000 CopyFromMAC operations in 20000763us (535879.6 ops/sec)
This results in the following the CBC ciphers. I measured
AES-128-CBC-SHA1 and AES-256-CBC-SHA384 which are, respectively, the
cipher where the other bits are the fastest and the cipher where N is
largest.
in-place:
Did 2634000 AES-128-CBC-SHA1 (16 bytes) open operations in 10000739us (263380.5 ops/sec): 4.2 MB/s
Did 1424000 AES-128-CBC-SHA1 (1350 bytes) open operations in 10002782us (142360.4 ops/sec): 192.2 MB/s
Did 531000 AES-128-CBC-SHA1 (8192 bytes) open operations in 10002460us (53086.9 ops/sec): 434.9 MB/s
N^2:
Did 2529000 AES-128-CBC-SHA1 (16 bytes) open operations in 10001474us (252862.7 ops/sec): 4.0 MB/s
Did 1392000 AES-128-CBC-SHA1 (1350 bytes) open operations in 10006659us (139107.4 ops/sec): 187.8 MB/s
Did 528000 AES-128-CBC-SHA1 (8192 bytes) open operations in 10001276us (52793.3 ops/sec): 432.5 MB/s
N lg N:
Did 2531000 AES-128-CBC-SHA1 (16 bytes) open operations in 10003057us (253022.7 ops/sec): 4.0 MB/s
Did 1390000 AES-128-CBC-SHA1 (1350 bytes) open operations in 10003287us (138954.3 ops/sec): 187.6 MB/s
Did 531000 AES-128-CBC-SHA1 (8192 bytes) open operations in 10002448us (53087.0 ops/sec): 434.9 MB/s
in-place:
Did 1249000 AES-256-CBC-SHA384 (16 bytes) open operations in 10001767us (124877.9 ops/sec): 2.0 MB/s
Did 879000 AES-256-CBC-SHA384 (1350 bytes) open operations in 10009244us (87818.8 ops/sec): 118.6 MB/s
Did 344000 AES-256-CBC-SHA384 (8192 bytes) open operations in 10025897us (34311.1 ops/sec): 281.1 MB/s
N^2:
Did 1072000 AES-256-CBC-SHA384 (16 bytes) open operations in 10008090us (107113.3 ops/sec): 1.7 MB/s
Did 780000 AES-256-CBC-SHA384 (1350 bytes) open operations in 10007787us (77939.3 ops/sec): 105.2 MB/s
Did 333000 AES-256-CBC-SHA384 (8192 bytes) open operations in 10016332us (33245.7 ops/sec): 272.3 MB/s
N lg N:
Did 1168000 AES-256-CBC-SHA384 (16 bytes) open operations in 10007671us (116710.5 ops/sec): 1.9 MB/s
Did 836000 AES-256-CBC-SHA384 (1350 bytes) open operations in 10001536us (83587.2 ops/sec): 112.8 MB/s
Did 339000 AES-256-CBC-SHA384 (8192 bytes) open operations in 10018522us (33837.3 ops/sec): 277.2 MB/s
TLS CBC performance isn't as important as it was before, and the costs
aren't that high, so avoid making assumptions about cache lines. (If we
care much about CBC open performance, we probably should get the malloc
out of EVP_tls_cbc_digest_record at the end.)
Change-Id: Ib8d8271be4b09e5635062cd3b039e1e96f0d9d3d
Reviewed-on: https://boringssl-review.googlesource.com/11003
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
For consistency and to avoid a pedantic GCC warning (even though it's
mostly old legacy code).
Change-Id: Iea63eb0a82ff52914adc33b83e48450f4f6a49ef
Reviewed-on: https://boringssl-review.googlesource.com/11021
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
In OpenSSL, they're used in the 32-bit x86 Blowfish, CAST, DES, and RC5
assembly bits. We don't have any of those.
Change-Id: I36f22ca873842a200323cd3f398d2446f7bbabca
Reviewed-on: https://boringssl-review.googlesource.com/10780
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 2a20b6d9731488bcb500e58a434375f59fb9adcc)
Change-Id: If3db4dac3d4cd675cf7854c4e154823d25d00eb9
Reviewed-on: https://boringssl-review.googlesource.com/10921
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
(Imported from upstream's a404656a8b40d9f1172e5e330f7e2d9d87cabab8)
Change-Id: I4ddebfbaeab433bae7c1393a8258d786801bb633
Reviewed-on: https://boringssl-review.googlesource.com/10920
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Now that we have the extern "C++" trick, we can just embed them in the
normal headers. Move the EVP_CIPHER_CTX deleter to cipher.h and, in
doing so, take away a little bit of boilerplate in defining deleters.
Change-Id: I4a4b8d0db5274a3607914d94e76a38996bd611ec
Reviewed-on: https://boringssl-review.googlesource.com/10804
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Unlike the Scoped* types, bssl::UniquePtr is available to C++ users, and
offered for a large variety of types. The 'extern "C++"' trick is used
to make the C++ bits digestible to C callers that wrap header files in
'extern "C"'.
Change-Id: Ifbca4c2997d6628e33028c7d7620c72aff0f862e
Reviewed-on: https://boringssl-review.googlesource.com/10521
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These test vectors include the k value, so we can get a deterministic
test.
Change-Id: Ie3cb61a99203cd55b01f4835be7c32043309748d
Reviewed-on: https://boringssl-review.googlesource.com/10701
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Changing parameters on renegotiation makes all our APIs confusing. This
one has no reason to change, so lock it down. In particular, our
preference to forbid Token Binding + renego may be overridden at the
IETF, even though it's insane. Loosening it will be a bit less of a
headache if EMS can't change.
https://www.ietf.org/mail-archive/web/unbearable/current/msg00690.html
claims that this is already in the specification and enforced by NSS. I
can't find anything to this effect in the specification. It just says
the client MUST disable renegotiation when EMS is missing, which is
wishful thinking. At a glance, NSS doesn't seem to check, though I could
be misunderstanding the code.
Nonetheless, locking this down is a good idea anyway. Accurate or not,
take the email as an implicit endorsement of this from Mozilla.
Change-Id: I236b05991d28bed199763dcf2f47bbfb9d0322d7
Reviewed-on: https://boringssl-review.googlesource.com/10721
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This removes all but the generic C RC4 implementation. At this point we
want to optimize for size/simplicity rather than speed.
See also upstream's 3e9e810f2e047effb1056211794d2d12ec2b04e7 which
removed the RC4_CHUNK code and standardized on RC4_INDEX. A
since-removed comment says that it was implemented for "pre-21164a Alpha
CPUs don't have byte load/store instructions" and helps with SPARC and
MIPS.
This also removes all the manual loop unrolling.
Change-Id: I91135568483260b2e1e675f190fb00ce8f9eff3d
Reviewed-on: https://boringssl-review.googlesource.com/10720
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This and the following commits will import NIST's ECC test vectors.
Right now all our tests pass if I make P-224 act like P-521, which is
kind of embarrassing. (Other curves are actually tested, but only
because runner.go tests them against BoGo.)
Change-Id: Id0b20451ebd5f10f1d09765a810ad140bea28fa0
Reviewed-on: https://boringssl-review.googlesource.com/10700
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I85216184f9277ce0c0caae31e379b638683e28c5
Reviewed-on: https://boringssl-review.googlesource.com/10703
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We may need to implement high tag number form someday. CBS_get_asn1 has
an unsigned output to allow for this, but CBB_add_asn1 takes a uint8_t
(I think this might be my fault). Fix that which also fixes a
-Wconversion warning.
Simply leaving room in tag representation will still cause troubles
because the class and constructed bits overlap with bits for tag numbers
above 31. Probably the cleanest option would be to shift them to the top
3 bits of a u32 and thus not quite match the DER representation. Then
CBS_get_asn1 and CBB_add_asn1 will internally munge that into the DER
representation and consumers may continue to write things like:
tag_number | CBS_ASN1_CONTEXT_SPECIFIC
I haven't done that here, but in preparation for that, document that
consumers need to use the values and should refrain from assuming the
correspond to DER.
Change-Id: Ibc76e51f0bc3b843e48e89adddfe2eaba4843d12
Reviewed-on: https://boringssl-review.googlesource.com/10502
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Some, very recent, versions of Clang now support `.arch`. Allow them to
see these directives with BORINGSSL_CLANG_SUPPORTS_DOT_ARCH.
BUG=39
Change-Id: I122ab4b3d5f14502ffe0c6e006950dc64abf0201
Reviewed-on: https://boringssl-review.googlesource.com/10600
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
nginx consumes these error codes without #ifdefs. Continue to define
them for compatibility, even though we never emit them.
BUG=95
Change-Id: I1e991987ce25fc4952cc85b98ffa050a8beab92e
Reviewed-on: https://boringssl-review.googlesource.com/10446
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
958aaf1ea1, imported from upstream, had an
off-by-one error. Reproducing the failure is fairly easy as it can't
even serialize 1. See also upstream's
099e2968ed3c7d256cda048995626664082b1b30.
Rewrite the function completely with CBB and add a basic test.
BUG=chromium:639740
Change-Id: I41a91514c4bb9e83854824ed5258ffe4e49d9491
Reviewed-on: https://boringssl-review.googlesource.com/10540
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: Ie60744761f5aa434a71a998f5ca98a8f8b1c25d5
Reviewed-on: https://boringssl-review.googlesource.com/10447
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I2e1ee319bb9852b9c686f2f297c470db54f72279
Reviewed-on: https://boringssl-review.googlesource.com/10370
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I found an earlier reference for an algorithm for the optimized
computation of n0 that is very similar to the one in the "Montgomery
Multiplication" paper cited in the comments. Add a reference to it.
Henry S. Warren, Jr. pointed out that his "Montgomery Multiplication"
paper is not a chapter of his book, but a supplement to the book.
Correct the reference to it.
Change-Id: Iadeb148c61ce646d1262ccba0207a31ebdad63e9
Reviewed-on: https://boringssl-review.googlesource.com/10480
Reviewed-by: Adam Langley <agl@google.com>
This was causing some Android breakage. The real bug is actually
entirely in Android for getting its error-handling code wrong and not
handling multiple errors. I'll fix that. (See b/30917411.)
That said, BN_R_NO_INVERSE is a perfectly legitimate reason for those
operations to fail, so ERR_R_INTERNAL_ERROR isn't really a right thing
to push in front anyway. We're usually happy enough with single-error
returns (I'm still a little skeptical of this queue idea), so let's just
leave it at that.
Change-Id: I469b6e2b5987c6baec343e2cfa52bdcb6dc42879
Reviewed-on: https://boringssl-review.googlesource.com/10483
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
If an oversize BIGNUM is presented to BN_bn2dec() it can cause
BN_div_word() to fail and not reduce the value of 't' resulting
in OOB writes to the bn_data buffer and eventually crashing.
Fix by checking return value of BN_div_word() and checking writes
don't overflow buffer.
Thanks to Shi Lei for reporting this bug.
CVE-2016-2182
(Imported from upstream's e36f27ddb80a48e579783bc29fb3758988342b71.)
Change-Id: Ib9078921b4460952c4aa5a6b03ec39a03704bb90
Reviewed-on: https://boringssl-review.googlesource.com/10367
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
RT#4530
(Imported from upstream's 7123aa81e9fb19afb11fdf3850662c5f7ff1f19c.)
We've yet to enable this code, but this confirms that we do indeed need
to get our future all-variants stuff working on Windows as well as
Linux and find an AVX2-capable CI setup on each.
The crash here is caused by some win64-only code using %rax as a frame
pointer (perlasm injects a mov rax,rsp in the prologue of every win64
function).
Change-Id: Ifbe59ceb6ae29266d9cf8a461920344a32b6e555
Reviewed-on: https://boringssl-review.googlesource.com/10366
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Check for error return in BN_div_word().
(Imported from upstream's d871284aca5524c85a6460119ac1b1e38f7e19c6.)
This function is only called from crypto/obj to convert strings like
"1.2.3.4.5" to OIDs. We may wish to see about rewriting it just so it's
out of the way.
Change-Id: Ia8379d2dd30606f6a81ce24dee8852312cb7f127
Reviewed-on: https://boringssl-review.googlesource.com/10365
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These functions are unused. Upstream recently needed to limit recursion
depth on this function in 81f69e5b69b8e87ca5d7080ab643ebda7808542c. It
looks like deeply nested BER constructed strings could cause unbounded
stack usage. Delete the function rather than import the fix.
Change-Id: I7868080fae52b46fb9f9147543c0f7970d8fff98
Reviewed-on: https://boringssl-review.googlesource.com/10368
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These are never used internally or externally. Upstream had some
bugfixes to them recently. Delete them instead.
Change-Id: I44a6cce1dac2c459237f6d46502657702782061b
Reviewed-on: https://boringssl-review.googlesource.com/10364
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
(Imported from upstream's b10c10422a9ec4db426be3ef99031f0807d2ded0,
ff8b6b92f44c682ad78f60c32ec154e0bfabebb2, and
134ab5139a8d41455a81d9fcc31b3edb8a4b2f5c.)
Change-Id: Icf1661a4d0249ae5af72cda15b12822b86e35a82
Reviewed-on: https://boringssl-review.googlesource.com/10361
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The weird function thing is a remnant of OpenSSL and I think something
weird involving Windows and symbols exported from dlls. These aren't
exposed in the public API, so have everything point to the tables
directly.
This is in preparation for making built-in EC_GROUPs static. (The static
EC_GROUPs won't be able to call a function wrapper.)
BUG=20
Change-Id: If33888430f32e51f48936db4046769aa1894e3aa
Reviewed-on: https://boringssl-review.googlesource.com/10346
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The old one was written somewhat weirdly.
Change-Id: I414185971a7d70105fded558da6d165570429d31
Reviewed-on: https://boringssl-review.googlesource.com/10345
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
A lot of codepaths are unreachable since the EC_GROUP is known to be
blank.
Change-Id: I5829934762e503241aa73f833c982ad9680d8856
Reviewed-on: https://boringssl-review.googlesource.com/10344
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
By using memcpy, GCC can already optimise things so that the compiled
code is identical on x86-64. Thus we don't need to worry about having
different versions for platforms with, and without, strict alignment.
(Thanks to Emil Mikulic.)
Change-Id: I08bc5fa9b67aa369be2dd2e29e4229fb5b5ff40c
Reviewed-on: https://boringssl-review.googlesource.com/10381
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I didn't look into whether this was reachable, but I assume not. Still,
better to be robust here becasue DH groups are commonly under some
amount of attacker control.
Change-Id: I1e0c33ccf314c73a9d34dd48312f6f7580049ba7
Reviewed-on: https://boringssl-review.googlesource.com/10261
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The server should not be allowed select a protocol that wasn't
advertised. Callers tend to not really notice and act as if some default
were chosen which is unlikely to work very well.
Change-Id: Ib6388db72f05386f854d275bab762ca79e8174e6
Reviewed-on: https://boringssl-review.googlesource.com/10284
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Since we are eliminating DHE support in TLS, this is just a waste of
bytes.
Change-Id: I3a23ece564e43f7e8874d1ec797def132ba59504
Reviewed-on: https://boringssl-review.googlesource.com/10260
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This more accurately reflects the documented contract for
|BN_mod_inverse_odd|.
Change-Id: Iae98dabe3943231859eaa5e798d06ebe0231b9f1
Reviewed-on: https://boringssl-review.googlesource.com/9160
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
In OpenSSL 1.1.0, this API has been renamed to gain a BN prefix. Now
that it's no longer squatting on a namespace, provide the function so
wpa_supplicant needn't carry a BoringSSL #ifdef here.
BUG=91
Change-Id: Iac8e90238c816caae6acf0e359893c14a7a970f1
Reviewed-on: https://boringssl-review.googlesource.com/10223
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The name of this has been annoying me every time I've seen it over the
past couple of days. Having a flag with a negation in the name isn't
always bad, but I think this case was.
Change-Id: I5922bf4cc94eab8c59256042a9d9acb575bd40aa
Reviewed-on: https://boringssl-review.googlesource.com/10242
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This gets cURL building against both BoringSSL as it is and BoringSSL
with OPENSSL_VERSION_NUMBER set to 1.1.0.
BUG=91
Change-Id: I5be73b84df701fe76f3055b1239ae4704a931082
Reviewed-on: https://boringssl-review.googlesource.com/10180
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The old one was rather confusing. Switch to returning 1/0 for whether
the padding is publicly invalid and then add an output argument which
returns a constant_time_eq-style boolean.
Change-Id: Ieba89d352faf80e9bcea993b716f4b2df5439d4b
Reviewed-on: https://boringssl-review.googlesource.com/10222
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Add the following cases:
- Maximal padding
- Maximal padding with each possible byte position wrong.
- When the input is not publicly too short to find a MAC, but the
unpadded value is too short. (This tests that
EVP_tls_cbc_remove_padding and EVP_tls_cbc_copy_mac coordinate
correctly. EVP_tls_cbc_remove_padding promises to also consider it
invalid padding if there is no room for a MAC.)
Change-Id: I8fe18121afb915e579a8236d0e3ef354f1f835bc
Reviewed-on: https://boringssl-review.googlesource.com/10182
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I44bc5979cb8c15ad8c4f9bef17049312b6f23a41
Reviewed-on: https://boringssl-review.googlesource.com/10200
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Use a separate |size_t| variable for all logic that happens after the
special casing of the negative values of the signed parameter, to
minimize the amount of mixed signed/unsigned math used.
Change-Id: I4aeb1ffce47f889f340f9583684910b0fb2ca7c7
Reviewed-on: https://boringssl-review.googlesource.com/9173
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
There is a comment "Note from a test above this value is guaranteed to
be non-negative". Reorganize the code to make it more clear that that
is actually the case, especially in the case where sLen == -1.
Change-Id: I09a3dd99458e34102c42d8d3a2f22c16c684c673
Reviewed-on: https://boringssl-review.googlesource.com/9172
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Initial stab at moving contents of scoped_types.h into
include/openssl/c++ and into the |bssl| namespace.
Started with one file. Will do the remaining ones once this looks good.
Change-Id: I51e2f7c1acbe52d508f1faee7740645f91f56386
Reviewed-on: https://boringssl-review.googlesource.com/9175
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This makes it easier to understand the |sLen|-related logic.
Change-Id: I98da4f4f7c82d5481544940407e6cc6a963f7e5b
Reviewed-on: https://boringssl-review.googlesource.com/9171
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The old implementation had a lot of size_t/int confusion. It also
accepted non-minimally-encoded OIDs. Unlike the old implementation, the
new one does not fall back to BIGNUMs and does not attempt to
pretty-print OIDs with components which do not fit in a uint64_t. Add
tests for these cases.
With this new implementation, hopefully we'll have a much easier time
enabling MSVC's size_t truncation warning later.
Change-Id: I602102b97cf9b02d874644f8ef67fe9bac70e45e
Reviewed-on: https://boringssl-review.googlesource.com/9131
Reviewed-by: Adam Langley <agl@google.com>
This eliminates duplicate logic.
Change-Id: I283273ae152f3644df4384558ee4a021f8c2d454
Reviewed-on: https://boringssl-review.googlesource.com/9104
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
BN_mod_inverse_odd was always being used on 64-bit platforms and was being used
for all curves with an order of 450 bits or smaller (basically, everything but
P-521). We generally don't care much about minor differences in the speed of
verifying signatures using curves other than P-256 and P-384. It is better to
always use the same algorithm.
This also allows |bn_mod_inverse_general|, |bn_mod_inverse_no_branch|, and
|BN_mod_inverse| to be dropped from programs that can somehow avoid linking in
the RSA key generation and RSA CRT recovery code.
Change-Id: I79b94bff23d2b07d5e0c704f7d44538797f8c7a0
Reviewed-on: https://boringssl-review.googlesource.com/9103
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The main RSA public modulus size of concern is 2048 bits.
bn_mod_inverse_odd is already used for public moduli of 2048 bits and
smaller on 64-bit platforms, so for 64-bit it is a no-op. For 32-bit
x86, this seems to slightly decrease the speed of RSA signing, but not
by a lot, and plus we don't care about RSA signing performance much on
32-bit platforms. It's better to have all platforms using the same
algorithms.
Change-Id: I869dbfc98994e36a04a535c1fe63b14a902a4f13
Reviewed-on: https://boringssl-review.googlesource.com/9102
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is a step towards exposing |bn_mod_inverse_odd| for use outside
of crypto/bn/gcd.c.
Change-Id: I2968f1e43306c03775b3573a022edd92f4e91df2
Reviewed-on: https://boringssl-review.googlesource.com/9101
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is in preparation for factoring out the binary Euclidean
implementation (the one used for odd numbers that aren't too big) for
direct use from outside of crypto/bn/gcd.c. The goal is to make the
resultant |BN_mod_inverse_odd|'s signature similar to
|BN_mod_inverse_blinded|. Thus, the logic for reducing the final result
isn't factored out because that yet-to-be-created |BN_mod_inverse_odd|
will need to do it itself.
Change-Id: Iaecb79fb17d13c774c4fb6ade8742937780b0006
Reviewed-on: https://boringssl-review.googlesource.com/9100
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is very far from all of it, but I did some easy ones before I got
bored. Snapshot the progress until someone else wants to continue this.
BUG=22
Change-Id: I2609e9766d883a273e53e01a75a4b1d4700e2436
Reviewed-on: https://boringssl-review.googlesource.com/9132
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Fix non-standard variable names, return value convention, unsigned vs
size_t, etc. This also fixes one size_t truncation warning.
BUG=22
Change-Id: Ibe083db90e8dac45d64da9ead8f519dd2fea96ea
Reviewed-on: https://boringssl-review.googlesource.com/9133
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
OBJ_obj2txt's implementation is kind of scary. Also it casts between int
and size_t a lot. In preparation for rewriting it, add a test.
Change-Id: Iefb1d0cddff58d67e5b04ec332477aab8aa687b6
Reviewed-on: https://boringssl-review.googlesource.com/9130
Reviewed-by: Adam Langley <agl@google.com>
We'd gotten rid of the macros, but not the underlying asn1_GetSequence
which is unused. Sadly this doesn't quite get rid of ASN1_(const_)?CTX.
There's still some code in the rest of crypto/asn1 that uses it.
Change-Id: I2ba8708ac5b20982295fbe9c898fef8f9b635704
Reviewed-on: https://boringssl-review.googlesource.com/9113
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
|BN_mod_exp_mont| uses |BN_nnmod| so it seems like
|BN_mod_exp_mont_consttime| should too. Further, I created
these test vectors by doing the math by hand, and the tests
passed for |BN_mod_exp_mont| but failed for
|BN_mod_exp_mont_consttime| without this change.
Change-Id: I7cffa1375e94dd8eaee87ada78285cd67fff1bac
Reviewed-on: https://boringssl-review.googlesource.com/9032
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Have |bn_correct_top| fix |bn->neg| if the input is zero so that we
don't have negative zeros lying around.
Thanks to Brian Smith for noticing.
Change-Id: I91bcadebc8e353bb29c81c4367e85853886c8e4e
Reviewed-on: https://boringssl-review.googlesource.com/9074
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BUG=59
Change-Id: If3a788ec1328226d69293996845fa1d14690bf40
Reviewed-on: https://boringssl-review.googlesource.com/9068
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
If two CRLs are equivalent then use the one with a later lastUpdate field:
this will result in the newest CRL available being used.
(Imported from upstream's 325da8231c8d441e6bb7f15d1a5a23ff63c842e5 and
3dc160e9be6dcaeec9345fbb61b1c427d7026103.)
Change-Id: I8c722663b979dfe08728d091697d8b8204dc265c
Reviewed-on: https://boringssl-review.googlesource.com/8947
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Negative zeros are nuts, but it will probably be a while before we've
fixed everything that can create them. Fix both to consistently print
'-0' rather than '0' so failures are easier to diagnose (BN_cmp believes
the values are different.)
Change-Id: Ic38d90601b43f66219d8f44ca085432106cf98e3
Reviewed-on: https://boringssl-review.googlesource.com/9073
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Simplify the calculation of the Montgomery constants in
|BN_MONT_CTX_set|, making the inversion constant-time. It should also
be faster by avoiding any use of the |BIGNUM| API in favor of using
only 64-bit arithmetic.
Now it's obvious how it works. /s
Change-Id: I59a1e1c3631f426fbeabd0c752e0de44bcb5fd75
Reviewed-on: https://boringssl-review.googlesource.com/9031
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
A caller using EVP_Digest* which a priori knows tighter bounds on the
hash function used (perhaps because it is always a particular hash) can
assume the function will not write more bytes than the size of the hash.
The letter of the rules before vaguely[*] allowed for more than
EVP_MD_MAX_SIZE bytes written which made for some unreasonable code in
Chromium. Officially clarify this and add tests which, when paired with
valgrind and ASan prove it.
BUG=59
[*] Not really. I think it already promised the output length will be
both the number of bytes written and the size of the hash and the size
of the hash is given by what the function promises to compute. Meh.
Change-Id: I736d526e81cca30475c90897bca896293ff30278
Reviewed-on: https://boringssl-review.googlesource.com/9066
Reviewed-by: Eric Roman <ericroman@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We managed to mix two comment styles in the Go license headers and
copy-and-paste it throughout the project.
Change-Id: Iec1611002a795368b478e1cae0b53127782210b1
Reviewed-on: https://boringssl-review.googlesource.com/9060
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Yo dawg I herd you like blinding so I put inversion blinding in your
RSA blinding so you can randomly mask your random mask.
This improves upon the current situation where we pretend that
|BN_mod_inverse_no_branch| is constant-time, and it avoids the need to
exert a lot of effort to make a actually-constant-time modular
inversion function just for RSA blinding.
Note that if the random number generator weren't working correctly then
the blinding of the inversion wouldn't be very effective, but in that
case the RSA blinding itself would probably be completely busted, so
we're not really losing anything by relying on blinding to blind the
blinding.
Change-Id: I771100f0ad8ed3c24e80dd859ec22463ef2a194f
Reviewed-on: https://boringssl-review.googlesource.com/8923
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This also adds a missing OPENSSL_EXPORT.
Change-Id: I6c2400246280f68f51157e959438644976b1171b
Reviewed-on: https://boringssl-review.googlesource.com/9041
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
There are many cases where we need |BN_rand_range| but with a minimum
value other than 0. |BN_rand_range_ex| provides that.
Change-Id: I564326c9206bf4e20a37414bdbce16a951c148ce
Reviewed-on: https://boringssl-review.googlesource.com/8921
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I6d552d26b3d72f6fffdc4d4d9fc3b5d82fb4e8bb
Reviewed-on: https://boringssl-review.googlesource.com/9010
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Fermat's Little Theorem is already used for the custom curve implementations.
Use it, for the same reasons, for the ec_montgomery-based implementations.
I tested the performance (only) on x86-64 Windows.
Change-Id: Ibf770fd3f2d3e2cfe69f06bc12c81171624ff557
Reviewed-on: https://boringssl-review.googlesource.com/8924
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Zero is only a valid input to or output of |BN_mod_inverse| when the
modulus is one. |BN_MONT_CTX_set| actually depends on this, so test
that this works.
Change-Id: Ic18f1fe786f668394951d4309020c6ead95e5e28
Reviewed-on: https://boringssl-review.googlesource.com/8922
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Some gerrit git hook says this is necessary.
Change-Id: I8a7a0a0e6732688c965b43824fe54b2db79a4919
Reviewed-on: https://boringssl-review.googlesource.com/8990
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
|BN_mod_inverse| is expensive and leaky. In this case, we can avoid
it completely by taking advantage of the fact that we already have
the two values that are supposed to be inverses of each other.
Change-Id: I2230b4166fb9d89c7445f9f7c045a4c9e4c377b3
Reviewed-on: https://boringssl-review.googlesource.com/8925
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Besides reducing code duplication, also move the relative location of
the check of |count|. Previously, the code was generating a random
value and then terminating the loop without using it if |count| went
to zero. Now the wasted call to |BN_rand| is not made.
Also add a note about the applicability of the special case logic for
|range| of the form |0b100...| to RSA blinding.
Change-Id: Iaa33b9529f1665ac59aefcc8b371fa32445e7578
Reviewed-on: https://boringssl-review.googlesource.com/8960
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
One less random environment variable for us to be sensitive to. (We
should probably unwind all this proxy cert stuff. I don't believe they
are ever enabled.)
Change-Id: I74993178679ea49e60c81d8416e502cbebf02ec9
Reviewed-on: https://boringssl-review.googlesource.com/8948
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
(Imported from upstream's a9b23465243b6d692bb0b419bdbe0b1f5a849e9c,
5e102f96eb6fcdba1db2dba41132f92fa492aea0, and
9bda72880113b2b2262d290b23bdd1d3b19ff5b3.)
Change-Id: Ib608acb86cc128cacf20811c21bf6b38b0520106
Reviewed-on: https://boringssl-review.googlesource.com/8944
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
tag2nbyte had -1 at 18th position, but underlying ASN1_mbstring_copy
supports NumericString. tag2nbyte is also used in do_print_ex which will
not be broken by setting 1 at 18th position of tag2nbyte
(Imported from upstream's bd598cc405e981de259a07558e600b5a9ef64bd6.)
Change-Id: Ie063afcaac8a7d5046cdb385059b991b92cd6659
Reviewed-on: https://boringssl-review.googlesource.com/8946
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The selector field could be omitted because it has a DEFAULT value.
In this case *sfld == NULL (sfld can never be NULL). This was not
noticed because this was never used in existing ASN.1 modules.
(Imported from upstream's c4210673313482edacede58d92e92c213d7a181a.)
svaldez and I stared at this for a while and we believe this change is
correct. It's also irrelevant because our only remaining ADB (ANY
DEFINED BY) table is POLICYQUALINFO which does not allow its selector to
be omitted. Also, if it did, it would be a slight change in behavior.
We'd switch from using POLICYQUALINFO's default_tt (filling in an
ASN1_ANY) to its null_tt (which doesn't exist, so error).
Change-Id: If6a929e3dafca18431775b01958d0dae1c09f3b4
Reviewed-on: https://boringssl-review.googlesource.com/8943
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This imports upstream's b62e9bf5cbbe278b7e0017c9234999dae68ee867 and
c3bc7f498815b355533d96b54b9a09e030d4130c. This is a no-op since we don't
use the XTS bits though keep the files in sync so long as we have them.
Comparing to master, we're now up-to-date on that file except for
a285992763f3961f69a8d86bf7dfff020a08cef9. (I've left that alone since
that touches lots of files and we should probably get better test
configuration before importing something scary like #undef __thumb2__.)
Change-Id: Ie0556757c954ef559e03a6d62c940d5901ca704a
Reviewed-on: https://boringssl-review.googlesource.com/8945
Reviewed-by: Adam Langley <agl@google.com>
All other CBB_add_u<N> functions take a narrowed type, but not every
uint32_t may fit in a u24. Check for this rather than silently truncate.
Change-Id: I23879ad0f4d2934f257e39e795cf93c6e3e878bf
Reviewed-on: https://boringssl-review.googlesource.com/8940
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
It's only called in one place. The comment about stack-allocated BIOs no
longer applies.
Change-Id: I5a3cec30bcb46bf1ee2bffd6117485383520b314
Reviewed-on: https://boringssl-review.googlesource.com/8902
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BN_mod_mul_montgomery has a problem where the modulus is much smaller
than one of the arguments. While bn_test.cc knows this and reduces the
inputs before testing |BN_mod_mul_montgomery|, none of the previous test
vectors actually failed without this. (Except those that passed negative
vaules.)
This change adds tests where M ≪ A and B.
Change-Id: I53b5188ea5fb5e48d0d197718ed33c644cde8477
Reviewed-on: https://boringssl-review.googlesource.com/8890
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Brian Smith <brian@briansmith.org>
Commit-Queue: David Benjamin <davidben@google.com>
It seems risky in the context of cross-signed certificates when the
same certificate might have multiple potential issuers. Also rarely
used, since chains in OpenSSL typically only employ self-signed
trust-anchors, whose self-signatures are not checked, while untrusted
certificates are generally ephemeral.
(Imported from upstream's 0e76014e584ba78ef1d6ecb4572391ef61c4fb51.)
This is in master and not 1.0.2, but having a per-certificate signature
cache when this is a function of signature and issuer seems dubious at
best. Thanks to Viktor Dukhovni for pointing this change out to me.
(And for making the original change upstream, of course.)
Change-Id: Ie692d651726f14aeba6eaab03ac918fcaedb4eeb
Reviewed-on: https://boringssl-review.googlesource.com/8880
Reviewed-by: Adam Langley <agl@google.com>
Revert 3f3358ac15. Add documentation
clarifying the misunderstanding that lead to the mistake, and make use
of the recently-added |bn_set_words|.
Change-Id: I58814bace3db3b0b44e2dfe09c44918a4710c621
Reviewed-on: https://boringssl-review.googlesource.com/8831
Reviewed-by: Adam Langley <agl@google.com>
Our CBB patterns do not make it safe to use a CBB after any operation
failed. Suppose one does:
int add_to_cbb(CBB *cbb) {
CBB child;
return CBB_add_u8(cbb, 1) &&
CBB_add_u8_length_prefixed(cbb, &child) &&
CBB_add_u8(&child, 2) &&
/* Flush |cbb| before |child| goes out of scoped. */
CBB_flush(cbb);
}
If one of the earlier operations fails, any attempt to use |cbb| (except
CBB_cleanup) would hit a memory error. Doing this would be a bug anyway,
since the CBB would be in an undefined state anyway (wrote only half my
object), but the memory error is bad manners.
Officially document that using a CBB after failure is illegal and, to
avoid the memory error, set a poison bit on the cbb_buffer_st to prevent
all future operations. In theory we could make failure +
CBB_discard_child work, but this is not very useful and would require a
more complex CBB pattern.
Change-Id: I4303ee1c326785849ce12b5f7aa8bbde6b95d2ec
Reviewed-on: https://boringssl-review.googlesource.com/8840
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This adds the machinery for doing TLS 1.3 1RTT.
Change-Id: I736921ffe9dc6f6e64a08a836df6bb166d20f504
Reviewed-on: https://boringssl-review.googlesource.com/8720
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is the equivalent of FragmentAcrossChangeCipherSuite for DTLS. It
is possible for us to, while receiving pre-CCS handshake messages, to
buffer up a message with sequence number meant for a post-CCS Finished.
When we then get to the new epoch and attempt to read the Finished, we
will process the buffered Finished although it was sent with the wrong
encryption.
Move ssl_set_{read,write}_state to SSL_PROTOCOL_METHOD hooks as this is
a property of the transport. Notably, read_state may fail. In DTLS
check the handshake buffer size. We could place this check in
read_change_cipher_spec, but TLS 1.3 has no ChangeCipherSpec message, so
we will need to implement this at the cipher change point anyway. (For
now, there is only an assert on the TLS side. This will be replaced with
a proper check in TLS 1.3.)
Change-Id: Ia52b0b81e7db53e9ed2d4f6d334a1cce13e93297
Reviewed-on: https://boringssl-review.googlesource.com/8790
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
prk should be a const parameter.
Change-Id: I2369ed9f87fc3c59afc07d3b667b86aec340052e
Reviewed-on: https://boringssl-review.googlesource.com/8810
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
For now, skip the 1.2 -> 1.1 signal since that will affect shipping
code. We may as well enable it too, but wait until things have settled
down. This implements the version in draft-14 since draft-13's isn't
backwards-compatible.
Change-Id: I46be43e6f4c5203eb4ae006d1c6a2fe7d7a949ec
Reviewed-on: https://boringssl-review.googlesource.com/8724
Reviewed-by: David Benjamin <davidben@google.com>
Upstream have added |EVP_PKEY_up_ref|, but their version returns an int.
Having this function with a different signature like that is dangerous
so this change aligns BoringSSL with upstream. Users of this function in
Chromium and internally should already have been updated.
Change-Id: I0a7aeaf1a1ca3b0f0c635e2ee3826aa100b18157
Reviewed-on: https://boringssl-review.googlesource.com/8736
Reviewed-by: David Benjamin <davidben@google.com>
libssh2 expects this function.
Change-Id: Ie2d6ceb25d1b633e1363e82f8a6c187b75a4319f
Reviewed-on: https://boringssl-review.googlesource.com/8735
Reviewed-by: David Benjamin <davidben@google.com>
Rather than blindly select SHA-1 if we can't find a matching one, act as
if the peer advertised rsa_pkcs1_sha1 and ecdsa_sha1. This means that we
will fail the handshake if no common algorithm may be found.
This is done in preparation for removing the SHA-1 default in TLS 1.3.
Change-Id: I3584947909d3d6988b940f9404044cace265b20d
Reviewed-on: https://boringssl-review.googlesource.com/8695
Reviewed-by: David Benjamin <davidben@google.com>
This reverts commits:
8d79ed674019fdcb52348d79ed6740
Because WebRTC (at least) includes our headers in an extern "C" block,
which precludes having any C++ in them.
Change-Id: Ia849f43795a40034cbd45b22ea680b51aab28b2d
Last month's canary for loop did not die in the coal mine of decrepit
toolchains. Make a note of this in STYLE.md so we know to start breeding
more of them. We can indeed declare index variables like it's 1999.
I haven't bothered to convert all of our for loops because that will be
tedious, but we can do it as we touch the code. Or if someone feels
really really bored.
BUG=47
Change-Id: Ib76c0767c1b509e825eac66f8c2e3ee2134e2493
Reviewed-on: https://boringssl-review.googlesource.com/8740
Reviewed-by: Adam Langley <agl@google.com>
This change scatters the contents of the two scoped_types.h files into
the headers for each of the areas of the code. The types are now in the
|bssl| namespace.
Change-Id: I802b8de68fba4786b6a0ac1bacd11d81d5842423
Reviewed-on: https://boringssl-review.googlesource.com/8731
Reviewed-by: Adam Langley <agl@google.com>
We currently have the situation where the |tool| and |bssl_shim| code
includes scoped_types.h from crypto/test and ssl/test. That's weird and
shouldn't happen. Also, our C++ consumers might quite like to have
access to the scoped types.
Thus this change moves some of the template code to base.h and puts it
all in a |bssl| namespace to prepare for scattering these types into
their respective headers. In order that all the existing test code be
able to access these types, it's all moved into the same namespace.
Change-Id: I3207e29474dc5fcc344ace43119df26dae04eabb
Reviewed-on: https://boringssl-review.googlesource.com/8730
Reviewed-by: David Benjamin <davidben@google.com>
We usually put main at the end. There's now nothing interesting in the
function, so avoid having to declare every test at the top.
Change-Id: Iac469f41f0fb7d1f58d12dfbf651bf0d39f073d0
Reviewed-on: https://boringssl-review.googlesource.com/8712
Reviewed-by: David Benjamin <davidben@google.com>
That removes the last of the bc stuff.
BUG=31
Change-Id: If64c974b75c36daf14c46f07b0d9355b7cd0adcb
Reviewed-on: https://boringssl-review.googlesource.com/8711
Reviewed-by: David Benjamin <davidben@google.com>
Amazingly, this function actually has (not crypto-related) callers, despite
being pretty much useless for cryptography.
BUG=31
Change-Id: I440827380995695c7a15bbf2220a05ffb28d9335
Reviewed-on: https://boringssl-review.googlesource.com/8594
Reviewed-by: David Benjamin <davidben@google.com>
These were generated by running test_mod_exp_mont5 10 times. The values with
Montgomery representation 1 were generated separately so the test file could
preserve the comment. (Though, at 10,000 lines, no one's going to find it...)
BUG=31
Change-Id: I8e9d4d6d7b5f7d283bd259df10a1dbdc90b888cf
Reviewed-on: https://boringssl-review.googlesource.com/8611
Reviewed-by: David Benjamin <davidben@google.com>
Honestly, with this size of number, they're pretty bad test vectors.
test_mod_exp_mont5 will be imported in the next commit which should help.
This was done by taking test_mod_exp's generation, running it a few times
(since otherwise the modulus is always the same). I also ran it a few times
with the odd constraint removed since BN_mod_exp is supposed to support it,
even if it's not actually useful.
BUG=31
Change-Id: Id53953f0544123a5ea71efac534946055dd5aabc
Reviewed-on: https://boringssl-review.googlesource.com/8610
Reviewed-by: David Benjamin <davidben@google.com>
That one needs reduced inputs and the other ought to be also tested against
unreduced ones is a bit annoying. But the previous commit made sure BN_nnmod
has tests, and test_mont could stand to inherit test_mod_mul's test data (it
only had five tests originally!), so I merged them.
BUG=31
Change-Id: I1eb585b14f85f0ea01ee81537a01e07ced9f5d9a
Reviewed-on: https://boringssl-review.googlesource.com/8608
Reviewed-by: David Benjamin <davidben@google.com>
In preparation for converting test_mont and test_mod_mul to test vectors, make
test_mont less silly. We can certainly get away with doing more than five
tests. Also generate |a| and |b| anew each time. Otherwise the first BN_nmod is
destructive.
Change-Id: I944007ed7b6013a16d972cb7290ab9992c9360ce
Reviewed-on: https://boringssl-review.googlesource.com/8605
Reviewed-by: David Benjamin <davidben@google.com>
No need for the special case and such.
Change-Id: If8fbc73eda0ccbaf3fd422e97c96fee6dc10b1ab
Reviewed-on: https://boringssl-review.googlesource.com/8604
Reviewed-by: David Benjamin <davidben@google.com>
Since the format no longer is readable by bc, compare it to Go's math/big
instead.
Change-Id: I34d37aa0c29c6f4178267858cb0d3941b4266b93
Reviewed-on: https://boringssl-review.googlesource.com/8603
Reviewed-by: David Benjamin <davidben@google.com>
Also, update the documentation about aliasing for |BN_usub|. It might
be better to find a way to factor out the shared logic between the
tests of these functions and the tests of |BN_add| and |BN_usub|, but
doing so would end up up creating a lot of parameters due to the many
distinct strings used in the messages.
Change-Id: Ic9d714858212fc92aa6bbcc3959576fe6bbf58c3
Reviewed-on: https://boringssl-review.googlesource.com/8593
Reviewed-by: David Benjamin <davidben@google.com>
Also update the documentation for |BN_sub|.
Change-Id: I544dbfc56f22844f6ca08e9e472ec13e76baf8c4
Reviewed-on: https://boringssl-review.googlesource.com/8592
Reviewed-by: David Benjamin <davidben@google.com>
|BN_mod_exp_mont| should be tested the same way as the other variants,
especially since it is exported.
Change-Id: I8c05725289c0ebcce7aba7e666915c4c1a841c2b
Reviewed-on: https://boringssl-review.googlesource.com/8590
Reviewed-by: David Benjamin <davidben@google.com>
In order to delay the digest of the handshake transcript and unify
around message-based signing callbacks, a copy of the transcript is kept
around until we are sure there is no certificate authentication.
This removes support for SSL_PRIVATE_KEY_METHOD as a client in SSL 3.0.
Change-Id: If8999a19ca021b4ff439319ab91e2cd2103caa64
Reviewed-on: https://boringssl-review.googlesource.com/8561
Reviewed-by: David Benjamin <davidben@google.com>
This file contains nothing but no-op functions. There's nothing to include.
Change-Id: I3a21207d6a47fab3a00c3f72011abef850ed7b27
Reviewed-on: https://boringssl-review.googlesource.com/8541
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The bc ones will all get replaced later.
Change-Id: Ic1c6ee320b3a5689c7dadea3f483bd92f7e39612
Reviewed-on: https://boringssl-review.googlesource.com/8528
Reviewed-by: Adam Langley <agl@google.com>
These can all share one test type. Note test_div had a separate
division by zero test which had to be extracted.
BUG=31
Change-Id: I1de0220fba78cd7f82a5dc96adb34b79c07929e9
Reviewed-on: https://boringssl-review.googlesource.com/8527
Reviewed-by: Adam Langley <agl@google.com>
crypto/bn/bn_test.cc:404:44: error: ‘n’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
Change-Id: Id590dfee4b9ae1a4fbd0965e133310dac0d06ed3
Two of these were even regression tests for a past bug. These are also
moved to the file, now with the amazing innovation that we *actually
check the regression test gave the right answer*.
BUG=31
Change-Id: I8097336ad39a2bb5c0af07dd8e1e34723b68d182
Reviewed-on: https://boringssl-review.googlesource.com/8525
Reviewed-by: Adam Langley <agl@google.com>
This adds tests for:
for i = 0 to 199:
Sum: 2^i
A: 2^i - 1
B: 1
for i = 0 to 199:
Sum: 2^200
A: 2^200 - 2^i
B: 2^i
I don't believe any of the existing tests actually stressed this,
amazingly enough.
Change-Id: I5edab6327bad45fc21c62bd47f4169f8bb745ff7
Reviewed-on: https://boringssl-review.googlesource.com/8523
Reviewed-by: Adam Langley <agl@google.com>
This took some finesse. I merged the lshift1 and rshift1 test vectors as
one counted down and the other up. The rshift1 vectors were all rounded
to even numbers, with the test handling the odd case. Finally, each run
only tested positive or negative (it wasn't re-randomized), so I added
both positive and negative versions of each test vector.
BUG=31
Change-Id: Ic7de45ab797074547c44c2e4ff8089b1feec5d57
Reviewed-on: https://boringssl-review.googlesource.com/8522
Reviewed-by: Adam Langley <agl@google.com>
MSVC 2015 seems to support it just fine.
Change-Id: I9c91c18c260031e6024480d1f57bbb334ed7118c
Reviewed-on: https://boringssl-review.googlesource.com/8501
Reviewed-by: Adam Langley <agl@google.com>
Test vectors taken from one run of bc_test with the -bc flag, along with
a handful of manual test vectors around numbers close to zero. (The
output was compared against bc to make sure it was correct.)
BUG=31
Change-Id: I9e9263ece64a877c8497716cd4713b4c3e44248c
Reviewed-on: https://boringssl-review.googlesource.com/8521
Reviewed-by: Adam Langley <agl@google.com>
Upstream added new instructions in
f4d456408d9d7bca31f34765d1a05fbd9fa55826 and
4e3d2866b6e8e7a700ea22e05840a093bfd7a4b1.
Change-Id: I835650426a0dffca2d8686d64aef99097a4bd186
Reviewed-on: https://boringssl-review.googlesource.com/8520
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 67b8bf4d849a7c40d0226de4ebe2590c4cc7c1f7.)
Verified a no-op in generate_build_files.py.
Change-Id: I09648893ab5c795f3934da0b2ecbc5fd7eb068d5
Reviewed-on: https://boringssl-review.googlesource.com/8519
Reviewed-by: Adam Langley <agl@google.com>
Depending on architecture, perlasm differed on which one or both of:
perl foo.pl flavor output.S
perl foo.pl flavor > output.S
Upstream has now unified on the first form after making a number of
changes to their files (the second does not even work for their x86
files anymore). Sync those portions of our perlasm scripts with upstream
and update CMakeLists.txt and generate_build_files.py per the new
convention.
This imports various commits like this one:
184bc45f683c76531d7e065b6553ca9086564576 (this was done by taking a
diff, so I don't have the full list)
Confirmed that generate_build_files.py sees no change.
BUG=14
Change-Id: Id2fb5b8bc2a7369d077221b5df9a6947d41f50d2
Reviewed-on: https://boringssl-review.googlesource.com/8518
Reviewed-by: Adam Langley <agl@google.com>
We're not using the masm output (and upstream does not even support it).
Reduce unnecessary diff from upstream.
Change-Id: Ic0b0f804bd7ec1429b3b1f40746297b57dcfcef6
Reviewed-on: https://boringssl-review.googlesource.com/8517
Reviewed-by: Adam Langley <agl@google.com>
It was missing. Writing NewSessionTicket will need it.
Change-Id: I39de237894f2e8356bd6861da2b8a4d805dcd2d6
Reviewed-on: https://boringssl-review.googlesource.com/8439
Reviewed-by: Adam Langley <agl@google.com>
This also adds a missing check to the C half to ensure fake record types are
always correct, to keep implementations honest.
Change-Id: I1d65272e647ffa67018c721d52c639f8ba47d647
Reviewed-on: https://boringssl-review.googlesource.com/8510
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
As of 67cb49d045 and the corresponding upstream
change, BN_mod_word may fail, like BN_div_word. Handle this properly and
document in bn.h. Thanks to Brian Smith for pointing this out.
Change-Id: I6d4f32dc37bcabf70847c9a8b417d55d31b3a380
Reviewed-on: https://boringssl-review.googlesource.com/8491
Reviewed-by: Adam Langley <agl@google.com>
This function returns a tri-state -1 on error. We should check this.
Change-Id: I6fe130c11d10690923aac5ac7a6dfe3e3ff3f5e9
Reviewed-on: https://boringssl-review.googlesource.com/8490
Reviewed-by: Adam Langley <agl@google.com>
It was already nearly clean. Just one undeclared variable.
(Imported from upstream's abeae4d3251181f1cedd15e4433e79406b766155.)
Change-Id: I3b8f20034f914fc44faabf165d1553d4084c87cc
Reviewed-on: https://boringssl-review.googlesource.com/8393
Reviewed-by: Adam Langley <agl@google.com>
This functionally pulls in a number of changes from upstream, including:
4e3d2866b6e8e7a700ea22e05840a093bfd7a4b1
1eb12c437bbeb2c748291bcd23733d4a59d5d1ca
6a4ea0022c475bbc2c7ad98a6f05f6e2e850575b
c25278db8e4c21772a0cd81f7873e767cbc6d219
e0a651945cb5a70a2abd9902c0fd3e9759d35867
d405aa2ff265965c71ce7331cf0e49d634a06924
ce3d25d3e5a7e82fd59fd30dff7acc39baed8b5e
9ba96fbb2523cb12747c559c704c58bd8f9e7982
Notably, c25278db8e4c21772a0cd81f7873e767cbc6d219 makes it enable 'use strict'.
To avoid having to deal with complex conflicts, this was done by taking a diff
of our copy of the file with the point just before
c25278db8e4c21772a0cd81f7873e767cbc6d219, and reapplying the non-reverting
parts of our diff on top of upstream's current version.
Confirmed with generate_build_files.py that this makes no changes *except*
d405aa2ff265965c71ce7331cf0e49d634a06924 causes this sort of change throughout
chacha-x86_64.pl's nasm output:
@@ -1179,7 +1179,7 @@ $L$oop8x:
vpslld ymm14,ymm0,12
vpsrld ymm0,ymm0,20
vpor ymm0,ymm14,ymm0
- vbroadcasti128 ymm14,YMMWORD[r11]
+ vbroadcasti128 ymm14,XMMWORD[r11]
vpaddd ymm13,ymm13,ymm5
vpxor ymm1,ymm13,ymm1
vpslld ymm15,ymm1,12
This appears to be correct. vbroadcasti128 takes a 128-bit-wide second
argument, so it wants XMMWORD, not YMMWORD. I suppose nasm just didn't care.
(Looking at a diff-diff may be a more useful way to review this CL.)
Change-Id: I61be0d225ddf13b5f05d1369ddda84b2f322ef9d
Reviewed-on: https://boringssl-review.googlesource.com/8392
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
In order to ensure that we don't randomly interoperate with
implementations that don't mask scalars correctly, always generate
scalars with the wrong fixed bits.
Change-Id: I82536a856f034cfe4464fc545a99c21b3cff1691
Reviewed-on: https://boringssl-review.googlesource.com/8391
Reviewed-by: David Benjamin <davidben@google.com>
It's always one. We don't support other kinds of curves with this framework.
(Curve25519 uses a much simpler API.) This also allows us to remove the
check_pub_key_order logic.
Change-Id: Ic15e1ecd68662b838c76b1e0aa15c3a93200d744
Reviewed-on: https://boringssl-review.googlesource.com/8350
Reviewed-by: Adam Langley <agl@google.com>
DSA is deprecated, but get this aligned with some of the BN_FLG_CONSTTIME work
going on elsewhere.
Change-Id: I676ceab298a69362bef1b61d6f597c5c90da2ff0
Reviewed-on: https://boringssl-review.googlesource.com/8309
Reviewed-by: Adam Langley <agl@google.com>
It's a prime, so computing a constant-time mod inverse is straight-forward.
Change-Id: Ie09b84363c3d5da827989300a844c470437fd8f2
Reviewed-on: https://boringssl-review.googlesource.com/8308
Reviewed-by: Adam Langley <agl@google.com>
The case where ec_group_get_mont_data is NULL is only for arbitrary groups
which we now require to be prime order. BN_mod_exp_mont is fine with a NULL
BN_MONT_CTX. It will just compute it. Saves a bit of special-casing.
Also don't mark p-2 as BN_FLG_CONSTTIME as the exponent is public anyway.
Change-Id: Ie868576d52fc9ae5f5c9f2a4039a729151bf84c7
Reviewed-on: https://boringssl-review.googlesource.com/8307
Reviewed-by: Adam Langley <agl@google.com>
The Conscrypt revert cycled in long ago.
Change-Id: If3cdb211d7347dca88bd70bdc643f80b19a7e528
Reviewed-on: https://boringssl-review.googlesource.com/8306
Reviewed-by: Adam Langley <agl@google.com>
Make |BN_mod_inverse_ex| symmetric with |BN_mod_inverse_no_branch| in
this respect.
Change-Id: I4a5cbe685edf50e13ee1014391bc4001f5371fec
Reviewed-on: https://boringssl-review.googlesource.com/8316
Reviewed-by: David Benjamin <davidben@google.com>
The alignas in NEWPOLY_POLY told the compiler that it could assume a
certain alignment. However, values were allocated with malloc with no
specific alignment.
We could try and allocate aligned memory but the alignment doesn't have
a performance impact (on x86-64) so this is the simpler change. (Also,
Windows doesn't have |posix_memalign|. The cloest thing is
_alligned_alloc but then one has to use a special free function.)
Change-Id: I53955a88862160c02aa5436d991b1b797c3c17db
Reviewed-on: https://boringssl-review.googlesource.com/8315
Reviewed-by: David Benjamin <davidben@google.com>
There's no use doing the remaining work if we're going to fail due to
there being no inverse.
Change-Id: Ic6d7c92cbbc2f7c40c51e6be2de3802980d32543
Reviewed-on: https://boringssl-review.googlesource.com/8310
Reviewed-by: David Benjamin <davidben@google.com>
This ensures that the test is not flaky after lots of iterations.
Along the way, change newhope_test.cc to C++.
Change-Id: I4ef139444b8c8a98db53d075105eb6806f6c5fc7
Reviewed-on: https://boringssl-review.googlesource.com/8110
Reviewed-by: Adam Langley <agl@google.com>
(This change will be sent upstream. Since the legacy X.509 stack is just
kept around for compatibility, if they decide to fix it in a different
way, we may wish to revert this and apply their fix.)
Dating back to SSLeay, X509_LOOKUP_METHOD had this X509_LU_RETRY
machinery. But it's not documented and it appears to have never worked.
Problems with the existing logic:
- X509_LU_* is not sure whether it is a type enum (to be passed into
X509_LOOKUP_by_*) or a return enum (to be retained by those same
functions).
- X509_LOOKUP_by_* is not sure whether it returns 0/1 or an X509_LU_*
value. Looking at the functions themselves, one might think it's the
latter, but for X509_LOOKUP_by_subject returning both 0 and
X509_LU_FAIL. But looking at the call sites, some expect 0/1 (such as
X509_STORE_get1_certs) while others expect an X509_LU_* enum (such as
X509_STORE_CTX_get1_issuer). It is very fortunate that FAIL happens to
be 0 and X509 happens to be 1.
These functions primarily call to X509_LOOKUP_METHOD hooks. Looking
through OpenSSL itself and code checked into Google, I found no
evidence that any hooks have been implemented except for
get_by_subject in by_dir.c. We take that one as definitive and observe
it believes it returns 0/1. Notably, it returns 1 on success even if
asked for a type other than X509_LU_X509. (X509_LU_X509 = 1. Others are
different.) I found another piece of third-party software which corroborates
this worldview.
- X509_STORE_get_by_subject's handling of X509_LU_RETRY (it's the j < 0
check) is broken. It saves j into vs->current_method where it probably
meant to save i. (This bug has existed since SSLeay.)
It also returns j (supposedly X509_LU_RETRY) while all callers of
X509_STORE_get_by_subject expect it to return 0/1 by checking with !
instead of <= 0. (Note that all other codepaths return 0 and 1 so this
function did not actually believe it returned X509_LU_* most of the
time.)
This, in turn, gives us a free of uninitialized pointers in
X509_STORE_get1_certs and other functions which expect that *ret is
filled in if X509_STORE_get_by_subject returns success. GCC 4.9 with
optimizations from the Android NDK noticed this, which trigged this
saga.
(It's only reachable if any X509_LOOKUP_METHOD returned
X509_LU_RETRY.)
- Although the code which expects X509_STORE_get_by_subject return 0/1
does not date to SSLeay, the X509_STORE_get_by_subject call in
X509_STORE_CTX_get1_issuer *does* (though, at the time, it was inline
in X509_verify_cert. That code believes X509_STORE_get_by_subject
returns an X509_LU_* enum, but it doesn't work either! It believes
*ret is filled in on X509_LU_RETRY, thus freeing another uninitialized
pointer (GCC noticed this too).
Since this "retry" code has clearly never worked, from SSLeay onwards,
unwind it completely rather than attempt to fix it. No
X509_LOOKUP_METHOD can possibly have depended on it.
Matching all non-broken codepaths X509_LOOKUP_by_* now returns 0/1 and
X509_STORE_get_by_subject returns 0/1. X509_LU_* is purely a type enum
with X509_LU_{REJECT,FAIL} being legacy constants to keep old code
compiling. (Upstream is recommended to remove those values altogether
for 1.1.0.)
On the off chance any get_by_* X509_LOOKUP_METHOD implementations did
not return 0/1 (I have found no evidence anywhere of this, and I believe
it wouldn't have worked anyway), the X509_LOOKUP_by_* wrapper functions
will coerce the return values back to 0/1 before passing up to the
callers which want 0/1. This both avoids the error-prone -1/0/1 calling
convention and, more importantly, avoids problems with third-party
callers which expect a X509_LU_* return code. 0/1 collide with FAIL/X509
while -1 will collide with RETRY and might confuse things.
Change-Id: I98ecf6fa7342866b9124dc6f0b422cb9ce4a1ae7
Reviewed-on: https://boringssl-review.googlesource.com/8303
Reviewed-by: Adam Langley <agl@google.com>
I did the same change in NaCl in
https://codereview.chromium.org/2070533002/. I thought NaCl is the only
place where this was needed, but at least it's due to SecureZeroMemory()
again. So it's two files now, but at least there's only one function we
know of that needs this, and it's only called in three files total in
all projects used by Chromium.
BUG=chromium:592745
Change-Id: I07ed197869e26ec70c1f4b75d91fd64abae5015e
Reviewed-on: https://boringssl-review.googlesource.com/8320
Reviewed-by: David Benjamin <davidben@google.com>
I named the compatibility function wrong.
Change-Id: Idc289c317c5826c338c1daf58a2d3b26b09a7e49
Reviewed-on: https://boringssl-review.googlesource.com/8301
Reviewed-by: Adam Langley <agl@google.com>
These are more remnants of CMS. Nothing uses them directly. Removing them means
more code we don't have to think about when importing upstream patches.
Also take out a bunch of dead prototypes nearby.
Change-Id: Ife094d9d2078570006d1355fa4e3323f435be608
Reviewed-on: https://boringssl-review.googlesource.com/8244
Reviewed-by: David Benjamin <davidben@google.com>
These are more pretty-printers for generic ASN.1 structures. They're never
called externally and otherwise are only used in the X509V3_EXT_PARSE_UNKNOWN
mode for the X509 pretty-print functions. That makes unknown extensions
pretty-print as ASN.1 structures.
This is a rather useless feature, so have that fall through to
X509V3_EXT_DUMP_UNKNOWN which does a hexdump instead.
(The immediate trigger is I don't know what |op| is in upstream's
8c918b7b9c93ba38790ffd1a83e23c3684e66f57 and don't think it is worth the time
to puzzle that out and verify it. Better ditch this code completely.)
Change-Id: I0217906367d83056030aea64ef344d4fedf74763
Reviewed-on: https://boringssl-review.googlesource.com/8243
Reviewed-by: David Benjamin <davidben@google.com>
These functions are never instantiated. (They're a remnant of the PKCS#7 and
CMS bits.) Next time upstream touches this code, we don't have to puzzle
through the diff and import it.
Change-Id: I67c2102ae13e8e0527d858e1c63637dd442a4ffb
Reviewed-on: https://boringssl-review.googlesource.com/8242
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 3892b95750b6aa5ed4328a287068f7cdfb9e55bc.)
More reasonable would have been to drop |to| altogether and act on from[len-1],
but I suppose this works.
Change-Id: I280b4991042b4d330ba034f6a631f8421ddb2643
Reviewed-on: https://boringssl-review.googlesource.com/8241
Reviewed-by: Adam Langley <agl@google.com>
There's a __pragma expression which allows this. Android builds us Windows with
MinGW for some reason, so we actually do have to tolerate non-MSVC-compatible
Windows compilers. (Clang for Windows is much more sensible than MinGW and
intentionally mimicks MSVC.)
MinGW doesn't understand MSVC's pragmas and warns a lot. #pragma warning is
safe to suppress, so wrap those to shush them. This also lets us do away with a
few ifdefs.
Change-Id: I1f5a8bec4940d4b2d947c4c1cc9341bc15ec4972
Reviewed-on: https://boringssl-review.googlesource.com/8236
Reviewed-by: Adam Langley <agl@google.com>
Switch one for loop to the new spelling as a canary. All our compilers seem to
support it fine, except GCC needs to be told to build with -std=c99. (And, upon
doing so, it'll require _XOPEN_SOURCE=700 for pthread_rwlock_t.)
We'll let this sit for a bit until it's gotten into downstreams without issue
and then open the floodgates.
BUG=47
Change-Id: I1c69d4b2df8206e0b55f30aa59b5874d82fca893
Reviewed-on: https://boringssl-review.googlesource.com/8235
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit 762e1d039c. We no longer need
to support out < in. Better to keep the assembly aligned with upstream.
Change-Id: I345bf822953bd0e1e79ad5ab4d337dcb22e7676b
Reviewed-on: https://boringssl-review.googlesource.com/8232
Reviewed-by: Adam Langley <agl@google.com>
While most of OpenSSL's assembly allows out < in too, some of it doesn't.
Upstream seems to not consider this a problem (or, at least, they're failing to
make a decision on whether it is a problem, so we should assume they'll stay
their course). Accordingly, require aliased buffers to exactly align so we
don't have to keep chasing this down.
Change-Id: I00eb3df3e195b249116c68f7272442918d7077eb
Reviewed-on: https://boringssl-review.googlesource.com/8231
Reviewed-by: Adam Langley <agl@google.com>
Decrypting is very easy to do in-place, but encrypting in-place is a hassle.
The rules actually were wrong due to record-splitting. The aliasing prefix and
the alignment prefix actually differ by 1. Take it out for now in preparation
for tightening the aliasing rules.
If we decide to do in-place encrypt later, probably it'd be more useful to
return header + in-place ciphertext + trailer. (That, in turn, needs a
scatter/gather thing on the AEAD thanks to TLS 1.3's padding and record type
construction.) We may also wish to rethink how record-splitting works here.
Change-Id: I0187d39c541e76ef933b7c2c193323164fd8a156
Reviewed-on: https://boringssl-review.googlesource.com/8230
Reviewed-by: Adam Langley <agl@google.com>
On systems where we do not have BN_ULLONG (notably Win64), BN_mod_word() can
return incorrect results if the supplied modulus is too big.
(Imported from upstream's e82fd1b4574c8908b2c3bb68e1237f057a981820 and
e4c4b2766bb97b34ea3479252276ab7c66311809.)
Change-Id: Icee8a7c5c67a8ee14c276097f43a7c491e68c2f9
Reviewed-on: https://boringssl-review.googlesource.com/8233
Reviewed-by: Adam Langley <agl@google.com>
Set ctx->error = X509_V_ERR_OUT_OF_MEM when verification cannot
continue due to malloc failure. Similarly for issuer lookup failures
and caller errors (bad parameters or invalid state).
Also, when X509_verify_cert() returns <= 0 make sure that the
verification status does not remain X509_V_OK, as a last resort set
it it to X509_V_ERR_UNSPECIFIED, just in case some code path returns
an error without setting an appropriate value of ctx->error.
Add new and some missing error codes to X509 error -> SSL alert switch.
(Imported from upstream's 5553a12735e11bc9aa28727afe721e7236788aab.)
Change-Id: I3231a6b2e72a3914cb9316b8e90ebaee009a1c5f
Reviewed-on: https://boringssl-review.googlesource.com/8170
Reviewed-by: David Benjamin <davidben@google.com>
x25519-x86_64.c, like the rest of crypto/curve25519, is descended from
SUPERCOP. Add the usual copyright header along with the SUPERCOP attribution.
BUG=64
Change-Id: I43f3de0731f33ab2aa48492c4b742e9f23c87fe1
Reviewed-on: https://boringssl-review.googlesource.com/8195
Reviewed-by: Adam Langley <agl@google.com>
The business with ssl_record_prefix_len is rather a hassle. Instead, have
tls_open_record always decrypt in-place and give back a CBS to where the body
is.
This way the caller doesn't need to do an extra check all to avoid creating an
invalid pointer and underflow in subtraction.
Change-Id: I4e12b25a760870d8f8a503673ab00a2d774fc9ee
Reviewed-on: https://boringssl-review.googlesource.com/8173
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's f792c663048f19347a1bb72125e535e4fb2ecf39.)
Change-Id: If9bbb10de3ea858076bd9587d21ec331e837dd53
Reviewed-on: https://boringssl-review.googlesource.com/8171
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Operations in the DSA signing algorithm should run in constant time in
order to avoid side channel attacks. A flaw in the OpenSSL DSA
implementation means that a non-constant time codepath is followed for
certain operations. This has been demonstrated through a cache-timing
attack to be sufficient for an attacker to recover the private DSA key.
CVE-2016-2178
(Imported from upstream's 621eaf49a289bfac26d4cbcdb7396e796784c534 and
b7d0f2834e139a20560d64c73e2565e93715ce2b.)
We should eventually not depend on BN_FLG_CONSTTIME since it's a mess (seeing
as the original fix was wrong until we reported b7d0f2834e to them), but, for
now, go with the simplest fix.
Change-Id: I9ea15c1d1cc3a7e21ef5b591e1879ec97a179718
Reviewed-on: https://boringssl-review.googlesource.com/8172
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
More spring-cleaning of unnecessary incompatibilities. Since
OpenSSL_add_all_algorithms_conf doesn't specify a configuration file, it's
perfectly sound to have such a function.
Dear BoringSSL, please add all algorithms.
Uh, sure. They were already all there, but I have added them!
PS: Could you also load all your configuration files while you're at it.
...I don't have any. Fine. I have loaded all configuration files which I
recognize. *mutters under breath* why does everyone ask all these strange
questions...
Change-Id: I57f956933d9e519445bf22f89853bd5f56904172
Reviewed-on: https://boringssl-review.googlesource.com/8160
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Some files were named 𝑥_test.txt and some 𝑥_tests.txt. This change
unifies around the latter.
Change-Id: Id6f29bad8b998f3c3466655097ef593f7f18f82f
Reviewed-on: https://boringssl-review.googlesource.com/8150
Reviewed-by: David Benjamin <davidben@google.com>
Some OpenSSL consumers use them, so provide no-op versions to make porting code
easier.
Change-Id: I4348568c1cb08d2b2c0a9ec9a17e2c0449260965
Reviewed-on: https://boringssl-review.googlesource.com/8142
Reviewed-by: David Benjamin <davidben@google.com>
Make building against software that expects OpenSSL easier.
Change-Id: I1af090ae8208218d6e226ee0baf51053699d85cc
Reviewed-on: https://boringssl-review.googlesource.com/8141
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Windows is, not unreasonably, complaining that taking abs() of an unsigned is
ridiculous. But these values actually are signed and fit very easily in an int
anyway.
Change-Id: I34fecaaa3616732112e3eea105a7c84bd9cd0bae
Reviewed-on: https://boringssl-review.googlesource.com/8144
Reviewed-by: Adam Langley <agl@google.com>
Otherwise builds fail with:
crypto/newhope/newhope_statistical_test.cc:136:27: error: format specifies type 'long' but the argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
Change-Id: I85d5816c1d7ee71eef362bffe983b2781ce310a4
One of these tests the distribution of noise polynomials; the other
tests that that agreed-upon keys (prior to whitening) have roughly equal
numbers of 0s and 1s.
Along the way, expose a few more API bits.
Change-Id: I6b04708d41590de45d82ea95bae1033cfccd5d67
Reviewed-on: https://boringssl-review.googlesource.com/8130
Reviewed-by: Adam Langley <agl@google.com>
This commit adds coverage of the "offer" (first) step, as well as
testing all outputs of the "accept" (second) step, not just the shared
key.
Change-Id: Id11fe24029abc302442484a6c01fa496a1578b3a
Reviewed-on: https://boringssl-review.googlesource.com/8100
Reviewed-by: Adam Langley <agl@google.com>
The test vectors are taken from the reference implementation, modified
to output the results of its random-number generator, and the results of
key generation prior to SHA3. This allows the interoperability of the
two implementations to be tested somewhat.
To accomplish the testing, this commit creates a new, lower-level API
that leaves the generation of random numbers and all wire encoding and
decoding up to the caller.
Change-Id: Ifae3517696dde4be4a0b7c1998bdefb789bac599
Reviewed-on: https://boringssl-review.googlesource.com/8070
Reviewed-by: Adam Langley <agl@google.com>
Now that we no longer support Windows XP, this is available.
Unfortunately, the public header version of CRYPTO_MUTEX means we
still can't easily merge CRYPTO_MUTEX and CRYPTO_STATIC_MUTEX.
BUG=37
Change-Id: If309de3f06e0854c505083b72fd64d1dbb3f4563
Reviewed-on: https://boringssl-review.googlesource.com/8081
Reviewed-by: Adam Langley <agl@google.com>
Windows SRWLOCK requires you call different functions here. Split
them up in preparation for switching Windows from CRITICAL_SECTION.
BUG=37
Change-Id: I7b5c6a98eab9ae5bb0734b805cfa1ff334918f35
Reviewed-on: https://boringssl-review.googlesource.com/8080
Reviewed-by: Adam Langley <agl@google.com>
This code has caused a long history of problems. This change rewrites it
completely with something that is, hopefully, much simplier and robust
and adds more testing.
Change-Id: Ibeef51f9386afd95d5b73316e451eb3a2d7ec4e0
Reviewed-on: https://boringssl-review.googlesource.com/8033
Reviewed-by: Adam Langley <agl@google.com>
The key schedule in TLS 1.3 requires a separate Extract and Expand phase
for the cryptographic computations.
Change-Id: Ifdac1237bda5212de5d4f7e8db54e202151d45ec
Reviewed-on: https://boringssl-review.googlesource.com/7983
Reviewed-by: David Benjamin <davidben@google.com>
CECPQ1 is a new key exchange that concatenates the results of an X25519
key agreement and a NEWHOPE key agreement.
Change-Id: Ib919bdc2e1f30f28bf80c4c18f6558017ea386bb
Reviewed-on: https://boringssl-review.googlesource.com/7962
Reviewed-by: David Benjamin <davidben@google.com>
Use of strdup, close, lseek, read, and write prevent linking
statically againt libcmt.lib.
Change-Id: I04f7876ec0f03f29f000bbcc6b2ccdec844452d2
Reviewed-on: https://boringssl-review.googlesource.com/8010
Reviewed-by: David Benjamin <davidben@google.com>
This is consistent with the new convention in ssl_ecdh.c.
Along the way, change newhope_test.c to not iterate 1000 times over each
test.
Change-Id: I7a500f45b838eba8f6df96957891aa8e880ba089
Reviewed-on: https://boringssl-review.googlesource.com/8012
Reviewed-by: David Benjamin <davidben@google.com>
BUF_MEM is actually a rather silly API for the SSL stack. There's separate
length and max fields, but init_buf effectively treats length as max and max as
nothing.
We possibly don't want to be using it long-term anyway (if nothing else, the
char*/uint8_t* thing is irritating), but in the meantime, it'll be easier to
separately fix up get_message's book-keeping and state tracking from where the
handshake gets its messages from.
Change-Id: I9e56ea008173991edc8312ec707505ead410a9ee
Reviewed-on: https://boringssl-review.googlesource.com/7947
Reviewed-by: Adam Langley <agl@google.com>
This function will return whether BoringSSL was built with
OPENSSL_NO_ASM. This will allow us to write a test in our internal
codebase which asserts that normal builds should always have assembly
code included.
Change-Id: Ib226bf63199022f0039d590edd50c0cc823927b9
Reviewed-on: https://boringssl-review.googlesource.com/7960
Reviewed-by: David Benjamin <davidben@google.com>
The performance measurements seem to be very out-of-date. Also, the
idea for optimizing the case of an even modulus is interesting, but it
isn't useful because we never use an even modulus.
Change-Id: I012eb37638cda3c63db0e390c8c728f65b949e54
Reviewed-on: https://boringssl-review.googlesource.com/7733
Reviewed-by: David Benjamin <davidben@google.com>
This function is only really useful for DSA signature verification,
which is something that isn't performance-sensitive. Replace its
optimized implementation with a naïve implementation that's much
simpler.
Note that it would be simpler to use |BN_mod_mul| in the new
implementation; |BN_mod_mul_montgomery| is used instead only to be
consistent with other work being done to replace uses of non-Montgomery
modular reduction with Montgomery modular reduction.
Change-Id: If587d463b73dd997acfc5b7ada955398c99cc342
Reviewed-on: https://boringssl-review.googlesource.com/7732
Reviewed-by: David Benjamin <davidben@google.com>
sk_FOO_num may be called on const stacks. Given that was wrong, I suspect no
one ever uses a const STACK_OF(T)...
Other macros were correctly const, but were casting the constness a way (only
to have it come back again).
Also remove the extra newline after a group. It seems depending on which
version of clang-format was being used, we'd either lose or keep the extra
newline. The current file doesn't have them, so settle on that.
Change-Id: I19de6bc85b0a043d39c05ee3490321e9f0adec60
Reviewed-on: https://boringssl-review.googlesource.com/7946
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
When *pp is NULL, don't write garbage, return an unexpected pointer
or leak memory on error.
(Imported from upstream's 36c37944909496a123e2656ad1f651769a7cc72f.)
This calling convention...
Change-Id: Ic733092cfb942a3e1d3ceda6797222901ad55bef
Reviewed-on: https://boringssl-review.googlesource.com/7944
Reviewed-by: Adam Langley <agl@google.com>
|BN_mod_exp_mont_word| is only useful when the base is a single word
in length and timing side channel protection of the exponent is not
needed. That's never the case in real life.
Keep the function in the API, but removes its single-word-base
optimized implementation with a call to |BN_mod_exp_mont|.
Change-Id: Ic25f6d4f187210b681c6ee6b87038b64a5744958
Reviewed-on: https://boringssl-review.googlesource.com/7731
Reviewed-by: David Benjamin <davidben@google.com>
|BN_mod_exp_mont| will forward to |BN_mod_exp_mont_consttime|, so this
is a no-op semantically. However, this allows the linker to drop the
implementation of |BN_mod_exp_mont| even when the DH code is in use.
Change-Id: I0cb8b260224ed661ede74923bd134acb164459c1
Reviewed-on: https://boringssl-review.googlesource.com/7730
Reviewed-by: David Benjamin <davidben@google.com>
Also add a test.
This is the last of the openssl/asn1.h includes from the directories that are
to be kept in the core libcrypto library. (What remains is to finish sorting
out the crypto/obj stuff. We'll also want to retain a decoupled version of the
PKCS#12 stuff.)
Functions that need to be audited for reuse:
i2d_DHparams
BUG=54
Change-Id: Ibef030a98d3a93ae26e8e56869f14858ec75601b
Reviewed-on: https://boringssl-review.googlesource.com/7900
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Previously, the verification was only done when using the CRT method,
as the CRT method has been shown to be extremely sensitive to fault
attacks. However, there's no reason to avoid doing the verification
when the non-CRT method is used (performance-sensitive applications
should always be using the CRT-capable keys).
Previously, when we detected a fault (attack) through this verification,
libcrypto would fall back to the non-CRT method and assume that the
non-CRT method would give a correct result, despite having just
detecting corruption that is likely from an attack. Instead, just give
up, like NSS does.
Previously, the code tried to handle the case where the input was not
reduced mod rsa->n. This is (was) not possible, so avoid trying to
handle that. This simplifies the equality check and lets us use
|CRYPTO_memcmp|.
Change-Id: I78d1e55520a1c8c280cae2b7256e12ff6290507d
Reviewed-on: https://boringssl-review.googlesource.com/7582
Reviewed-by: David Benjamin <davidben@google.com>
Only treat an ASN1_ANY type as an integer if it has the V_ASN1_INTEGER
tag: V_ASN1_NEG_INTEGER is an internal only value which is never used
for on the wire encoding.
(Imported from upstream's d4b25980020821d4685752ecb9105c0902109ab5.)
This is redundant with our fb2c6f8c85 which I
think is a much better fix (having two notions of "type" depending on whether
we're in an ASN1_TYPE or an ASN1_STRING is fragile), so I think we should keep
our restriction too. Still, this is also worth doing.
Change-Id: I6ea54aae7b517a59c6e563d8c993d0ee22e25bee
Reviewed-on: https://boringssl-review.googlesource.com/7848
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's 172c6e1e14defe7d49d62f5fc9ea6a79b225424f, but note our
values have different types. In particular, because we put in_len in a size_t
and C implicitly requires that all valid buffers' lengths fit in a ptrdiff_t
(signed), the overflow was impossible, assuming EVP_ENCODE_CTX::length is
untouched externally.
More importantly, this function is stuck taking an int output and has no return
value, so the only plausible contract is the caller is responsible for ensuring
the length fits anyway. Indeed, callers all call EVP_EncodeUpdate in bounded
chunks, so upstream's analysis is off.
Anyway, in theory that logic could locally overflow, so tweak it slightly. Tidy
up some of the variable names while I'm here.
Change-Id: Ifa78707cc26c11e0d67019918a028531b3d6738c
Reviewed-on: https://boringssl-review.googlesource.com/7847
Reviewed-by: Adam Langley <agl@google.com>
This adds an explicit limit to the size of an X509_NAME structure. Some
part of OpenSSL (e.g. TLS) already effectively limit the size due to
restrictions on certificate size.
See also upstream's 65cb92f4da37a3895437f0c9940ee0bcf9f28c8a, although this is
different from upstream's. Upstream's version bounds both the X509_NAME *and*
any data after it in the immediately containing structure. While adding a bound
on all of crypto/asn1 is almost certainly a good idea (will look into that for
a follow-up), it seems bizarre and unnecessary to have X509_NAME affect its
parent.
Change-Id: Ica2136bcd1455d7c501ccc6ef2a19bc5ed042501
Reviewed-on: https://boringssl-review.googlesource.com/7846
Reviewed-by: Adam Langley <agl@google.com>
An overflow can occur in the EVP_EncryptUpdate function. If an attacker is
able to supply very large amounts of input data after a previous call to
EVP_EncryptUpdate with a partial block then a length check can overflow
resulting in a heap corruption.
Following an analysis of all OpenSSL internal usage of the
EVP_EncryptUpdate function all usage is one of two forms.
The first form is like this:
EVP_EncryptInit()
EVP_EncryptUpdate()
i.e. where the EVP_EncryptUpdate() call is known to be the first called
function after an EVP_EncryptInit(), and therefore that specific call
must be safe.
The second form is where the length passed to EVP_EncryptUpdate() can be seen
from the code to be some small value and therefore there is no possibility of
an overflow. [BoringSSL: We also have code that calls EVP_CIPHER functions by
way of the TLS/SSL3 "AEADs". However, there we know the inputs are bounded by
2^16.]
Since all instances are one of these two forms, I believe that there can
be no overflows in internal code due to this problem.
It should be noted that EVP_DecryptUpdate() can call EVP_EncryptUpdate()
in certain code paths. Also EVP_CipherUpdate() is a synonym for
EVP_EncryptUpdate(). Therefore I have checked all instances of these
calls too, and came to the same conclusion, i.e. there are no instances
in internal usage where an overflow could occur.
This could still represent a security issue for end user code that calls
this function directly.
CVE-2016-2106
Issue reported by Guido Vranken.
(Imported from upstream's 3ab937bc440371fbbe74318ce494ba95021f850a.)
Change-Id: Iabde896555c39899c7f0f6baf7a163a7b3c2f3d6
Reviewed-on: https://boringssl-review.googlesource.com/7845
Reviewed-by: Adam Langley <agl@google.com>
Upstream decided to reset *pp on error and to later fix up the other i2d
functions to behave similarly. See upstream's
c5e603ee182b40ede7713c6e229c15a8f3fdb58a.
Change-Id: I01f82b578464060d0f2be5460fe4c1b969124c8e
Reviewed-on: https://boringssl-review.googlesource.com/7844
Reviewed-by: Adam Langley <agl@google.com>
Sanity check field lengths and sums to avoid potential overflows and reject
excessively large X509_NAME structures.
Issue reported by Guido Vranken.
(Imported from upstream's 9b08619cb45e75541809b1154c90e1a00450e537.)
Change-Id: Ib2e1e7cd086f9c3f0d689d61947f8ec3e9220049
Reviewed-on: https://boringssl-review.googlesource.com/7842
Reviewed-by: Adam Langley <agl@google.com>
Reject zero length buffers passed to X509_NAME_oneline().
Issue reported by Guido Vranken.
(Imported from upstream's 66e731ab09f2c652d0e179df3df10d069b407604.)
Tweaked slightly to use <= 0 instead of == 0 since the length is signed.
Change-Id: I5ee54d77170845e4699fda7df5e94538c8e55ed9
Reviewed-on: https://boringssl-review.googlesource.com/7841
Reviewed-by: Adam Langley <agl@google.com>
The traditional private key encryption algorithm doesn't function
properly if the IV length of the cipher is zero. These ciphers
(e.g. ECB mode) are not suitable for private key encryption
anyway.
(Imported from upstream's 4436299296cc10c6d6611b066b4b73dc0bdae1a6.)
Change-Id: I218c9c1d11274ef11b7c0cfce380521efa415215
Reviewed-on: https://boringssl-review.googlesource.com/7840
Reviewed-by: Adam Langley <agl@google.com>
In the past we have needed the ability to deploy security fixes to our
frontend systems without leaking them in source code or in published
binaries.
This change adds a function that provides some infrastructure for
supporting this in BoringSSL while meeting our internal build needs. We
do not currently have any specific patch that requires this—this is
purely preparation.
Change-Id: I5c64839e86db4e5ea7419a38106d8f88b8e5987e
Reviewed-on: https://boringssl-review.googlesource.com/7849
Reviewed-by: David Benjamin <davidben@google.com>
BUG=43
Change-Id: I46ad1ca62b8921a03fae51f5d7bbe1c68fc0b170
Reviewed-on: https://boringssl-review.googlesource.com/7821
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The logic to reset *pp doesn't actually work if pp is NULL. (It also doesn't
work if *pp is NULL, but that didn't work before either.) Don't bother
resetting it. This is consistent with the template-based i2d functions which do
not appear to leave *pp alone on error.
Will send this upstream.
Change-Id: I9fb5753e5d36fc1d490535720b8aa6116de69a70
Reviewed-on: https://boringssl-review.googlesource.com/7812
Reviewed-by: Adam Langley <agl@google.com>
See upstream's 34b9acbd3f81b46967f692c0af49020c8c405746.
Change-Id: I88d5b3cfbbe87e883323a9e6e1bf85227ed9576e
Reviewed-on: https://boringssl-review.googlesource.com/7811
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
See also upstream's 91fb42ddbef7a88640d1a0f853c941c20df07de7, though that has a
bug if |out| was non-NULL on entry. (I'll send them a patch.)
Change-Id: I807f23007b89063c23e02dac11c4ffb41f847fdf
Reviewed-on: https://boringssl-review.googlesource.com/7810
Reviewed-by: David Benjamin <davidben@google.com>
GCC gets unhappy if we don't initialize the padding.
Change-Id: I084ffee1717d9025dcb10d8f32de0da2339c7f01
Reviewed-on: https://boringssl-review.googlesource.com/7797
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
If we're to allow the buggy CPU workaround to fire when __ARM_NEON__ is set,
CRYPTO_is_NEON_capable also needs to be aware of it. Also add an API to export
this value out of BoringSSL, so we can get some metrics on how prevalent this
chip is.
BUG=chromium:606629
Change-Id: I97d65a47a6130689098b32ce45a8c57c468aa405
Reviewed-on: https://boringssl-review.googlesource.com/7796
Reviewed-by: Adam Langley <agl@google.com>
The getauxval (and friends) code would be filling that in anyway. The default
only serves to enable NEON even if the OS is old enough to be missing getauxval
(and everything else).
Notably, this unbreaks the has_buggy_neon code when __ARM_NEON__ is set, as is
the case in Chrome for Android, as of M50. Before, the default
OPENSSL_armcap_P value was getting in the way.
Arguably, this doesn't make a whole lot of sense. We're saying we'll let the
CPU run compiler-generated NEON code, but not our hand-crafted stuff. But, so
far, we only have evidence of the hand-written NEON tickling the bug and not
the compiler-generated stuff, so avoid the unintentional regression. (Naively,
I would expect the hand-crafted NEON is better at making full use of the
pipeline and is thus more likely to tickle the CPU bug.)
This is not the fix for M50, as in the associated Chromium bug, but it will fix
master and M51. M50 will instead want to revert
https://codereview.chromium.org/1730823002.
BUG=chromium:606629
Change-Id: I394f97fea2f09891dd8fa30e0ec6fc6b1adfab7a
Reviewed-on: https://boringssl-review.googlesource.com/7794
Reviewed-by: Adam Langley <agl@google.com>
This reverts commits:
- 9158637142
- a90aa64302
- c0d8b83b44
It turns out code outside of BoringSSL also mismatches Init and Update/Final
functions. Since this is largely cosmetic, it's probably not worth the cost to
do this.
Change-Id: I14e7b299172939f69ced2114be45ccba1dbbb704
Reviewed-on: https://boringssl-review.googlesource.com/7793
Reviewed-by: Adam Langley <agl@google.com>
