Test that SSLv3 accepts arbitrary padding bytes (hello, POODLE) and rejects
non-minimal padding, while TLS accepts non-minimal padding but rejects
arbitrary padding bytes.
Also test what happens when the MAC is correct, but there is no padding. This
is the case that triggers a failing padding_ok check after the MAC check
on padding_len = 0 passes.
Change-Id: Ia1444c526437899fc57ceafcbcef9c8f5cb9a6c5
Reviewed-on: https://boringssl-review.googlesource.com/2702
Reviewed-by: Adam Langley <agl@google.com>
This introduces another knob into SSL_AEAD_CTX to omit the version from the ad
parameter. It also allows us to fold a few more SSL3_ENC_METHOD hooks together.
Change-Id: I6540d410d4722f734093554fb434dab6e5217d4f
Reviewed-on: https://boringssl-review.googlesource.com/2698
Reviewed-by: Adam Langley <agl@google.com>
HMAC_CTX_copy's documentation is off. It actually follows the old copy
functions which call FOO_init on dest first. Notably this means that they leak
memory if dest is currently in use.
Add HMAC_CTX_copy_ex as an analog of EVP_MD_CTX_copy and deprecate
HMAC_CTX_copy. (EVP_CIPHER_CTX_copy, in contrast, was correct from the start.)
Change-Id: I48566c858663d3f659bd356200cf862e196576c9
Reviewed-on: https://boringssl-review.googlesource.com/2694
Reviewed-by: Adam Langley <agl@google.com>
CBC modes in SSLv3 are bust already with POODLE and we're moving away from it.
Align all the names from 'ssl3' and 'tls1' to 'tls', to match the names of the
TLS-only AEADs.
Change-Id: If742296a8e2633ef42a484e4d873b4a83558b6aa
Reviewed-on: https://boringssl-review.googlesource.com/2693
Reviewed-by: Adam Langley <agl@google.com>
The EVP_CIPHER codepath should no longer be used with TLS. It still exists for
DTLS and SSLv3. The AEAD construction in TLS does not allow for
variable-overhead AEADs, so stateful AEADs do not include the length in the ad
parameter. Rather the AEADs internally append the unpadded length once it is
known. EVP_aead_rc4_md5_tls is modified to account for this.
Tests are added (and RC4-MD5's regenerated) for each of the new AEADs. The
cipher tests are all moved into crypto/cipher/test because there's now a lot of
them and they clutter the directory listing.
In ssl/, the stateful AEAD logic is also modified to account for stateful AEADs
with a fixed IV component, and for AEADs which use a random nonce (for the
explicit-IV CBC mode ciphers).
The new implementation fixes a bug/quirk in stateless CBC mode ciphers where
the fixed IV portion of the keyblock was generated regardless. This is at the
end, so it's only relevant for EAP-TLS which generates a MSK from the end of
the key block.
Change-Id: I2d8b8aa11deb43bde2fd733f4f90b5d5b8cb1334
Reviewed-on: https://boringssl-review.googlesource.com/2692
Reviewed-by: Adam Langley <agl@google.com>
These helper functions will be used in the implementation of the legacy CBC
mode AEADs. The file is copied as-is and then modified to remove the dependency
on ssl/. Notably explicit IV logic is removed (that's a side effect of how
explicit IVs are currently implemented) and the padding length is returned
directly rather than smuggled into rec->type.
(Diffing tls_cbc.c and s3_cbc.c is probably the easiest for a review.)
The helpers are currently unused.
Change-Id: Ib703f4d3620196c9f2921cb3b8bf985f2d1777db
Reviewed-on: https://boringssl-review.googlesource.com/2691
Reviewed-by: Adam Langley <agl@google.com>
The extra free in ex_data_impl.c is fixing a mistake: when calling
|CRYPTO_cleanup_all_ex_data| the |EX_CLASS_ITEM| itself wouldn't be
freed.
The change in err_impl.c is to free the thread-id hash also. This allows
programs to free absolutely all memory allocated by BoringSSL, which
allows fuzz testing to find any memory leaks.
Change-Id: I1e518adf2b3e0efa7d7f00f7ab4e65e1dc70161e
Reviewed-on: https://boringssl-review.googlesource.com/2670
Reviewed-by: Adam Langley <agl@google.com>
DSA_verify and DSA_check_signature didn't share a codepath, so the fix was only
applied to the former. Implement verify in terms of check_signature and add
tests for bad DER variants.
Change-Id: I6577f96b13b57fc89a5308bd8a7c2318defa7ee1
Reviewed-on: https://boringssl-review.googlesource.com/2820
Reviewed-by: Adam Langley <agl@google.com>
By using non-DER or invalid encodings outside the signed portion of a
certificate the fingerprint can be changed without breaking the signature.
Although no details of the signed portion of the certificate can be changed
this can cause problems with some applications: e.g. those using the
certificate fingerprint for blacklists.
1. Reject signatures with non zero unused bits.
If the BIT STRING containing the signature has non zero unused bits reject the
signature. All current signature algorithms require zero unused bits.
2. Check certificate algorithm consistency.
Check the AlgorithmIdentifier inside TBS matches the one in the certificate
signature. NB: this will result in signature failure errors for some broken
certificates.
3. Check DSA/ECDSA signatures use DER.
Reencode DSA/ECDSA signatures and compare with the original received signature.
Return an error if there is a mismatch.
This will reject various cases including garbage after signature (thanks to
Antti Karjalainen and Tuomo Untinen from the Codenomicon CROSS program for
discovering this case) and use of BER or invalid ASN.1 INTEGERs (negative or
with leading zeroes).
CVE-2014-8275
(Imported from upstream's 85cfc188c06bd046420ae70dd6e302f9efe022a9 and
4c52816d35681c0533c25fdd3abb4b7c6962302d)
Change-Id: Ic901aea8ea6457df27dc542a11c30464561e322b
Reviewed-on: https://boringssl-review.googlesource.com/2783
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Write the array literal of all zeros as {0} rather than {}.
Change-Id: If15330d96d019be671d3bcbbdea60c2b3ecc2128
Reviewed-on: https://boringssl-review.googlesource.com/2740
Reviewed-by: Adam Langley <agl@google.com>
I typoed this word and then auto-complete duplicated it all over the
place. This change fixes all the comments.
This change has no semantic effect (comment only).
Change-Id: I8952e9e71302043574757cd74a05e66500008432
Using OPENSSL_WINDOWS for this is inaccurate because it's really a
feature of the compiler, not the platform. I think it's only MSVC that
uses the UI64 suffix.
Change-Id: I4a95961b94e69e72b93f5ed1e0457661b74242c8
Reviewed-on: https://boringssl-review.googlesource.com/2730
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
ERR_get_error returns the least recent error, not the most recent error.
Nothing in err_test was actually asserting on that.
Change-Id: Ia49e29c231de4bbec77d037860ad1ffa8cce4779
Reviewed-on: https://boringssl-review.googlesource.com/2750
Reviewed-by: Adam Langley <agl@google.com>
One about a possible uninitialised variable (incorrect, but it's easier
to keep the compiler happy) and one warning about "const static" being
backwards.
Change-Id: Ic5976a5f0b48f32e09682e31b65d8ea1c27e5b88
Reviewed-on: https://boringssl-review.googlesource.com/2632
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Since this is C89 we need to maintain this ancient practice.
Change-Id: I7223e7c38a35cf551b6e3c9159d2e21ebf7e62be
Reviewed-on: https://boringssl-review.googlesource.com/2631
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It's a static function anyway so it doesn't affect anything and it's
colliding with a debugging function on one platform.
Change-Id: Iae0595cce7cb2bdd4c56217f6f1de51ff3134a8b
Reviewed-on: https://boringssl-review.googlesource.com/2630
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The expectation when calling HMAC with key=NULL and keylen=0 is to compute
HMAC on the provided data with a key of length 0 instead of using the
"previous" key, which in the case of HMAC() is whatever bytes happen to be
left on the stack when the HMAC_CTX struct is allocated.
Change-Id: I52a95e262ee4e15f1af3136cb9c07f42f40ce122
Reviewed-on: https://boringssl-review.googlesource.com/2660
Reviewed-by: Adam Langley <agl@google.com>
RAND_pseudo_bytes just calls RAND_bytes now and only returns 0 or 1. Switch all
callers within the library call the new one and use the simpler failure check.
This fixes a few error checks that no longer work (< 0) and some missing ones.
Change-Id: Id51c79deec80075949f73fa1fbd7b76aac5570c6
Reviewed-on: https://boringssl-review.googlesource.com/2621
Reviewed-by: Adam Langley <agl@google.com>
Bruce Dawson pointed out that the shadowing of |ret| in |s3_srvr.c|
looked dodgy. It was actually deliberate (we don't want to reset the
default value of the function's |ret| variable with a successful return
from the callback) but it does look dodgy.
This change adds -Wshadow to ban variable shadowing and fixes all
current instances.
Change-Id: I1268f88b9f26245c7d16d6ead5bb9014ea471c01
Reviewed-on: https://boringssl-review.googlesource.com/2520
Reviewed-by: Adam Langley <agl@google.com>
All serialization functions take point format as input, and
asn1_form is never used.
Change-Id: Ib1ede692e815ac0c929e3b589c3a5869adb0dc8b
Reviewed-on: https://boringssl-review.googlesource.com/2511
Reviewed-by: Adam Langley <agl@google.com>
According to rfc5480 and rfc4492 the hybrid format is not allowed
neither in certificates or the tls protocol.
Change-Id: I1d3fb5bef765bc7b58d29bdd60e15247fac4dc7a
Reviewed-on: https://boringssl-review.googlesource.com/2510
Reviewed-by: Adam Langley <agl@google.com>
The files should round-trip now. This corrects some discrepancies between
obj_mac.h and obj_mac.num which were also present in upstream. There seems to
be a mismerge in upstream's eebd5e5dd7dff58297ea52e1c21df8fccd593965.
(The discrepancy is harmless; those OIDs are not in obj_xref.txt.)
Change-Id: I1f6cda016533ec3182750310f9936f7e072b54a0
Reviewed-on: https://boringssl-review.googlesource.com/2474
Reviewed-by: Adam Langley <agl@google.com>
Probably best to keep the original format, trailing whitespace and all.
Change-Id: I81a0ac46fd4ab4bb9d2b03d930b191024971447c
Reviewed-on: https://boringssl-review.googlesource.com/2473
Reviewed-by: Adam Langley <agl@google.com>
This got reset at some point, but not the files generated from it.
obj_mac.num is an input/output parameter to objects.pl and used to keep the
NIDs stable.
Imported from f2d678e6e89b6508147086610e985d4e8416e867, the point at which we
forked.
Change-Id: Ifd52b1aaa55054d37bc1217f2375a93302839e23
Reviewed-on: https://boringssl-review.googlesource.com/2472
Reviewed-by: Adam Langley <agl@google.com>
Make the commands print a short usage summary and add a README file that
explains the dependencies.
Change-Id: I0c3f0713749ecfca23afaa2b536ac70dbdd7db0a
Reviewed-on: https://boringssl-review.googlesource.com/2471
Reviewed-by: Adam Langley <agl@google.com>
Upstream (impressively quickly) fixed the missing intrinsic. Switch Windows
clang back to building the same code as MSVC. Also include the intrin.h header
rather than forward-declare the intrinsic. clang only works if the header is
explicitly included. Chromium forcibly includes it to work around these kinds
of issues, but we shouldn't rely on it.
BUG=crbug.com/438382
Change-Id: I0ff6d48e1a3aa455cff99f8dc4c407e88b84d446
Reviewed-on: https://boringssl-review.googlesource.com/2461
Reviewed-by: Adam Langley <agl@google.com>
Use it in ssl3_cert_verify_hash so signing a pre-TLS-1.2 handshake hash can go
through RSA_sign and be intercepted via RSA_METHOD appropriately. This avoids
Windows needing to intercept sign_raw. (CAPI keys cannot provide sign_raw,
unless the input size happens to be that of NID_md5_sha1.)
Also use it in processing ServerKeyExchange to avoid special-casing RSA.
BUG=crbug.com/437023
Change-Id: Ia07433f468b75fdf7bfc8fa90c9751639b2478e6
Reviewed-on: https://boringssl-review.googlesource.com/2420
Reviewed-by: David Benjamin <davidben@google.com>
Windows clang lacks _umul128, but it has inline assembly so just use
that.
Change-Id: I6ff5d2465edc703a4d47ef0efbcea43d6fcc79fa
Reviewed-on: https://boringssl-review.googlesource.com/2454
Reviewed-by: Adam Langley <agl@google.com>
This probably snuck in when adapting the code from upstream. There's a header
file for it now. (Also it's uint32_t now rather than unsigned int.)
Change-Id: Ie8f45bc7a88988744174182a70512c0eff37cc1c
Reviewed-on: https://boringssl-review.googlesource.com/2441
Reviewed-by: Adam Langley <agl@google.com>
MSVC does not allow pointer arithmetic on void* pointers. Also
fix some style issues around whether * hugs the type or the
variable name.
Change-Id: I40cc1627830b37879fd70e2b688a42df62b6c62a
Reviewed-on: https://boringssl-review.googlesource.com/2452
Reviewed-by: Adam Langley <agl@google.com>
Don't use |BIO_set_foo_buffer_size| when setting the
sizes of the buffers while making buffer pair. Since it
happens in pair.c we know the BIOs are BIO pairs and using
bio_ctrl here complicates setting external buffers. Also
zero out bio_bio_st during construction.
This fixes a problem that would happen if the default buffer
sizes were not set, since buf_externally_allocated was
not yet initialized.
Remove BIO_C_SET_BUFF_SIZE and BIO_CTRL_RESET which are
not used for bio pairs.
Change-Id: I365091d5f44f6f1c5522c325a771bdf03d8fe950
Reviewed-on: https://boringssl-review.googlesource.com/2370
Reviewed-by: Adam Langley <agl@google.com>
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.
(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)
This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.
Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
This is intended for TLS client auth with Windows CAPI- and CNG-backed keys
which implement sign over sign_raw and do not support all hash functions. Only
plumbed through RSA for now.
Change-Id: Ica42e7fb026840f817a169da9372dda226f7d6fd
Reviewed-on: https://boringssl-review.googlesource.com/2250
Reviewed-by: Adam Langley <agl@google.com>
Also add functionality for setting external buffers to give the
caller better control of the buffers. This is typical needed if OS
sockets can outlive the bio pair.
Change-Id: I500f0c522011ce76e9a9bce5d7b43c93d9d11457
No need to include unistd.h. (Though it probably should include string.h for
memcmp and strcmp.)
Change-Id: Ib09d2da4f7079c9d87338df75ec3560f4f203764
Reviewed-on: https://boringssl-review.googlesource.com/2260
Reviewed-by: Adam Langley <agl@google.com>
The error queue should only take ownership of the data if ERR_get_* is called,
not ERR_peek_*. Add a test for ERR_peek_error_line_data.
Change-Id: I976fc90fb54437dff723418ef3afd94f1c967922
Reviewed-on: https://boringssl-review.googlesource.com/2237
Reviewed-by: Adam Langley <agl@google.com>
Saves doing it ad-hoc all the time.
Change-Id: Ic1a1180f56eec37c19799649bb8f18237bd617f8
Reviewed-on: https://boringssl-review.googlesource.com/2241
Reviewed-by: Adam Langley <agl@google.com>
r and s are scalars, not EC coordinates.
Change-Id: I46a20215d3c602559c18c74a1da9a91543ea73ca
Reviewed-on: https://boringssl-review.googlesource.com/2240
Reviewed-by: Adam Langley <agl@google.com>
One of them was never implemented upstream or downstream. The other no longer
works in BoringSSL. They're not used within BoringSSL (this still compiles),
even in X509_INFO, and do not appear to be used by consumers. If they were, we
would like to know via a compile failure.
This removes the last consumer within BoringSSL of the ASN.1 parsing macros.
Change-Id: Ifb72b1fcd0a4f7b3e6b081486f8638110872334b
Reviewed-on: https://boringssl-review.googlesource.com/2203
Reviewed-by: Adam Langley <agl@google.com>
(Imported form upstream's 455b65dfab0de51c9f67b3c909311770f2b3f801 and
0d6a11a91f4de238ce533c40bd9507fe5d95f288)
Change-Id: Ia195c7fe753cfa3a7f8c91d2d7b2cd40a547be43
Imported from upstream's 9bed73adaa6f834177f29e478d9a2247a6577c04.
Upstream's commit appears to have been based on BoringSSL's commits to
improve the constant-time behaviour of RSA padding checks and thus I've
not tried to import those bits of the change.
Change-Id: I0ea5775b0f1e18741bbbc9f792a6af0d3d2a4caf
