The call to asn1_do_adb can return NULL on error, so we should check the
return value before attempting to use it.
(Imported from upstream's 34a7ed0c39aa3ab67eea1e106577525eaf0d7a00.)
Change-Id: Ia43cdc73b5f1d16e6fc907b5aaf13c9df5a9958c
Reviewed-on: https://boringssl-review.googlesource.com/4007
Reviewed-by: Adam Langley <agl@google.com>
ASN1_primitive_new takes an ASN1_ITEM * param |it|. There are a couple
of conditional code paths that check whether |it| is NULL or not - but
later |it| is deref'd unconditionally. If |it| was ever really NULL then
this would seg fault. In practice ASN1_primitive_new is marked as an
internal function in the public header file. The only places it is ever
used internally always pass a non NULL parameter for |it|. Therefore, change
the code to sanity check that |it| is not NULL, and remove the conditional
checking.
(Imported from upstream's 9e488fd6ab2c295941e91a47ab7bcd346b7540c7)
Change-Id: Icbb13cd00d0ec5529871b678b0bcc465956a7572
Reviewed-on: https://boringssl-review.googlesource.com/4006
Reviewed-by: Adam Langley <agl@google.com>
In the event of an error |rr| could be NULL. Therefore don't assume you can
use |rr| in the error handling code.
(Imported from upstream's 8c5a7b33c6269c3bd6bc0df6b4c22e4fba03b485.)
Change-Id: I0b392991ce8170dc418e93003af256d535d1e2e8
Reviewed-on: https://boringssl-review.googlesource.com/4005
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's e1b568dd2462f7cacf98f3d117936c34e2849a6b.)
Our RAND_bytes secretly can't actually fail, but we should propagate the check
upwards.
Change-Id: Ieaaea98dad00bf73b1c0a42c039507d76b10ac78
Reviewed-on: https://boringssl-review.googlesource.com/4003
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 0b142f022e2c5072295e00ebc11c5b707a726d74.)
Modified further because these ought to just be BIO_puts.
Change-Id: I8b70d70d0f626bb19b455adb4f0e08bacf453c1d
Reviewed-on: https://boringssl-review.googlesource.com/4002
Reviewed-by: Adam Langley <agl@google.com>
(Thanks to William Hesse.)
Change-Id: I8479663250546a5ec0a024f80e50541f91d833bc
Reviewed-on: https://boringssl-review.googlesource.com/4020
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
They do not quite measure the same value for EC keys. "size" is a really weird
notion to generalize on so we should document what it means for each key type.
EVP_PKEY_size's meaning is most tied to signatures, thanks to EVP_SignFinal
implicitly using it as output bounds.
Change-Id: I7504c142818f8f90f8bcf6891c97a6adaf2d574e
Reviewed-on: https://boringssl-review.googlesource.com/4000
Reviewed-by: Adam Langley <agl@google.com>
C99 doesn't, technically, allow empty statements. Thus if a #define'ed
function ends in a semicolon, and the use of it also ends in a
semicolon, then the compiler sees “;;” at the end.
Since a choice has to be made, I prefer that the semicolon exist at the
“callsite” of a #define'ed fuction. But I haven't gone and changed
everything to follow that in this patch.
Change-Id: I1343e52a5ac6255db49aa053048d0df3225bcf43
Reviewed-on: https://boringssl-review.googlesource.com/3890
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Firstly, it was odd that AES-NI was a special case. Secondly, I have a
need coming up for being able to get the block function and not create a
GCM context.
Change-Id: Ie87de5e7ea42dc042d302c5eafecbc6af03c714b
Reviewed-on: https://boringssl-review.googlesource.com/3910
Reviewed-by: Adam Langley <agl@google.com>
At least the linker can discard this function in the cases where nobody
is calling it.
Change-Id: I30050e918e6bc1dd9c97cc70f3a56408701abebc
Reviewed-on: https://boringssl-review.googlesource.com/3724
Reviewed-by: Adam Langley <agl@google.com>
Because NTLM authentication is still a thing.
Change-Id: I3308a8431c82f0b614e09ce3e5efac1526881f1e
Reviewed-on: https://boringssl-review.googlesource.com/3723
Reviewed-by: Adam Langley <agl@google.com>
This allows the current RC4 state of an SSL* to be extracted. We have
internal uses for this functionality.
Change-Id: Ic124c4b253c8325751f49e7a4c021768620ea4b7
Reviewed-on: https://boringssl-review.googlesource.com/3722
Reviewed-by: Adam Langley <agl@google.com>
This callback receives information about the ClientHello and can decide
whether or not to allow the handshake to continue.
Change-Id: I21be28335fa74fedb5b73a310ee24310670fc923
Reviewed-on: https://boringssl-review.googlesource.com/3721
Reviewed-by: Adam Langley <agl@google.com>
* Eliminate the possibility of multiple lock IDs having the same
value (CRYPTO_LOCK_FIPS2 and CRYPTO_LOCK_OBJ were both 40 prior to
this commit).
* Remove unused lock IDs.
* Automatically guarantee that lock IDs and lock names stay in sync.
Change-Id: If20e462db1285fa891595a7e52404ad011ff16f6
Reviewed-on: https://boringssl-review.googlesource.com/3923
Reviewed-by: Adam Langley <agl@google.com>
The TLS AEADs require EVP_AEAD_CTX_init_with_direction now. The speed command
was never updated to account for this.
BUG=467762
Change-Id: I8c7d4566b51d54d91d17318b8f4813ad8f19c777
Reviewed-on: https://boringssl-review.googlesource.com/3971
Reviewed-by: Adam Langley <agl@google.com>
Windows x64 uses the IL32P64 data model, which means that unsigned int
is 32 bits and size_t is 64 bits. Previously, the expression
|~(alignment - 1)| resulted in the 32-bit value 0xFFFFFFF0, which was
then extended to the 64-bit value 0x00000000FFFFFFF0 when promoted to
size_t. When the input pointer was masked with this value, the result
was a pointer that was usually way outside the boundaries of the array.
The new code casts |alignment| to size_t first prior to the bitwise
negation, resulting in the correct mask value of 0xFFFFFFFFFFFFFFF0.
Change-Id: I04754aa9e1ce7a615c2b4c74051cfcca38dbb52f
Reviewed-on: https://boringssl-review.googlesource.com/3961
Reviewed-by: Adam Langley <agl@google.com>
This mostly[*] doesn't matter for TLS since the message would have been
rejected anyway, but, in DTLS, if the peer rejects our Finished, it will send
an encrypted alert. This will then cause it to hang, which isn't very helpful.
I've made the change on both TLS and DTLS so the two protocols don't diverge on
this point. It is true that we're accepting nominally encrypted and
authenticated alerts before Finished, but, prior to ChangeCipherSpec, the
alerts are sent in the clear anyway so an attacker could already inject alerts.
A consumer could only be sensitive to it being post-CCS if it was watching
msg_callback. The only non-debug consumer of msg_callback I've found anywhere
is some hostapd code to detect Heartbeat.
See https://code.google.com/p/webrtc/issues/detail?id=4403 for an instance
where the equivalent behavior in OpenSSL masks an alert.
[*] This does change behavior slightly if the peer sends a warning alert
between CCS and Finished. I believe this is benign as warning alerts are
usually ignored apart from info_callback and msg_callback. The one exception is
a close_notify which is a slightly new state (accepting close_notify during a
handshake seems questionable...), but they're processed pre-CCS too.
Change-Id: Idd0d49b9f9aa9d35374a9f5e2f815cdb931f5254
Reviewed-on: https://boringssl-review.googlesource.com/3883
Reviewed-by: Adam Langley <agl@google.com>
May as well use this convenience function when we can. A little tidier. Even
fixes a leak on malloc failure in eckey_type2param.
Change-Id: Ie48dd98f2fe03fa9911bd78db4423ab9faefc63d
Reviewed-on: https://boringssl-review.googlesource.com/3772
Reviewed-by: Adam Langley <agl@google.com>
Upstream needs this to deal with size_t, but our low-level DES APIs take
size_t, so this is not a concern.
Change-Id: I9dc4c7248c5dd9515246a4b224147b932328a400
Reviewed-on: https://boringssl-review.googlesource.com/3882
Reviewed-by: Adam Langley <agl@google.com>
Upstream added another test vector in 4e049c52599d4a3fd918ba8570f49d88159e551b.
Change-Id: I17855dd479214657f0698b78f93e183cd6cb912e
Reviewed-on: https://boringssl-review.googlesource.com/3880
Reviewed-by: Adam Langley <agl@google.com>
Fix a few remnants of them being unsigned long. Also rename extremely unhelpful
variable names in SSL_get_error. i is now ret_code to match the header.
Change-Id: Ic31d6626bfe09c9e21c03691dfc716c5573833ea
Reviewed-on: https://boringssl-review.googlesource.com/3881
Reviewed-by: Adam Langley <agl@google.com>
Now that much of EVP has been normalized to 0/1, a lot of code can just use
boolean operators. (As can some code which was already using them...)
Change-Id: I6bb17edfd6f67050bf1706d59d8f37df57535faa
Reviewed-on: https://boringssl-review.googlesource.com/3875
Reviewed-by: Adam Langley <agl@google.com>
No code within BoringSSL or Google (grep for EVP_PKEY_CTX_(ctrl|get|set)) is
sensitive to the various failure cases. Normalize it all to 0/1 for simplicity.
This does carry a slight risk: any new ctrl hooks we import from upstream that,
like EVP_PKEY_CTX_get_rsa_oaep_md, return something other than success/failure
cannot be called directly via EVP_PKEY_CTX_ctrl. They instead need to
internally be routed through a struct like CBS and only called through the
wrappers. To that end, unexport EVP_PKEY_CTX_ctrl and require that callers use
the wrappers. No code in Google uses it directly and, if need be, switching to
the wrapper would be an incredibly upstreamable patch.
Change-Id: I3fd4e5a1a0f3d4d1c4122c52d4c74a5105b99cd5
Reviewed-on: https://boringssl-review.googlesource.com/3874
Reviewed-by: Adam Langley <agl@google.com>
This is the only EVP_PKEY ctrl hook which returns something other than a
boolean.
Change-Id: Ic226aef168abdf72e5d30e8264a559ed5039a055
Reviewed-on: https://boringssl-review.googlesource.com/3873
Reviewed-by: Adam Langley <agl@google.com>
This removes another place where we're internally sensitive to the
success/failure conditions.
Change-Id: I18fecf6457e841ba0afb718397b9b5fd3bbdfe4c
Reviewed-on: https://boringssl-review.googlesource.com/3872
Reviewed-by: Adam Langley <agl@google.com>
All EVP_PKEY types return 1 on that. (It can go away entirely when
EVP_PKEY_HMAC is gone.) This removes a place internally where we're sensitive
to the failure code.
Change-Id: Ic6cda2da9337ba7ef1c66a18e40c5dcc44fcf840
Reviewed-on: https://boringssl-review.googlesource.com/3871
Reviewed-by: Adam Langley <agl@google.com>
Some things were misindented in the reformatting.
Change-Id: I97642000452ce4d5b4c8a39b794cec13097d8760
Reviewed-on: https://boringssl-review.googlesource.com/3870
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's 687eaf27a7e4bdfc58dd455e2566b915a7a25c20. I don't think any
of the *Update functions can actually fail (we should verify this and, if
accurate, document it), but HMAC_Final can. It internally copies an EVP_MD_CTX.
Change-Id: I318cb9d0771d536249a26b61d34fe0413a4d3a10
Reviewed-on: https://boringssl-review.googlesource.com/3830
Reviewed-by: Adam Langley <agl@google.com>
signed/unsigned comparison. Just add a cast for now as in s3_both.c. Later
we'll properly size_t it alongside other tightening of this interface.
Change-Id: Idc8441d65e8ca65e39ab7172a8ec87d9ad710ed6
Reviewed-on: https://boringssl-review.googlesource.com/3860
Reviewed-by: Adam Langley <agl@google.com>
Imported from upstream's 918bb8652969fd53f0c390c1cd909265ed502c7e. (The
remainder is in code we've since deleted.)
Change-Id: Ie878272114086ba60a0fd5eae059b641b00ec0c4
Reviewed-on: https://boringssl-review.googlesource.com/3831
Reviewed-by: Adam Langley <agl@google.com>
The assert was supposed to be *added* in fcf25833 but instead replaced
the check.
BUG=465557
Change-Id: I0d3db5038515021e5bdd1ccb9ff08d4f78552621
Reviewed-on: https://boringssl-review.googlesource.com/3850
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It's never called or implemented.
Change-Id: Id41c2fbd23d27cc440e8a23ac1b2d590e50ff20f
Reviewed-on: https://boringssl-review.googlesource.com/3770
Reviewed-by: Adam Langley <agl@google.com>
None of these are version-specific. SSL_PROTOCOL_METHOD's interface will change
later, but this gets us closer to folding away SSL3_ENC_METHOD.
Change-Id: Ib427cdff32d0701a18fe42a52cdbf798f82ba956
Reviewed-on: https://boringssl-review.googlesource.com/3769
Reviewed-by: Adam Langley <agl@google.com>
Turn them into static functions that take in an hm_fragment. It's not
immediately obvious that the frag_off/frag_len bounds checks and the msg_len
consistency check are critical to avoiding an out-of-bounds write. Better to
have dtls1_hm_fragment_mark also check internally.
Also rework the bitmask logic to be clearer and avoid a table.
Change-Id: Ica54e98f66295efb323e033cb6c67ab21e7d6cbc
Reviewed-on: https://boringssl-review.googlesource.com/3765
Reviewed-by: Adam Langley <agl@google.com>
Replace unsigned long with the appropriate sized integer type.
Change-Id: I7b4641d84568f6c11efa25350a9e488a556fc92e
Reviewed-on: https://boringssl-review.googlesource.com/3766
Reviewed-by: Adam Langley <agl@google.com>
Notably, drop all special cases around receiving a message in order and
receiving a full message. It makes things more complicated and was the source
of bugs (the MixCompleteMessageWithFragments tests added in this CL did not
pass before). Instead, every message goes through an hm_fragment, and
dtls1_get_message always checks buffered_messages to see if the next is
complete.
The downside is that we pay one more copy of the message data in the common
case. This is only during connection setup, so I think it's worth the
simplicity. (If we want to optimize later, we could either tighten
ssl3_get_message's interface to allow the handshake data being in the
hm_fragment's backing store rather than s->init_buf or swap out s->init_buf
with the hm_fragment's backing store when a mesasge completes.
This CL does not address ssl_read_bytes being an inappropriate API for DTLS.
Future work will revise the handshake/transport boundary to align better with
DTLS's needs. Also other problems that I've left as TODOs.
Change-Id: Ib4570d45634b5181ecf192894d735e8699b1c86b
Reviewed-on: https://boringssl-review.googlesource.com/3764
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I582eaa2ff922bbf1baf298a5c6857543524a8d4e
Reviewed-on: https://boringssl-review.googlesource.com/3810
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
