This is not quite an end state (it still outputs an X509_ALGOR, the way
the generated salt is fed into key derivation is odd, and it uses the
giant OID table), but replaces a large chunk of it.
BUG=54
Change-Id: I0a0cca13e44e6a09dfaf6aed3b357cb077dc46d1
Reviewed-on: https://boringssl-review.googlesource.com/13065
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Many of these parameters are constants.
Change-Id: I148dbea0063e478a132253f4e9dc71d5d20320c2
Reviewed-on: https://boringssl-review.googlesource.com/13064
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is a very basic test, but it's something.
Change-Id: Ic044297e97ce5719673869113ce581de4621ebbd
Reviewed-on: https://boringssl-review.googlesource.com/13061
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
libcrypto can now be split in two, with everything that depends on
crypto/asn1 in a separate library. That said, Chromium still needs
crypto/pkcs8 to be implemented with CBS/CBB first. (Also libssl and
anything which uses X509* directly.)
BUG=54
Change-Id: Iec976ae637209882408457e94a1eb2465bce8d56
Reviewed-on: https://boringssl-review.googlesource.com/13059
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: Iad9b0898b3a602fc2e554c4fd59a599c61cd8ef7
Reviewed-on: https://boringssl-review.googlesource.com/13063
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
They're not called externally. Unexporting these will make it easier to
rewrite the PKCS{5,8,12} code to use CBS/CBB rather than X509_ALGOR.
Getting rid of those callers in Chromium probably won't happen for a
while since it's in our on-disk formats. (And a unit test for some NSS
client cert glue uses it.)
BUG=54
Change-Id: Id4148a2ad567484782a6e0322b68dde0619159fc
Reviewed-on: https://boringssl-review.googlesource.com/13062
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
This includes examples with both the NULL and empty passwords, thanks to
PKCS#12's password ambiguity.
Change-Id: Iae31840c1d31929fa9ac231509acaa80ef5b74bb
Reviewed-on: https://boringssl-review.googlesource.com/13060
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
The motiviation is that M2Crypto passes an ASN1_GENERALIZEDTIME to
this function. This is not distinct from ASN1_UTCTIME (both are
asn1_string_st), but ASN1_GENERALIZEDTIME uses a 4-digit year in its
string representation, whereas ASN1_UTCTIME uses a 2-digit year.
ASN1_UTCTIME_print previously did not return an error on such inputs.
So, stricten (?) the function, ensuring that it checks for trailing
data, and rejects values that are invalid for their place. Along the
way, clean it up and add tests.
Change-Id: Ia8298bed573f2acfdab96638ea69c78b5bba4e4b
Reviewed-on: https://boringssl-review.googlesource.com/13082
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Towards an eventual goal of opaquifying BoringSSL structs, we want
our consumers -- in this case, Android's libcore -- to not directly
manipulate BigNums; and it would be convenient for them if we would
perform the appropriate gymnastics to interpret little-endian byte
streams.
It also seems a priori a bit strange to have only big-endian varieties
of BN byte-conversions.
This CL provides little-endian equivalents of BN_bn2bin_padded
and BN_bin2bn.
BUG=97
Change-Id: I0e92483286def86d9bd71a46d6a967a3be50f80b
Reviewed-on: https://boringssl-review.googlesource.com/12641
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>
Passing in an array of scalars was removed some time ago, but a few
remnants of it remain.
Change-Id: Id75abedf60b1eab59f24bf7232187675b63291ab
Reviewed-on: https://boringssl-review.googlesource.com/13056
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 is a remnant of signature EVP_MDs. Detach them from
EVP_get_digestby{nid,obj}. Nothing appears to rely on this for those two
functions. Alas, Node.js appears to rely on it for EVP_get_digestbyname,
so keep that working.
This avoids causing every consumer's parsing to be unintentionally lax.
It also means fewer OIDs to transcribe when detaching the last of
libcrypto from the legacy ASN.1 stack and its giant OID table.
Note this is an externally visible change. There was one consumer I had
to fix, but otherwise everything handled things incorrectly due to this
quirk, so it seemed better to just fix the API rather than fork off a
second set.
Change-Id: I705e073bc05d946e71cd1c38acfa5e3c6b0a22b4
Reviewed-on: https://boringssl-review.googlesource.com/13058
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Passing in an array of scalars was removed some time ago, but a few
remnants of it remain.
Change-Id: Ia51dcf1f85116ec663e657cc8dbef7f23ffa2edb
Reviewed-on: https://boringssl-review.googlesource.com/13055
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Instead, use BN_mod_exp_mont_consttime of p - 2. This removes two more
call sites sensitive to BN_FLG_CONSTTIME. We're down to just that last
BN_mod_inverse modulo φ(n). (Sort of. It's actually not sensitive
because even mod inverses always hit the other codepath. Perhaps we
should just leave it alone.)
Note this comes with a slight behavior change. The BN_MONT_CTXs are
initialized a little earlier. If a caller calls RSA_generate_* and then
reaches into the struct to scrap all the fields on it, they'll get
confused. Before, they had to perform an operation on it to get
confused. This is a completely ridiculous thing to do.
Since we do this a lot, this introduces some convenience functions for
doing the Fermat's Little Theorem mod inverse and fixes a leak in the
DSA code should computing kinv hit a malloc error.
BUG=125
Change-Id: Iafcae2fc6fd379d161f015c90ff7050e2282e905
Reviewed-on: https://boringssl-review.googlesource.com/12925
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>
There's an authenticator, so test that AES_unwrap_key notices invalid
inputs.
Change-Id: Icbb941f91ffd9c91118f956fd74058d241f91ecb
Reviewed-on: https://boringssl-review.googlesource.com/13047
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 13ab87083af862e4af752efa4b0552149ed2cc19.)
Change-Id: I2f7cf8454d28d47f5ca19544479b2ab98143a3ef
Reviewed-on: https://boringssl-review.googlesource.com/13048
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This was noticed by observing we had one line of missing test coverage
in polyval.c. CRYPTO_POLYVAL_update_blocks acts 32 blocks at a time and
all existing test vectors are smaller than that.
Test vector obtained by just picking random values and seeing what our
existing implementation did if I modified CRYPTO_POLYVAL_update_blocks
to consume many more blocks at a time. Then I fixed the bug and ensured
the answer was still the same.
Change-Id: Ib7002dbc10952229ff42a17132c30d0e290d4be5
Reviewed-on: https://boringssl-review.googlesource.com/13041
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is a memory error for anything other than LHASH_OF(char), which
does not exist.
No code outside the library creates (or even queries) an LHASH, so we
can change this module freely.
Change-Id: Ifbc7a1c69a859e07650fcfaa067bdfc68d83fbbc
Reviewed-on: https://boringssl-review.googlesource.com/12978
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Use it to compare the contents of lh and dummy_lh are identical. Leave a
TODO for testing other LHASH cases.
Change-Id: Ifbaf17c196070fdff1530ba0e284030527855f5d
Reviewed-on: https://boringssl-review.googlesource.com/12977
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I0e8a24367cd33fa4aed2ca15bd369b8697f538e6
Reviewed-on: https://boringssl-review.googlesource.com/12974
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Use a std::map as the dummy lhash and use unique_ptr. This also improves
the test to check on pointer equality; we wish to ensure the lhash
stores the particular pointer value we asked for.
dummy_lh now also owns the pointers. It makes things simpler and since
LHASH doesn't free things, we weren't getting anything out of testing
that.
Change-Id: I97159175ca79a5874586650f272a7846100395e1
Reviewed-on: https://boringssl-review.googlesource.com/12976
Reviewed-by: Adam Langley <agl@google.com>
No source changes, just a rename.
Change-Id: Iaef406d2a04dc8c68c94eb2a98eec6378eaeab66
Reviewed-on: https://boringssl-review.googlesource.com/12975
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
BUG=97
Change-Id: I4799cc99511e73af44def1d4daa36a8b4699f62d
Reviewed-on: https://boringssl-review.googlesource.com/12904
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: If565a5fdfa0f314422aa26c2e8f869965ca08c1b
Reviewed-on: https://boringssl-review.googlesource.com/12969
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Chaining doesn't make much sense. This means we have a discontinuity
when buffer BIOs are empty. For a general filter BIO, this isn't even
meaningful. E.g., the base64 BIO's next_bio doesn't use the same units
(There's one consumer which does call BIO_pending on a base64 BIO, hits
this case, and is only working on accident, I've left it alone for this
CL until we can fix that consumer.)
The DTLS code, notably, assumes BIO_wpending to only report what's in
the buffer BIO. Ideally we'd get rid of the buffer BIO (I'll work on
this next), but, in the meantime, get the sizing right. The immediate
motivation is ssl_test using a BIO pair for DTLS doesn't work. We've
just been lucky none of the tests have been near the MTU.
The buffer BIO is actually unused outside of the SSL stack, so this
shouldn't break external consumers. But for the base64 BIO consumer
mentioned above, I see nothing else which relies on this BIO_[w]pending
chaining.
Change-Id: I6764df8ede0f89fe73c774a8f7c9ae4c054d4184
Reviewed-on: https://boringssl-review.googlesource.com/12964
Reviewed-by: Adam Langley <agl@google.com>
The perl script is a little nuts. obj_dat.pl actually parses the header
file that objects.pl emits to figure out what all the objects are.
Replace it all with a single Go script.
BUG=16
Change-Id: Ib1492e22dbe4cf9cf84db7648612b156bcec8e63
Reviewed-on: https://boringssl-review.googlesource.com/12963
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 thought I'd rewritten this, but apparently didn't. The old version
dated to a prior iteration which used macros.
Change-Id: Idefbdb2c11700a44dd5b0733b98efec102b10dd2
Reviewed-on: https://boringssl-review.googlesource.com/12968
Reviewed-by: Adam Langley <agl@google.com>
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html
Add OPENSSL_memcpy, etc., wrappers which avoid this problem.
BUG=23
Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This avoids having more generated bits. The table is quite small,
especially so when we take out anything we don't implement. There's no
real need to do the binary search. (Exotic things like GOST, the legacy
NID_rsa and NID_dsa_2 spellings of RSA and DSA, and hash functions we
don't implement.)
Mostly this saves me from having to reimplement obj_xref.pl.
(obj_xref.pl processes nid.h, formerly obj_mac.h, so we can't just use
the existing one and still change nid.h.)
Change-Id: I90911277e691a8b04ea8930f3f314d517f314d29
Reviewed-on: https://boringssl-review.googlesource.com/12962
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Chromium on Linux builds against libstdc++'s debug mode which makes
clang unhappy due to:
../crypto/bytestring/bytestring_test.cc:910:7: error: chosen constructor
is explicit in copy-initialization
{},
^~
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/debug/vector:79:7:
note: constructor declared here
vector(const _Allocator& __a = _Allocator())
^
I believe this was fixed here, but it's too recent:
36f540c70b
Change-Id: I2942d153e1278785c3b81294bc99b86f297cf719
Reviewed-on: https://boringssl-review.googlesource.com/12967
Reviewed-by: Adam Langley <agl@google.com>
X509_STORE_set0_additional_untrusted allows one to set a stack of
additional untrusted certificates that can be used during chain
building. These will be merged with the untrusted certificates set on
the |X509_STORE_CTX|.
Change-Id: I3f011fb0854e16a883a798356af0a24cbc5a9d68
Reviewed-on: https://boringssl-review.googlesource.com/12980
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 code, and in particular make |BN_div|, |BN_mod|, and
|BN_nnmod| insensitive to |BN_FLG_CONSTTIME|. This improves the
effectiveness of testing by reducing the number of branches that are
likely to go untested or less tested.
There is no performance-sensitive code that uses BN_div but doesn't
already use BN_FLG_CONSTTIME except RSA signature verification and
EC_GROUP creation. RSA signature verification, ECDH, and ECDSA
performance aren't significantly different with this change.
Change-Id: Ie34c4ce925b939150529400cc60e1f414c7676cd
Reviewed-on: https://boringssl-review.googlesource.com/9105
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
MSAN doesn't hook |syscall| and thus doesn't know that the kernel has
filled the output buffer when |getrandom| is called.
This change tells MSAN to trust that the memory that |getrandom| writes
to has been initialised. This should avoid false-positives when code
operates on |RAND_bytes| output.
Change-Id: I0a74ebb21bcd1de1f28eda69558ee27f82db807a
Reviewed-on: https://boringssl-review.googlesource.com/12903
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This reverts commit 97db926cf7,
effectively unreverting the two changes that it contained. A subsequent
change will fix this code for MSAN.
Change-Id: I54a82b667b7a4208c7a960aa28b01cb246bc78c7
Reviewed-on: https://boringssl-review.googlesource.com/12902
Commit-Queue: Adam Langley <alangley@gmail.com>
Reviewed-by: David Benjamin <davidben@google.com>
Get one step closer to removing the dependency on |BN_div| from most
programs. Also get one step closer to a constant-time implementation of
|BN_MONT_CTX_set|; we now "just" need to create a constant-time variant
of |BN_mod_lshift1_quick|.
Note that this version might actually increase the side channel signal,
since the variance in timing in |BN_div| is probably less than the variance
from the many conditional reductions in the new method.
On one Windows x64 machine, the speed of RSA verification using the new
version is not too different from the speed of the old code. However,
|BN_div| is generally slow on Windows x64 so I expect this isn't faster
on all platforms. Regardless, we generally consider ECDSA/EdDSA
signature verification performance to be adaquate and RSA signature
verification is much, much faster even with this change.
For RSA signing the performance is not a significant factor since
performance-sensitive applications will cache the |RSA| structure and
the |RSA| structure will cache the Montgomery contexts.
Change-Id: Ib14f1a35c99b8da435e190342657f6a839381a1a
Reviewed-on: https://boringssl-review.googlesource.com/10520
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>
Call |RSA_check_key| after parsing an RSA private key in order to
verify that the key is consistent. This is consistent with ECC key
parsing, which does a similar key check.
Call |RSA_check_key| after key generation mostly as a way of
double-checking the key generation was done correctly. A similar check
was not added to |EC_KEY_generate| because |EC_KEY_generate| is used
for generating ephemeral ECDH keys, and the check would be too
expensive for that use.
Change-Id: I5759d0d101c00711bbc30f81a3759f8bff01427c
Reviewed-on: https://boringssl-review.googlesource.com/7522
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 is imported from upstream's
71bbc79b7d3b1195a7a7dd5f547d52ddce32d6f0 and test vectors taken
initially from 2d7bbd6c9fb6865e0df480602c3612652189e182 (with a handful
more added).
The tests are a little odd because OpenSSL supports this "salt length
recovery" mode and they go through that codepath for all verifications.
Change-Id: I220104fe87e2a1a1458c99656f9791d8abfbbb98
Reviewed-on: https://boringssl-review.googlesource.com/12822
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This reverts commits 36ca21415a and
7b668a873e. We believe that we need to
update ASAN to be aware of getrandom before we can use it. Otherwise it
believes that the memory with the entropy from this syscall is
uninitialised.
Change-Id: I1ea1c4d3038b3b2cd080be23d7d8b60fc0c83df2
Reviewed-on: https://boringssl-review.googlesource.com/12901
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This change includes C versions of some of the functions from the x86-64
P-256 code that are currently implemented in assembly. These functions
were part of the original submission by Intel and are covered by the ISC
license.
No semantic change; code is commented out.
Change-Id: Ifdd2fac6caeb73d375d6b125fac98f3945003b32
Reviewed-on: https://boringssl-review.googlesource.com/12861
Reviewed-by: Adam Langley <agl@google.com>
This gives a 15-16% perf boost for 1024-bit RSA keys, but 1024-bit RSA
keys are no longer important enough for this code to carry its weight.
Change-Id: Ia9f0e7fec512c28e90754ababade394c1f11984d
Reviewed-on: https://boringssl-review.googlesource.com/12841
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 regression tests for
https://boringssl-review.googlesource.com/c/12525/ that target the
RSAZ-512 code rather than the disabled RSAZ-1024 code.
These were created by extracting p and dmp1 from
ssl/test/rsa_1024_key.pem and creating similar test vectors as with the
AVX2 test vectors. They currently fail, but pass if the RSAZ-512 code is
disabled.
Change-Id: I99dd3f385941ddbb1cc64b5351f4411081b42dd7
Reviewed-on: https://boringssl-review.googlesource.com/12840
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Note that this adds new non-constant-time code into the RSAZ-based
code path.
Change-Id: Ibca3bc523ede131b55c70ac5066c0014df1f5a70
Reviewed-on: https://boringssl-review.googlesource.com/12525
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 input base, |a|, isn't reduced mod |m| in the RSAZ case so
incorrect results are given for out-of-range |a| when the RSAZ
implementation is used. On the other hand, the RSAZ implementation is
more correct as far as constant-time operation w.r.t. |a| is concerned.
Change-Id: Iec4d0195cc303ce442ce687a4b7ea42fb19cfd06
Reviewed-on: https://boringssl-review.googlesource.com/12524
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 original bug only affected their big-endian code which we don't
have, but import the test vector anyway. Imported from upstream's
b47f116b1e02d20b1f8a7488be5a04f7cf5bc712.
Change-Id: I349e41d87006533da0e18c948f9cc7dd15f42a44
Reviewed-on: https://boringssl-review.googlesource.com/12820
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>
$1<<32>>32 worked fine with either 32- or 64-bit perl for a good while,
relying on quirk that [pure] 32-bit perl performed it as $1<<0>>0. But
this apparently changed in some version past minimally required 5.10,
and operation result became 0. Yet, it went unnoticed for another while,
because most perl package providers configure their packages with
-Duse64bitint option.
(Imported from upstream's 82e089308bd9a7794a45f0fa3973d7659420fbd8.)
Change-Id: Ie9708bb521c8d7d01afd2e064576f46be2a811a5
Reviewed-on: https://boringssl-review.googlesource.com/12821
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Querying a bit in a BIT STRING is a little finicky. Add some functions
to help with this.
Change-Id: I813b9b6f2d952d61d8717b47bca1344f0ad4b7d1
Reviewed-on: https://boringssl-review.googlesource.com/12800
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 change removes the use of |X509_get_pubkey| from the TLS <= 1.2
code. That function is replaced with a shallow parse of the certificate
to extract the public key instead.
Change-Id: I8938c6c5a01b32038c6b6fa58eb065e5b44ca6d2
Reviewed-on: https://boringssl-review.googlesource.com/12707
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 error value is -2, but at this point ret has already been set to
some running answer and must be reset to -2.
(This is unreachable. BN_rshift only fails on caller or malloc error,
and it doesn't need to malloc when running in-place.)
Change-Id: I33930da84b00d1906bdee9d09b9504ea8121fac4
Reviewed-on: https://boringssl-review.googlesource.com/12681
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Iaac633616a54ba1ed04c14e4778865c169a68621
Reviewed-on: https://boringssl-review.googlesource.com/12703
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: Ie947ab176d10feb709c6e135d5241c6cf605b8e8
Reviewed-on: https://boringssl-review.googlesource.com/12700
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>
A recent change to curl[1] added support for HTTPS proxies, which
involves running a TLS connection inside another TLS connection. This
was done by using SSL BIOs, which we removed from BoringSSL for being
crazy.
This change adds a stripped-down version of the SSL BIO to decrepit in
order to suport curl.
[1] cb4e2be7c6
Change-Id: I9cb8f2db5b28a5a70724f6f93544297c380ac124
Reviewed-on: https://boringssl-review.googlesource.com/12631
Reviewed-by: Adam Langley <agl@google.com>
d2i_X509 will free an existing |X509*| on parse failure. Thus
|X509_parse_from_buffer| would double-free the result on error.
Change-Id: If2bca2f1e1895bc426079f6ade4b82008707888d
Reviewed-on: https://boringssl-review.googlesource.com/12635
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>
Change-Id: I6514d68435ac4b7e2c638c7612b57bde5886bbba
Reviewed-on: https://boringssl-review.googlesource.com/12629
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've taken to writing bssl::UniquePtr in full, so it's not buying
us much.
Change-Id: Ia2689366cbb17282c8063608dddcc675518ec0ca
Reviewed-on: https://boringssl-review.googlesource.com/12628
Reviewed-by: David Benjamin <davidben@google.com>
This was only used by Chromium and was since replaced with a custom BIO.
Though it meant a new ring buffer implementation, custom BIOs seem a
better solution for folks who wish to do particularly complicated
things, until the new SSL API is available. External-buffer BIO pairs
were effectively a really confusing and leaky abstraction over a ring
buffer anyway.
Change-Id: I0e201317ff87cdccb17b2f8c260ee5bb06c74771
Reviewed-on: https://boringssl-review.googlesource.com/12626
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
AES-GCM-SIV is an AEAD with nonce-misuse resistance. It can reuse
hardware support for AES-GCM and thus encrypt at ~66% the speed, and
decrypt at 100% the speed, of AES-GCM.
See https://tools.ietf.org/html/draft-irtf-cfrg-gcmsiv-02
This implementation is generic, not optimised, and reuses existing AES
and GHASH support as much as possible. It is guarded by !OPENSSL_SMALL,
at least for now.
Change-Id: Ia9f77b256ef5dfb8588bb9ecfe6ee0e827626f57
Reviewed-on: https://boringssl-review.googlesource.com/12541
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 change will cause getrandom to be used in preference to
/dev/urandom when supported by the kernel.
This will also cause BoringSSL-using processes to block until the
entropy pool is initialised on systems that support getrandom(2).
Change-Id: I2d3a17891502c85884c77138ef0f3a719d7ecfe6
Reviewed-on: https://boringssl-review.googlesource.com/12421
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>
There only needs to be a single place where we do the generic
initialisation. All the processor-specific implementations can just
return early.
Change-Id: Ifd8a9c3bd7bec1ee8307aaa7bbeb9afe575e8a47
Reviewed-on: https://boringssl-review.googlesource.com/12540
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Previously, gcm.c contained a lot of workarounds for cases where BSWAP8
wasn't defined. Rather than handle this in each place, just make it
always available.
While we're here, make these macros inline functions instead and rename
them to something less likely to collide.
Change-Id: I9f2602f8b9965c63a86b177a8a084afb8b53a253
Reviewed-on: https://boringssl-review.googlesource.com/12479
Commit-Queue: Adam Langley <alangley@gmail.com>
Reviewed-by: Adam Langley <agl@google.com>
CRYPTO_ghash_init exposes the (often hardware accelerated) internals for
evaluating GHASH. These can be used for evaluating POLYVAL[1] on
platforms where we don't have dedicated code for it.
[1] https://tools.ietf.org/html/draft-irtf-cfrg-gcmsiv-02#section-3
Change-Id: Ida49ce4911f8657fa384b0bca968daa2ac6b26c1
Reviewed-on: https://boringssl-review.googlesource.com/12478
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 key is only needed during initialisation because after that point it
is implicit in the table of powers. So no need to keep it around. There
was a non-specific “haunted house” comment about not changing this, but
I've successfully tested with all the assembly versions so I think that
comment is no longer true.
Change-Id: Id110156afb528904f114d9a4ff2440e03a1a69b8
Reviewed-on: https://boringssl-review.googlesource.com/12477
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 GCM code has lots of cases of big-endian support left over from
OpenSSL. Since we don't support big-endian systems, drop that code.
Change-Id: I28eb95a9c235c6f705a145fbea72e7569dad2c70
Reviewed-on: https://boringssl-review.googlesource.com/12476
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>
j and md_size are public values, so this can just be done directly. (If
they weren't, we'd have worse problems.) This makes the loop look the
same as the rotation loop below.
Change-Id: Ic75550ad4e40b2015668cb12c26ca2d20bd285b6
Reviewed-on: https://boringssl-review.googlesource.com/12474
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Some declarations can be moved closer to use, etc.
Change-Id: Ifa9a51ad77639b94020b15478af234c82466390f
Reviewed-on: https://boringssl-review.googlesource.com/12473
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Although we ignore all but the first identity, keep clients honest by
parsing the whole thing. Also explicitly check that the binder and
identity counts match.
Change-Id: Ib9c4caae18398360f3b80f8db1b22d4549bd5746
Reviewed-on: https://boringssl-review.googlesource.com/12469
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>
BUG=101
Change-Id: Ia1edbccee535b0bc3a0e18465286d5bcca240035
Reviewed-on: https://boringssl-review.googlesource.com/12470
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 change imports sha256-armv4.pl from upstream at rev 8d1ebff4. This
includes changes to remove the use of adrl, which is not supported by
Clang.
Change-Id: I429e7051d63b59acad21601e40883fc3bd8dd2f5
Reviewed-on: https://boringssl-review.googlesource.com/12480
Commit-Queue: Adam Langley <alangley@gmail.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>
This code wants something which can represent -128..127 or so, not
something about characters.
Change-Id: Icdbfec370317a5e03803939a3b8d1555f8efff1d
Reviewed-on: https://boringssl-review.googlesource.com/12468
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
clang-format mangled this a little.
Change-Id: Ic4d8de0e1f6e926efbe8d14e390fe874b4a7cdcb
Reviewed-on: https://boringssl-review.googlesource.com/12467
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The compiler should be plenty smart enough to decide whether to inline a
static function called only once. We don't need to resort to so
unreadable a ternary chain.
Change-Id: Iacc8e0c4147fc69008806a0cc36d9e632169601a
Reviewed-on: https://boringssl-review.googlesource.com/12466
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I3350ff0e4ffe7495a83211b89c675a0125fb2f06
Reviewed-on: https://boringssl-review.googlesource.com/12465
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
EC_R_INVALID_COMPRESSED_POINT makes more sense than
EC_R_INVALID_COMPRESSION_BIT here.
Change-Id: I0dbdc91bab59843d5c04f2d0e97600fe7644753e
Reviewed-on: https://boringssl-review.googlesource.com/12464
Reviewed-by: Adam Langley <agl@google.com>
If y is zero, there is no point with odd y, so the odd bit may not be
set, hence EC_R_INVALID_COMPRESSION_BIT. This code instead computed the
Kronecker symbol of x and changed the error code to
EC_R_INVALID_COMPRESSED_POINT if not a square.
As the comment says, this was (intended to be) unreachable. But it
seems x was a typo for tmp1. It dates to before upstream's
6fb60a84dd1ec81953917e0444dab50186617432, when BN_mod_sqrt gave
garbage if its input was not square. Now it emits BN_R_NOT_A_SQUARE.
Upstream's 48fe4d6233ac2d60745742a27f820dd88bc6689d then mapped
BN_R_NOT_A_SQUARE to EC_R_INVALID_COMPRESSED_POINT.
Change-Id: Id9e02fa1c154b61cc0c3a768c9cfe6bd9674c378
Reviewed-on: https://boringssl-review.googlesource.com/12463
Reviewed-by: Adam Langley <agl@google.com>
Zero only has one allowed square root, not two.
Change-Id: I1dbd2137a7011d2f327b271b267099771e5499c3
Reviewed-on: https://boringssl-review.googlesource.com/12461
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This change causes SSL_CTX_set_signed_cert_timestamp_list to check the
SCT list for shallow validity before allowing it to be set.
Change-Id: Ib8a1fe185224ff02ed4ce53a0109e60d934e96b3
Reviewed-on: https://boringssl-review.googlesource.com/12401
Commit-Queue: 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>
Reviewed-by: David Benjamin <davidben@google.com>
This prevents a compiler warning from breaking ppc64le build.
Change-Id: I6752109bd02c6d078e656f89327093f8fb13a125
Reviewed-on: https://boringssl-review.googlesource.com/12363
Reviewed-by: Adam Langley <agl@google.com>
This change contains a C implementation of SHA-1 for POWER using
AltiVec. It is almost as fast as the scalar-only assembly implementation
for POWER/POWERPC family in OpenSSL but it is easier to maintain and it
allows error checking with tools like ASAN.
This is tested only for ppc64le. It may nor may not work for other
platforms in the POWER/POWERPC familiy.
Before:
SHA-1 @ 16 bytes: ~30 MB/s
SHA-1 @ 8K: ~140 MB/s
After:
SHA-1 @ 16 bytes: ~70 MB/s
SHA-1 @ 8K: ~480 MB/s
Change-Id: I790352e86d9c0cc4e1e57d11c5a0aa5b0780ca6b
Reviewed-on: https://boringssl-review.googlesource.com/12203
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>
Otherwise the run_tests target sometimes gets confused.
Change-Id: If49e945bf5137c68db4927ab0f9845d25be63bac
Reviewed-on: https://boringssl-review.googlesource.com/12315
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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>
A BN_ULONG[P256_LIMBS] can't represent a negative number and
bn_set_words won't produce one. We only need to compare against P.
Change-Id: I7bd1c9e8c162751637459f23f3cfc56884d85864
Reviewed-on: https://boringssl-review.googlesource.com/12304
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
RT#4625
(Imported from upstream's e3057a57caf4274ea1fb074518e4714059dfcabf.)
Add a test in ec_test to cover the ecp_nistz256_points_mul change. Also
revise the low-level infinity tests to cover the changes in
ecp_nistz256_point_add. Upstream's 'infty' logic was also cleaned up to
be simpler and take advantage of the only cases where |p| is infinity.
Change-Id: Ie22de834bf79ecb6191e824ad9fc1bd6f9681b8b
Reviewed-on: https://boringssl-review.googlesource.com/12225
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>
As a client, we must tolerate this to avoid interoperability failures
with allowed server behaviors.
BUG=117
Change-Id: I9c40a2a048282e2e63ab5ee1d40773fc2eda110a
Reviewed-on: https://boringssl-review.googlesource.com/12311
Reviewed-by: David Benjamin <davidben@google.com>
We were using a fully-qualified name for nearly everything anyway.
Change-Id: Ia32c68975ed4126feeab7b420f12b726ad6b89b3
Reviewed-on: https://boringssl-review.googlesource.com/12226
Reviewed-by: Adam Langley <agl@google.com>
The other field operations have an explicit _mont suffix to denote their
inputs and outputs are in the Montgomery domain, aside from
ecp_nistz256_neg which works either way. Do the same here.
Change-Id: I63741adaeba8140e29fb0b45dff72273e231add7
Reviewed-on: https://boringssl-review.googlesource.com/12224
Reviewed-by: Adam Langley <agl@google.com>
The file is util-64.c in BoringSSL.
Change-Id: I51891103254ae1541ea4c30f92c41d5d47c2ba55
Reviewed-on: https://boringssl-review.googlesource.com/12223
Reviewed-by: Adam Langley <agl@google.com>
For the most part, this is with random test data which isn't
particularly good. But we'll be able to add more interesting test
vectors as they come up.
Change-Id: I9c50db7ac2c4bf978d4901000ab32e3642aea82b
Reviewed-on: https://boringssl-review.googlesource.com/12222
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>
Addition was not preserving inputs' property of being fully reduced.
Thanks to Brian Smith for reporting this.
(Imported from upstream's b62b2454fadfccaf5e055a1810d72174c2633b8f and
d3034d31e7c04b334dd245504dd4f56e513ca115.)
See also this thread.
https://mta.openssl.org/pipermail/openssl-dev/2016-August/008179.html
Change-Id: I3731f949e2e2ef539dec656c58f1820cc09a56a6
Reviewed-on: https://boringssl-review.googlesource.com/11409
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I0e1d79e85a2d20ab4105b81d39cdbbd692ba67da
Reviewed-on: https://boringssl-review.googlesource.com/12221
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 don't support big-endian so this could only slow down whatever
platforms weren't listed in the #if.
Change-Id: Ie36f862663d947f591dd4896e6a2ab19122bbc0d
Reviewed-on: https://boringssl-review.googlesource.com/12202
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 Poly1305 state defined in the header file is just a 512-byte buffer.
The vector code aligns to 64 bytes but the non-vector code did not.
Since we have lots of space to spare, this change causes the non-vector
code to also align to 64 bytes.
Change-Id: I77e26616a709e770d6eb23df47d9e292742625d7
Reviewed-on: https://boringssl-review.googlesource.com/12201
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Some old assemblers can't cope with r0 in address. It's actually
sensible thing to do, because r0 is shunted to 0 in address arithmetic
and by refusing r0 assembler effectively makes you understand that.
(Imported from upstream's a54aba531327285f64cf13a909bc129e9f9d5970.)
This also pulls in a trailing whitespace fix from upstream's
609b0852e4d50251857dbbac3141ba042e35a9ae.
Change-Id: Ieec0bc8d24b98f86ce4fc9ee6ce5126d127cf452
Reviewed-on: https://boringssl-review.googlesource.com/12188
Reviewed-by: Adam Langley <agl@google.com>
The ASN1_GENERALIZEDTIME_adj() function leaks an ASN1_GENERALIZEDTIME
object on an error path.
(Imported from upstream's fe71bb3ad97ed01ccf92812891cc2bc3ef3dce76.)
Thanks to Jinguang Dong for pointing out the bug.
Change-Id: I2c14662bb03b0cf957bd277bda487f05f07e89e7
Reviewed-on: https://boringssl-review.googlesource.com/12185
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 2a7dd548a6f5d6f7f84a89c98323b70a2822406e and
9ebcbbba81eba52282df9ad8902f047e2d501f51.)
This is only in the ADX assembly codepath which we do not enable. See
$addx = 0 at the top of the file. Nonetheless, import the test vector
and fix since we still have the code in there.
Upstream's test vector only compares a*b against b*a. The expected
answer was computed using Python.
Change-Id: I3a21093978c5946d83f2d6f4f8399f69d78202cf
Reviewed-on: https://boringssl-review.googlesource.com/12186
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Fuzzer mode explores the handshake, but at the cost of losing coverage
on the record layer. Add a separate build flag and client/server
corpora for this mode.
Note this requires tweaks in consumers' fuzzer build definitions.
BUG=111
Change-Id: I1026dc7301645e165a761068a1daad6eedc9271e
Reviewed-on: https://boringssl-review.googlesource.com/12108
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In https://boringssl-review.googlesource.com/#/c/11920/2, I addressed a
number of comments but then forgot to upload the change before
submitting it. This change contains the changes that should have been
included in that commit.
Change-Id: Ib70548e791f80abf07a734e071428de8ebedb907
Reviewed-on: https://boringssl-review.googlesource.com/12160
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>
This function allows callers to unpack an Ed25519 “seed” value, which is
a 32 byte value that contains sufficient information to build a public
and private key from.
Change-Id: Ie5d8212a73e5710306314b4f8a93b707665870fd
Reviewed-on: https://boringssl-review.googlesource.com/12040
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The naming breaks layering, but it seems we're stuck with it. We don't
seem to have bothered making first-party code call it BIO_print_errors
(I found no callers of BIO_print_errors), so let's just leave it at
ERR_print_errors.
Change-Id: Iddc22a6afc2c61d4b94ac555be95079e0f477171
Reviewed-on: https://boringssl-review.googlesource.com/11960
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I87cbc12aeb399646c6426b7a099dbf13aafc2532
Reviewed-on: https://boringssl-review.googlesource.com/11983
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
d2i_X509_from_buffer parses an |X509| from a |CRYPTO_BUFFER| but ensures
that the |X509_CINF.enc| doesn't make a copy of the encoded
TBSCertificate. Rather the |X509| holds a reference to the given
|CRYPTO_BUFFER|.
Change-Id: I38a4e3d0ca69fc0fd0ef3e15b53181844080fcad
Reviewed-on: https://boringssl-review.googlesource.com/11920
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I0aab9c94fcfa58b9cd46eaf716d9317f532f79a2
Reviewed-on: https://boringssl-review.googlesource.com/11850
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Tagging non-pointer return types const doesn't do anything and makes
some compilers grumpy. Thanks to Daniel Hirche for the report.
Change-Id: I157ddefd8f7e604b4d8317ffa2caddb8f0dd89de
Reviewed-on: https://boringssl-review.googlesource.com/11849
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Neither branch of the |if| statement is expected to touch |data_len|.
Clarify this by moving |data_len| after the |if| statement.
Change-Id: Ibbc81e5b0c006882672df18442a6e7987064ca6d
Reviewed-on: https://boringssl-review.googlesource.com/11880
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>
This patch changes the urandom PRNG to read one byte from the
getrandom(2) Linux syscall on initialization in order to find any
unexpected behavior.
Change-Id: I8ef676854dc361e4f77527b53d1a14fd14d449a8
Reviewed-on: https://boringssl-review.googlesource.com/8681
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These structures allow for blobs of data (e.g. certificates) to be
deduplicated in memory.
Change-Id: Iebfec90b85d55565848a178b6951562b4ccc083e
Reviewed-on: https://boringssl-review.googlesource.com/11820
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
At some point, we'll forget to look in the commit message.
Change-Id: I3153aab679209f4f11f56cf3f883c4c74a17af1d
Reviewed-on: https://boringssl-review.googlesource.com/11800
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
clang-cl now supports enough of `#pragma intrinsic` that
it can use SecureZeroMemory() without an explicit intrin.h include.
This reverts https://boringssl-review.googlesource.com/#/c/8320/
BUG=chromium:592745
Change-Id: Ib766133f1713137bddd07654376a3b4888d4b0fb
Reviewed-on: https://boringssl-review.googlesource.com/11780
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 missed these in the last round.
Change-Id: I9b47216eef87c662728e454670e9e516de71ca21
Reviewed-on: https://boringssl-review.googlesource.com/11740
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I51e5a7dac3ceffc41d3a7a57157a11258e65bc42
Reviewed-on: https://boringssl-review.googlesource.com/11721
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Unhandled critical CRL extensions were not detected if they appeared
after the handled ones. (Upstream GitHub issue 1757). Thanks to John
Chuah for reporting this.
(Imported from upstream's 3ade92e785bb3777c92332f88e23f6ce906ee260.)
This additionally adds a regression test for this issue, generated with
der-ascii. The signatures on the CRLs were repaired per notes in
https://github.com/google/der-ascii/blob/master/samples/certificates.md
Change-Id: I74a77f92710e6ef7f46dcde5cb6ae9350084ddcb
Reviewed-on: https://boringssl-review.googlesource.com/11720
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
It's possible that a BIO implementation could return a negative number
(say -1) for BIO_CTRL_PENDING or BIO_CTRL_WPENDING. Assert that this
doesn't happen and map it to zero if it happens anyway in NDEBUG builds.
Change-Id: Ie01214e80ff19acc1c7681a1125bbbf2038679c3
Reviewed-on: https://boringssl-review.googlesource.com/11700
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>
Nodejs 6.9.0 calls this function.
Change-Id: I375f222cb819ebcb9fdce0a0d63df6817fa2dcae
Reviewed-on: https://boringssl-review.googlesource.com/11625
Commit-Queue: Adam Langley <alangley@gmail.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>
This code reportedly upsets VC++'s static analysis. Make it clear that,
yes, we want to count backwards.
Change-Id: I5caba219a2b87750d1a9d69b46d336a98c5824c9
Reviewed-on: https://boringssl-review.googlesource.com/11624
Commit-Queue: Adam Langley <alangley@gmail.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>
Macros need a healthy dose of parentheses to avoid expression-level
misparses. Most of this comes from the clang-tidy CL here:
https://android-review.googlesource.com/c/235696/
Also switch most of the macros to use do { ... } while (0) to avoid all
the excessive comma operators and statement-level misparses.
Change-Id: I4c2ee51e347d2aa8c74a2d82de63838b03bbb0f9
Reviewed-on: https://boringssl-review.googlesource.com/11660
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We have bool here. Also the comments were a mix of two styles.
Change-Id: I7eb6814b206efa960ae7e6e1abc14d64be6d61cf
Reviewed-on: https://boringssl-review.googlesource.com/11602
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 reverts commit 7b9bbd9639. This seems
to cause some problem linking with gold in Chromium:
../../third_party/binutils/Linux_x64/Release/bin/ld.gold: warning: Cannot export local symbol 'free'
../../third_party/binutils/Linux_x64/Release/bin/ld.gold: warning: Cannot export local symbol 'malloc'
../../third_party/binutils/Linux_x64/Release/bin/ld.gold: warning: Cannot export local symbol 'realloc'
../../third_party/binutils/Linux_x64/Release/bin/ld.gold: error: treating warnings as errors
The same error in https://crbug.com/368351 implies we're actually
causing the compiler to make some assumptions it shouldn't make. The
obvious fix of marking things as visible causes crashes when built with
ASan (ASan's malloc interceptors and ours are conflicting somehow).
Revert this for now. We should study how ASan's interceptors work and
figure out how to make these two coexist.
BUG=655938
Change-Id: Iaad245d1028c442bd924d46519b20115d37a57c4
Reviewed-on: https://boringssl-review.googlesource.com/11604
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>
This clears the last of Android's build warnings from BoringSSL. These
pragmas aren't actually no-ops, but it just means that MinGW consumers
(i.e. just Android) need to explicitly list the dependency (which they
do).
There may be something to be said for removing those and having everyone
list dependencies, but I don't really want to chase down every
consumer's build files. Probably not worth the trouble.
Change-Id: I8fcff954a6d5de9471f456db15c54a1b17cb937a
Reviewed-on: https://boringssl-review.googlesource.com/11573
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 part of TLS 1.3 draft 16 but isn't much of a wire format change,
so go ahead and add it now. When rolling into Chromium, we'll want to
add an entry to the error mapping.
Change-Id: I8fd7f461dca83b725a31ae19ef96c890d603ce53
Reviewed-on: https://boringssl-review.googlesource.com/11563
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: Id8099cc3a250e36e62b8a48e74706b75e5fa127c
Reviewed-on: https://boringssl-review.googlesource.com/11566
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
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>
