Commit Graph

3451 Commits

Author SHA1 Message Date
Adam Langley
52f9f62283 Change aes_set_gcm_key to be more flexible.
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>
2015-03-18 21:16:39 +00:00
Adam Langley
5edc4e2a9b Correct three incorrect function names in errors.
Thanks to Sean Burford.

Change-Id: I4efa352f3e830c4c3761660508a1a8aa927eedf1
Reviewed-on: https://boringssl-review.googlesource.com/3841
Reviewed-by: Adam Langley <agl@google.com>
2015-03-18 21:15:04 +00:00
Adam Langley
ca8feeb301 Add support for 3DES ECB.
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>
2015-03-18 19:55:14 +00:00
Adam Langley
7282738bad Add DES_set_odd_parity.
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>
2015-03-18 19:54:53 +00:00
Adam Langley
3f92d21094 Add SSL_get_rc4_state.
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>
2015-03-18 19:54:34 +00:00
Adam Langley
524e717b87 Add a callback for DDoS protection.
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>
2015-03-18 19:53:29 +00:00
Brian Smith
ab2a8e03d9 Fix duplicate lock IDs & simplify lock ID maintenance.
* 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>
2015-03-17 23:58:20 +00:00
David Benjamin
d434f28ef2 Fix bssl speed.
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>
2015-03-17 23:57:02 +00:00
David Benjamin
1b3a95122d Document exactly how the stateful AEADs are weird.
Probably worth spelling that out.

Change-Id: Ie8815fc645e2836f4fb5bf7d09df1e5326751544
Reviewed-on: https://boringssl-review.googlesource.com/3970
Reviewed-by: Adam Langley <agl@google.com>
2015-03-17 23:55:56 +00:00
Brian Smith
1d1562d9b5 Free new[]'d array using delete[] instead of free in speed.cc.
Change-Id: Ic3d5e8de0b6800c0852c2057427836302c1f1aaa
Reviewed-on: https://boringssl-review.googlesource.com/3962
Reviewed-by: Adam Langley <agl@google.com>
2015-03-17 21:04:15 +00:00
Brian Smith
a42b4163f8 Remove remaining remnants of RIPEMD-160 support.
Change-Id: I59d06bcb9245ba93d3c3b63afbc24d6cef7c5af4
Reviewed-on: https://boringssl-review.googlesource.com/3925
Reviewed-by: Adam Langley <agl@google.com>
2015-03-17 21:03:42 +00:00
Brian Smith
d53b2c3c88 Fix out-of-bounds memory write in speed.cc.
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>
2015-03-17 19:12:54 +00:00
Brian Smith
4df48dd30f Define more OPENSSL_NO_xxx flags in opensslfeatures.h.
Change-Id: I464159dd03a2ea9a5e01452888fda86a5af51024
Reviewed-on: https://boringssl-review.googlesource.com/3924
Reviewed-by: Adam Langley <agl@google.com>
2015-03-14 00:21:01 +00:00
Brian Smith
949b26827a Remove Unused SPARC & PowerPC perlasm scripts.
Change-Id: I6139a3e9a2780c5a52169c72afc95665d453da8c
Reviewed-on: https://boringssl-review.googlesource.com/3922
Reviewed-by: Adam Langley <agl@google.com>
2015-03-14 00:16:55 +00:00
Brian Smith
28120a18f6 Fix mistaken use of CRYPTO_EX_INDEX_DSA in rsa.c.
Change-Id: I4a81fcbdd79f96b34cafcbd33279906b55dafbac
Reviewed-on: https://boringssl-review.googlesource.com/3920
Reviewed-by: Adam Langley <agl@google.com>
2015-03-14 00:14:40 +00:00
David Benjamin
dc3da93899 Process alerts between ChangeCipherSpec and Finished.
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>
2015-03-13 20:19:11 +00:00
David Benjamin
4f7783eaea Use EC_KEY_new_by_curve_name.
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>
2015-03-13 19:49:39 +00:00
David Benjamin
7cb0f44d9d Remove EVP_MAXCHUNK from DES EVP code.
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>
2015-03-13 19:46:49 +00:00
David Benjamin
165de16c2e Import additional AES-GCM test vector from upstream.
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>
2015-03-13 19:41:49 +00:00
David Benjamin
1a5c50f3a8 Error codes are uint32_t, not unsigned long.
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>
2015-03-13 19:41:42 +00:00
David Benjamin
7cc29ab833 Take advantage of normalized return values.
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>
2015-03-13 19:40:42 +00:00
David Benjamin
e0ba4dddf6 Normalize all EVP_PKEY_CTX_ctrl return values.
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>
2015-03-13 19:39:42 +00:00
David Benjamin
bc5d8ee0d4 Route EVP_PKEY_CTRL_GET_RSA_OAEP_LABEL through a CBS.
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>
2015-03-13 19:19:59 +00:00
David Benjamin
e60202797d Move EVP_R_COMMAND_NOT_SUPPORTED into individual EVP_PKEY ctrl hooks.
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>
2015-03-13 19:18:39 +00:00
David Benjamin
33c8d60a1f Remove EVP_PKEY_CTRL_DIGESTINIT -2 special-case in md_begin_digest.
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>
2015-03-13 19:17:59 +00:00
David Benjamin
b18f024816 Minor formatting tweaks.
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>
2015-03-13 19:17:23 +00:00
David Benjamin
bcd374570c Fix some missing return value checks in ssl3_send_new_session_ticket.
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>
2015-03-13 19:17:01 +00:00
David Benjamin
bf0df92964 Fix Windows build.
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>
2015-03-10 05:28:56 +00:00
David Benjamin
389939422a ARMv4 assembly pack: add Cortex-A15 performance data.
(Imported from upstream's e390ae50e0bc41676994c6fa23f7b65a8afc4d7f)

Change-Id: Ifee85b0936c06c42cc7c09f8327d15fec51da48a
Reviewed-on: https://boringssl-review.googlesource.com/3832
Reviewed-by: Adam Langley <agl@google.com>
2015-03-10 02:32:05 +00:00
David Benjamin
5a93342fab Fix two unchecked mallocs in crypto/asn1.
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>
2015-03-10 02:31:29 +00:00
Adam Langley
c4482d65dc Switch an assert back to a check.
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>
2015-03-10 02:15:50 +00:00
David Benjamin
f298019547 Remove ssl_version hook.
It's never called or implemented.

Change-Id: Id41c2fbd23d27cc440e8a23ac1b2d590e50ff20f
Reviewed-on: https://boringssl-review.googlesource.com/3770
Reviewed-by: Adam Langley <agl@google.com>
2015-03-10 01:16:54 +00:00
David Benjamin
2fa83de3f2 Move handshake method hooks to SSL_PROTOCOL_METHOD.
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>
2015-03-10 01:16:38 +00:00
David Benjamin
4c31123bfc Remove finish_mac_length from SSL3_ENC_METHOD.
It's unused.

Change-Id: I234c19990758ad761a72d5b0abe404a8583705a7
Reviewed-on: https://boringssl-review.googlesource.com/3768
Reviewed-by: Adam Langley <agl@google.com>
2015-03-10 01:16:17 +00:00
David Benjamin
f7f0f3a74b Remove ccs_header_st.
It doesn't do anything.

Change-Id: I0bba4e22a0b2053aae491d0ad695c892803cafca
Reviewed-on: https://boringssl-review.googlesource.com/3767
Reviewed-by: Adam Langley <agl@google.com>
2015-03-10 01:13:01 +00:00
David Benjamin
ee562b987e Get rid of the RSMBLY macros.
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>
2015-03-10 01:11:21 +00:00
David Benjamin
8cb7a7c0d5 Fix types of hm_header_st.
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>
2015-03-10 01:10:09 +00:00
David Benjamin
7538122ca6 Rework DTLS handshake message reassembly logic.
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>
2015-03-10 00:56:45 +00:00
David Benjamin
d03b5eddb9 Fix some stray tabs that snuck into e_tls.c.
Change-Id: Iee44636a3f80d86f94b0c34bb708d4d723964159
Reviewed-on: https://boringssl-review.googlesource.com/3820
Reviewed-by: Adam Langley <agl@google.com>
2015-03-10 00:55:22 +00:00
Adam Langley
8a4ab4fc1d Don't include .extern and .hidden on OS X.
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>
2015-03-06 21:44:19 +00:00
Adam Langley
d216b71f90 Include .extern and .hidden in x86-64 asm.
It's unclear why .extern was being suppressed, it's also a little
unclear how the Chromium build was working without this. None the less,
it's causing problems with Android and it's more obviously correct to
make these symbols as hidden.

Change-Id: Id13ec238b80b8bd08d8ae923ac659835450e77f8
Reviewed-on: https://boringssl-review.googlesource.com/3800
Reviewed-by: Adam Langley <agl@google.com>
2015-03-06 19:08:11 +00:00
David Benjamin
d27eda00a4 Generate error data at build time.
This avoids cluttering up the diff and making merge conflicts a pain.  It does,
however, mean we need to generate err_data.c ahead of time in Chromium and
likely other downstream builds. It also adds a build dependency on Go.

Change-Id: I6e0513ed9f50cfb030f7a523ea28519590977104
Reviewed-on: https://boringssl-review.googlesource.com/3790
Reviewed-by: Adam Langley <agl@google.com>
2015-03-06 18:59:25 +00:00
David Benjamin
f46cea8cd8 Fix the derivation of SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD.
It happens to give the same value anyway (64 + 16), but only on accident.

Change-Id: I1415f4015e3de472dbeb9ada0d92607c9d1bcd40
Reviewed-on: https://boringssl-review.googlesource.com/3780
Reviewed-by: Adam Langley <agl@google.com>
2015-03-06 18:56:57 +00:00
David Benjamin
a4e6d48749 runner: Move Finished special-case into dtlsWriteRecord.
We actually don't really care about this special-case since we only test client
full handshakes where the runner sends the second Finished not the shim
(otherwise the overlap logic and retransmitting on every fragment would
probably break us), but it should probably live next to the fragmentation
logic.

Change-Id: I54097d84ad8294bc6c42a84d6f22f496e63eb2a8
Reviewed-on: https://boringssl-review.googlesource.com/3763
Reviewed-by: Adam Langley <agl@google.com>
2015-03-06 18:55:58 +00:00
David Benjamin
7eaab4cd57 Only retransmit on Finished if frag_off == 0.
If the peer fragments Finished into multiple pieces, there is no need to
retransmit multiple times.

Change-Id: Ibf708ad079e1633afd420ff1c9be88a80020cba9
Reviewed-on: https://boringssl-review.googlesource.com/3762
Reviewed-by: Adam Langley <agl@google.com>
2015-03-06 18:55:47 +00:00
David Benjamin
9d102ddbc0 Check public key is not NULL.
CVE-2015-0288
PR#3708

(Imported from upstream's 28a00bcd8e318da18031b2ac8778c64147cd54f9.)

Change-Id: I6d60691da4c6860f87c753c065e93e9d0704aba9
Reviewed-on: https://boringssl-review.googlesource.com/3750
Reviewed-by: Adam Langley <agl@google.com>
2015-03-06 18:55:32 +00:00
Adam Langley
e023ad2d83 Fix SHA-384 and -512 in NO_ASM mode.
This transcription bug comes from the start of BoringSSL and, as you can
imagine, was a complete delight to track down.

Change-Id: I3051934195098a1d3bf893b154389ec7f14d3609
Reviewed-on: https://boringssl-review.googlesource.com/3740
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-03-05 22:44:00 +00:00
Adam Langley
5f5bf6f210 Don't add another error to the queue when RSA_private_transform fails.
Some code, sadly, tests the error and the extra error is breaking it.

Change-Id: I89eabadf5d2c5f7dd761030da33dd4c3f2ac8382
Reviewed-on: https://boringssl-review.googlesource.com/3720
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-03-05 22:43:43 +00:00
David Benjamin
0685b68216 Remove HelloRequest special-case in DTLS message parsing.
We don't support DTLS renego. Removing this separately from the rewrite to call
out intentionally dropping this logic.

Change-Id: Ie4428eea0d2dbbb8b4b8b6474df4821de62558cc
Reviewed-on: https://boringssl-review.googlesource.com/3761
Reviewed-by: Adam Langley <agl@google.com>
2015-03-05 21:26:55 +00:00
David Benjamin
5ca39fb50c Switch SSL_GET_MESSAGE_HASH_MESSAGE to an enum.
Matches the others.

Change-Id: If8a5164ed25f9e0bc495585bd705862a61a39fd6
Reviewed-on: https://boringssl-review.googlesource.com/3760
Reviewed-by: Adam Langley <agl@google.com>
2015-03-05 21:26:28 +00:00