Tag number 31 is a long form tag that requires multiple octets. It
cannot be handled by adding a single uint8. Changed CBB_add_asn1()
to return 0 when it is passed in the extension for tag 31.
Change-Id: Ia33936d4f174d1a7176eb11da0b5c7370efb9416
CBS_get_asn1() and CBS_get_any_asn1_element() only support the single
byte ASN.1 identifier octets (aka short form tags). Tag number 31 is
the start of the multi-byte long form per X.690 section 8.1.2.4.
Change-Id: I892600f4946e880a4ff03d219181116ef04f5372
Reviewed-on: https://boringssl-review.googlesource.com/3241
Reviewed-by: Adam Langley <agl@google.com>
All but one field is a no-op.
Change-Id: Ib7bc59a12ce792d5e42fb6e04a4aff54f42643a9
Reviewed-on: https://boringssl-review.googlesource.com/3213
Reviewed-by: Adam Langley <agl@google.com>
This is so the tests needn't be sensitive to the clock. It is, unfortunately, a
test-only hook, but the DTLS retransmit/timeout logic more-or-less requires it
currently. Use this hook to, for now, freeze the clock at zero. This makes the
tests deterministic.
It might be worth designing a saner API in the future. The current one,
notably, requires that the caller's clock be compatible with the one we
internally use. It's also not clear whether the caller needs to call
DTLSv1_handle_timeout or can just rely on the state machine doing it internally
(as it does do). But mock clocks are relatively tame and WebRTC wants to
compile against upstream OpenSSL for now, so we're limited in how much new API
we can build.
Change-Id: I7aad51570596f69275ed0fc1a8892393e4b7ba13
Reviewed-on: https://boringssl-review.googlesource.com/3210
Reviewed-by: Adam Langley <agl@google.com>
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.
This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.
Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Forgot to export those when adding them.
Change-Id: I206f488eb38e5ff55b8c212911aced0cf28b7664
Reviewed-on: https://boringssl-review.googlesource.com/3090
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 2747d73c1466c487daf64a1234b6fe2e8a62ac75.)
Also fix up some stylistic issues in conf.c and clarify empty case in
documentation.
Change-Id: Ibacabfab2339d7566d51db4b3ac4579aec0d1fbf
Reviewed-on: https://boringssl-review.googlesource.com/3023
Reviewed-by: Adam Langley <agl@google.com>
Since we can't update wpa_supplicant nearly as fast as we would like, we
need to try and keep it happy. Unfortunately, the recent switch to
EVP_AEAD breaks it so this dismal change adds some dummy variables that
will allow it to compile.
Change-Id: I03d6b81c30bbebc07af3af0d6cda85a26b461edf
Reviewed-on: https://boringssl-review.googlesource.com/2960
Reviewed-by: Adam Langley <agl@google.com>
Missing newlines. I think they got lost in some patch reordering.
Change-Id: Ib1e5833623f4ef613965d32b4e82ba18b6a551e6
Reviewed-on: https://boringssl-review.googlesource.com/2970
Reviewed-by: Adam Langley <agl@google.com>
Before it was possible to pass a NULL-terminated C-string to the PBKDF2
functions, and indicate the parameter was a C-string by passing a length
of -1.
This is not relied on anywhere in the BoringSSL code, and the API contract is
possible to misuse as it is not the common way of doing things.
(A problem would arise when passing in a large unsigned length that
subsequently gets interpreted as -1).
Change-Id: Ifbd31ff76e183fa74e9fa346908daf4bfb8fc3da
Reviewed-on: https://boringssl-review.googlesource.com/2953
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
As feared, 2bca0988 did cause some leak checkers to get upset about the
state_hash pointer getting cleared.
This change makes err_shutdown free all the error queues to try and
avoid this. Hopefully this doesn't upset TSAN in turn.
BUG=448296
Change-Id: I827da63c793dcabc73168ece052cdcd3d3cc64e3
Reviewed-on: https://boringssl-review.googlesource.com/2890
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
With GCC 4.9 and -O2 (and only -O2, -O1 and -O3 didn't trigger it), the
Poly1305 code can end up writing to an unaligned address otherwise and
that triggers a bus error on ARM.
Change-Id: Ifbeb7e2066a893d91d6f63c6565bac7d5542ef81
Reviewed-on: https://boringssl-review.googlesource.com/2850
Reviewed-by: Adam Langley <agl@google.com>
This is an initial cut at aarch64 support. I have only qemu to test it
however—hopefully hardware will be coming soon.
This also affects 32-bit ARM in that aarch64 chips can run 32-bit code
and we would like to be able to take advantage of the crypto operations
even in 32-bit mode. AES and GHASH should Just Work in this case: the
-armx.pl files can be built for either 32- or 64-bit mode based on the
flavour argument given to the Perl script.
SHA-1 and SHA-256 don't work like this however because they've never
support for multiple implementations, thus BoringSSL built for 32-bit
won't use the SHA instructions on an aarch64 chip.
No dedicated ChaCha20 or Poly1305 support yet.
Change-Id: Ib275bc4894a365c8ec7c42f4e91af6dba3bd686c
Reviewed-on: https://boringssl-review.googlesource.com/2801
Reviewed-by: Adam Langley <agl@google.com>
SSL_library_init already loads the error strings (unlike upstream). Code which
calls both will end up loading error strings twice. Instead make the second
call a no-op.
Change-Id: Ifd34ab20ed46aabeba14661e58f8dac2bbb29f69
Reviewed-on: https://boringssl-review.googlesource.com/2790
Reviewed-by: Adam Langley <agl@google.com>
Removes a bit of unused code. This effectively reverts upstream's
25af7a5dbc05c7359d1d7f472d50d65a9d876b7e. It's new with OpenSSL 1.0.2 so
nothing can be using it yet. We can restore it with tests if we end up wanting
it later.
(Also I think it might be misnamed. The KDF seems to be defined in X9.63, not
X9.62.)
Change-Id: I482daf681e0cf5c3bbdc72c57793f91448deaee8
Reviewed-on: https://boringssl-review.googlesource.com/2846
Reviewed-by: Adam Langley <agl@google.com>
The use in s3_srvr.c doesn't care (it doesn't even have to be in bounds), but
it's good to have the value be initialized and not a function of the input.
(The old uninitialized case wasn't hit in s3_srvr.c because of the earlier
bounds check.)
Change-Id: Ib6b418b3c140aa564f8a46da3d34bb2b69f06195
Reviewed-on: https://boringssl-review.googlesource.com/2845
Reviewed-by: Adam Langley <agl@google.com>
Add some missing headers and ensure each header has a short description. doc.go
gets confused at declarations that break before the first (, so avoid doing
that. Also skip a/an/deprecated: in markupFirstWord and process pipe words in
the table of contents.
Change-Id: Ia08ec5ae8e496dd617e377e154eeea74f4abf435
Reviewed-on: https://boringssl-review.googlesource.com/2839
Reviewed-by: Adam Langley <agl@google.com>
According to X6.90 null, object identifier, boolean, integer and enumerated
types can only have primitive encodings: return an error if any of
these are received with a constructed encoding.
(Imported from upstream's 89f40f369f414b52e00f7230b0e3ce99e430a508.)
Change-Id: Ia5d15eef72e379119f50fdbac4e92c4761bf5eaf
Reviewed-on: https://boringssl-review.googlesource.com/2835
Reviewed-by: Adam Langley <agl@google.com>
Based in part on upstream's cf75017bfd60333ff65edf9840001cd2c49870a3. This
situation really shouldn't be able to happen, but between no static asserts
that the minimum MTU is always large enough and a bug in reseting the MTU later
(to be fixed be a follow-up import from upstream), check these and return a
useful error code.
Change-Id: Ie853e5d35a6a7bc9c0032e74ae71529d490f4fe2
Reviewed-on: https://boringssl-review.googlesource.com/2825
Reviewed-by: Adam Langley <agl@google.com>
As of our 82b7da271f, an SSL_SESSION created
externally always has a cipher set. Unknown ciphers are rejected early. Prior
to that, an SSL_SESSION would only have a valid cipher or valid cipher_id
depending on whether it came from an internal or external session cache.
See upstream's 6a8afe2201cd888e472e44225d3c9ca5fae1ca62 and
c566205319beeaa196e247400c7eb0c16388372b for more context.
Since we don't get ourselves into this strange situation and s->cipher is now
always valid for established SSL_SESSION objects (the existence of
unestablished SSL_SESSION objects during a handshake is awkward, but something
to deal with later), do away with s->cipher_id altogether. An application
should be able to handle failing to parse an SSL_SESSION instead of parsing it
successfuly but rejecting all resumptions.
Change-Id: I2f064a815e0db657b109c7c9269ac6c726d1ffed
Reviewed-on: https://boringssl-review.googlesource.com/2703
Reviewed-by: Adam Langley <agl@google.com>
Now that BoringSSL no longer uses it internally, deprecate it until we can get
any Google code off it and remove it altogether.
Change-Id: I0e15525600b27a65f84b4bb820b879b2424a0ef7
Reviewed-on: https://boringssl-review.googlesource.com/2701
Reviewed-by: Adam Langley <agl@google.com>
This CL removes the last of the EVP_CIPHER codepath in ssl/. The dead code is
intentionally not pruned for ease of review, except in DTLS-only code where
adding new logic to support both, only to remove half, would be cumbersome.
Fixes made:
- dtls1_retransmit_state is taught to retain aead_write_ctx rather than
enc_write_ctx.
- d1_pkt.c reserves space for the variable-length nonce when echoed into the
packet.
- dtls1_do_write sizes the MTU based on EVP_AEAD max overhead.
- tls1_change_cipher_state_cipher should not free AEAD write contexts in DTLS.
This matches the (rather confused) ownership for the EVP_CIPHER contexts.
I've added a TODO to resolve this craziness.
A follow-up CL will remove all the resultant dead code.
Change-Id: I644557f4db53bbfb182950823ab96d5e4c908866
Reviewed-on: https://boringssl-review.googlesource.com/2699
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>
This lets us fold away the SSLv3-specific generate_master_secret. Once SSLv3
uses AEADs, others will fold away as well.
Change-Id: I27c1b75741823bc6db920d35f5dd5ce71b6fdbb3
Reviewed-on: https://boringssl-review.googlesource.com/2697
Reviewed-by: Adam Langley <agl@google.com>
Fix up the generate_master_secret parameter while we're here.
Change-Id: I1c80796d1f481be0c3eefcf3222f2d9fc1de4a51
Reviewed-on: https://boringssl-review.googlesource.com/2696
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>
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>
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>
Some parts of Android can't be updated yet so this change adds
declarations (only) for some functions that will be stubbed in
Android-specific code. (That Android-specific code will live in the
Android repo, not the BoringSSL repo.)
Trying to use these functions outside of Android will result in a link
error.
Change-Id: Iaa9b956e6408d21cd8fc34d90d9c15657e429877
Reviewed-on: https://boringssl-review.googlesource.com/2760
Reviewed-by: David Benjamin <davidben@chromium.org>
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
Add a dedicated error code to the queue for a handshake_failure alert in
response to ClientHello. This matches NSS's client behavior and gives a better
error on a (probable) failure to negotiate initial parameters.
BUG=https://crbug.com/446505
Change-Id: I34368712085a6cbf0031902daf2c00393783d96d
Reviewed-on: https://boringssl-review.googlesource.com/2751
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 doesn't retain partial blocks but it DOES update internal cipher state. ssl/
depends on this property.
Change-Id: I1e44b612c2e1549e096de8b71726007dcbc68de3
Reviewed-on: https://boringssl-review.googlesource.com/2640
Reviewed-by: Adam Langley <agl@google.com>
Turns out the EVP_CIPH_FLAG_CUSTOM_CIPHER ciphers (i.e. legacy EVP_CIPHER
AES-GCM) have a completely different return value setup than the normal ones
which are the standard one/zero. (Except that they never return zero; see
TODO.)
Fix checks in ssl/ and remove remnants of EVP_CIPH_FLAG_CUSTOM_CIPHER in ssl/
as we're using EVP_AEAD now.
See CHANGES entry added in upstream's 3da0ca796cae6625bd26418afe0a1dc47bf5a77f.
Change-Id: Ia4d0ff59b03c35fab3a08141c60b9534cb7172e2
Reviewed-on: https://boringssl-review.googlesource.com/2606
Reviewed-by: Adam Langley <agl@google.com>
This avoids needing a should_add_to_finished_hash boolean on do_write. The
logic in do_write was a little awkward because do_write would be called
multiple times if the write took several iterations. This also gets complex if
DTLS retransmits are involved. (At a glance, it's not obvious the
BIO_CTRL_DGRAM_MTU_EXCEEDED case actually works.)
Doing it as the handshake message is being prepared avoids this concern. It
also gives a natural point for the extended master secret logic which needs to
do work after the finished hash has been sampled.
As a bonus, we can remove s->d1->retransmitting which was only used to deal
with this issue.
Change-Id: Ifedf23ee4a6c5e08f960d296a6eb1f337a16dc7a
Reviewed-on: https://boringssl-review.googlesource.com/2604
Reviewed-by: Adam Langley <agl@google.com>
This makes SSLv23_method go through DTLS_ANY_VERSION's version negotiation
logic. This allows us to get rid of duplicate ClientHello logic. For
compatibility, SSL_METHOD is now split into SSL_PROTOCOL_METHOD and a version.
The legacy version-locked methods set min_version and max_version based this
version field to emulate the original semantics.
As a bonus, we can now handle fragmented ClientHello versions now.
Because SSLv23_method is a silly name, deprecate that too and introduce
TLS_method.
Change-Id: I8b3df2b427ae34c44ecf972f466ad64dc3dbb171
Tested manually by replacing SSLv23_method() with TLSv1_2_method() in
bssl_shim. This is a large chunk of code which is not run in SSLv23_method(),
but it will be run after unification. It's split out separately to ease review.
Change-Id: I6bd241daca17aa0f9b3e36e51864a29755a41097
Amend the version negotiation tests to test this new spelling of max_version.
min_version will be tested in a follow-up.
Change-Id: Ic4bfcd43bc4e5f951140966f64bb5fd3e2472b01
Reviewed-on: https://boringssl-review.googlesource.com/2583
Reviewed-by: Adam Langley <agl@google.com>
SSL3_ENC_METHOD will remain version-specific while SSL_METHOD will become
protocol-specific. This finally removes all the version-specific portions of
SSL_METHOD but the version tag itself.
(SSL3_ENC_METHOD's version-specific bits themselves can probably be handled by
tracking a canonicalized protocol version. It would simplify version
comparisons anyway. The one catch is SSLv3 has a very different table. But
that's a cleanup for future. Then again, perhaps a version-specific method
table swap somewhere will be useful later for TLS 1.3.)
Much of this commit was generated with sed invocation:
s/method->ssl3_enc/enc_method/g
Change-Id: I2b192507876aadd4f9310240687e562e56e6c0b1
Reviewed-on: https://boringssl-review.googlesource.com/2581
Reviewed-by: Adam Langley <agl@google.com>
Missed this one. It requires that we be able to change an SSL_METHOD after the
after, which complicates compiling the version locking into min_version /
max_version configurations.
Change-Id: I24ba54b7939360bbfafe3feb355a65840bda7611
Reviewed-on: https://boringssl-review.googlesource.com/2579
Reviewed-by: Adam Langley <agl@google.com>
SSL_ST_BEFORE isn't a possible state anymore. It seems this state meant the
side wasn't known, back in the early SSLeay days. Now upstream guesses
(sometimes incorrectly with generic methods), and we don't initialize until
later. SSL_shutdown also doesn't bother to call ssl3_shutdown at all if the
side isn't initialized and SSL_ST_BEFORE isn't the uninitialized state, which
seems a much more sensible arrangement.
Likewise, because bare SSL_ST_BEFOREs no longer exist, SSL_in_init implies
SSL_in_before and there is no need to check both.
Change-Id: Ie680838b2f860b895073dabb4d759996e21c2824
Reviewed-on: https://boringssl-review.googlesource.com/2564
Reviewed-by: Adam Langley <agl@google.com>
