Commit Graph

340 Commits

Author SHA1 Message Date
David Benjamin
c92c2d7a07 Prune some dead quirks and document the SSL_OP_ALL ones.
Update SSL_OP_ALL to account for SSL_OP_CRYPTOPRO_TLSEXT_BUG being gone,
and update ssl3_setup_write_buffer to account for SSL_MODE_CBC_RECORD_SPLITTING
rather than the now defunct SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS.

Also remove SSL_OP_TLS_BLOCK_PADDING_BUG. This is to allow for a buggy peer
which pads CBC with N bytes of value N rather than N+1 bytes of value N. This
quirk has been broken since CBC padding checks became constant-time, as
demonstrated by this attempt at a test. (Instead of just decrementing
padding_length, it needs to also keep track of a separate padding_value and not
decrement that one.)

https://boringssl-review.googlesource.com/#/c/1690/

(The quirk would also fall over anyway if the buggy client ever did a session
resumption; then the server speaks first rather than the client, and the quirk
triggered on reading the first encrypted record from the peer.)

Change-Id: I19942dc629a47832aead77a46bb50e0b0a9780b3
Reviewed-on: https://boringssl-review.googlesource.com/1694
Reviewed-by: Adam Langley <agl@google.com>
2014-09-03 20:17:45 +00:00
David Benjamin
859ec3cc09 Add SSL_CTX_set_keylog_bio.
Configures the SSL stack to log session information to a BIO. The intent is to
support NSS's SSLKEYLOGFILE environment variable. Add support for the same
environment variable to tool/client.cc.

Tested against Wireshark 1.12.0.

BUG=393477

Change-Id: I4c231f9abebf194eb2df4aaeeafa337516774c95
Reviewed-on: https://boringssl-review.googlesource.com/1699
Reviewed-by: Adam Langley <agl@google.com>
2014-09-03 20:15:55 +00:00
David Benjamin
df90a64483 Remove MD5, SHA-224, and SHA-512 handling from s3_cbc.c.
The only MD5 CBC-mode cipher suites are TLS_KRB5_WITH_DES_CBC_MD5,
TLS_KRB5_WITH_3DES_EDE_CBC_MD5, and TLS_KRB5_WITH_IDEA_CBC_MD5. We do not
support those, and it seems quite safe to assume that list will not grow.

No current cipher suites use SHA-224 or SHA-512 MACs. We can restore those
cases if that ever changes, but hopefully any future cipher suites we care
about will be using the AEAD construction.

Change-Id: I7f2d30238e2156a59b5fed1e48fabe6660fc9b67
Reviewed-on: https://boringssl-review.googlesource.com/1697
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 23:42:15 +00:00
David Benjamin
39ebf53dd3 Check the server did not use a TLS 1.2 cipher suite pre-TLS 1.2.
This check got refactored in OpenSSL 1.0.2 and broke in the process. Fix this
and add a test. Otherwise things like client auth can get slightly confused; it
will try to sign the MD5/SHA-1 hash, but the TLS 1.2 cipher suite may not use
SSL_HANDSHAKE_MAC_DEFAULT, so those digests won't be available.

Based on upstream's 226751ae4a1f3e00021c43399d7bb51a99c22c17.

Change-Id: I5b864d3a696f3187b849c53b872c24fb7df27924
Reviewed-on: https://boringssl-review.googlesource.com/1696
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 23:41:34 +00:00
David Benjamin
120a674c00 Fix the return values for most of SRTP.
Switch all of SRTP code to the standard return value convention with two
exceptions. Unfortunately, OpenSSL exposed API with the wrong error code. Keep
the public API flipped and document.

Change-Id: I43ac82513f4f52bb36a0b54aba9b9e0fa285730e
Reviewed-on: https://boringssl-review.googlesource.com/1691
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 23:41:22 +00:00
David Benjamin
5c24a1d6b6 Add a test for SSL_OP_TLS_D5_BUG.
If this is part of SSL_OP_ALL, we should have a test for it.

Change-Id: Ia72422beb2da6434726e78e174f3416f90f7c897
Reviewed-on: https://boringssl-review.googlesource.com/1695
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 22:43:54 +00:00
David Benjamin
f0fd37323b Remove remnants of EVP_CIPHER-based AES_GCM cipher.
Those codepaths are never hit.

Change-Id: Ib6908ebe90ab667774785298fdc3f96acc4b50df
Reviewed-on: https://boringssl-review.googlesource.com/1693
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 22:42:26 +00:00
Adam Langley
b2cb0ece76 Fix minor issues found by Clang's analysis.
Thanks to Denis Denisov for running the analysis.

Change-Id: I80810261e013423e746fd8d8afefb3581cffccc0
Reviewed-on: https://boringssl-review.googlesource.com/1701
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 22:39:41 +00:00
Adam Langley
ed8270a55c Fix crash as server when resuming with SNI.
Thanks to Denis Denisov for noting that |host_name| could be used while
uninitialised in the resumption case.

While in the area, this change also renames |servername_done| to
something more reasonable and removes a documented value that was never
used. Additionally, the SNI ack was only sent when not resuming so
calculating whether it should be sent when processing ClientHello
extensions (which is after s->hit has been set) is superfluous.

Lastly, since SNI is only acked by servers, there's no need to worry
about the SNI callback returning NOACK in the client case.

Change-Id: Ie4ecfc347bd7afaf93b12526ff9311cc45da4df6
Reviewed-on: https://boringssl-review.googlesource.com/1700
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 21:30:50 +00:00
Alex Chernyakhovsky
04dbb7f1d1 Add tests for pqueue
Reorder the tests in all_tests.sh to be in alphabetical order.

Change-Id: Idc6df6ab4a25709312a6f58635061bb643582c70
Reviewed-on: https://boringssl-review.googlesource.com/1680
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 20:09:23 +00:00
David Benjamin
6c7aed048c Client-side OCSP stapling support.
Remove the old implementation which was excessively general. This mirrors the
SCT support and adds a single boolean flag to request an OCSP response with no
responder IDs, extensions, or frills. The response, if received, is stored on
the SSL_SESSION so that it is available for (re)validation on session
resumption; Chromium revalidates the saved auth parameters on resume.

Server support is unimplemented for now. This API will also need to be adjusted
in the future if we implement RFC 6961.

Change-Id: I533c029b7f7ea622d814d05f934fdace2da85cb1
Reviewed-on: https://boringssl-review.googlesource.com/1671
Reviewed-by: Adam Langley <agl@google.com>
2014-08-29 00:39:33 +00:00
David Benjamin
e098ec2460 Test client auth under TLS 1.2 hash mismatch and SSL 3.
Maintain a handshake buffer in prf.go to implement TLS 1.2 client auth. Also
use it for SSL 3. This isn't strictly necessary as we know the hash functions,
but Go's hash.Hash interface lacks a Copy method.

Also fix the server-side tests which failed to test every TLS version.

Change-Id: I98492c334fbb9f2f0f89ee9c5c8345cafc025600
Reviewed-on: https://boringssl-review.googlesource.com/1664
Reviewed-by: Adam Langley <agl@google.com>
2014-08-29 00:23:50 +00:00
David Benjamin
041b58a668 Remove session_ctx macro.
Don't pollute the embedder's namespace with a session_ctx macro. It looks like
the difference was that, without TLS extensions, session_ctx was ctx rather
than initial_ctx. Now it's always initial_ctx. Retain the semantics of
switching SSL_CTX's out after the fact, until/unless we decide to replace that
with something less scary-sounding.

Change-Id: Ie5df5138aec25218ca80031cf645671968b8a54a
Reviewed-on: https://boringssl-review.googlesource.com/1663
Reviewed-by: Adam Langley <agl@google.com>
2014-08-28 00:42:20 +00:00
David Benjamin
457112e197 unifdef a bunch of OPENSSL_NO_* ifdefs.
Get all this stuff out of the way.

- OPENSSL_NO_MD5
- OPENSSL_NO_SHA
- OPENSSL_NO_EC
- OPENSSL_NO_ECDSA
- OPENSSL_NO_ECDH
- OPENSSL_NO_NEXTPROTONEG
- OPENSSL_NO_DH
- OPENSSL_NO_SSL3
- OPENSSL_NO_RC4
- OPENSSL_NO_RSA

Also manually removed a couple instances of OPENSSL_NO_DSA that seemed to be
confused anyway. Did some minor manual cleanup. (Removed a few now-pointless
'if (0)'s.)

Change-Id: Id540ba97ee22ff2309ab20ceb24c7eabe766d4c4
Reviewed-on: https://boringssl-review.googlesource.com/1662
Reviewed-by: Adam Langley <agl@google.com>
2014-08-28 00:41:34 +00:00
David Benjamin
854dd654d1 Refactor server-side CertificateVerify handling.
This moves CertificateVerify digest processing to the new
SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag. It also refactors it similarly to
ssl3_send_cert_verify and moves that logic to a common ssl3_cert_verify_hash
function to compute the handshake hash.

This removes a large chunk of duplicate (and divergent!) logic between TLS and
DTLS. It also removes TLS1_FLAGS_KEEP_HANDSHAKE.

Change-Id: Ia63c94f7d76d901bc9c4c33454fbfede411adf63
Reviewed-on: https://boringssl-review.googlesource.com/1633
Reviewed-by: Adam Langley <agl@google.com>
2014-08-27 01:55:27 +00:00
David Benjamin
5b8f104ee8 Revise hash management for reading the Finished message.
Upstream originally sampled the Finished message's hash at ChangeCipherSpec,
but our patches to add messages between the two complicated this. Move DTLS to
this path, but use the new SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag to avoid
special-casing message types in ssl3_get_message.

Change-Id: I9c8ddd9cc500c94dff2ec2f696f89d50ab01b3ad
Reviewed-on: https://boringssl-review.googlesource.com/1632
Reviewed-by: Adam Langley <agl@google.com>
2014-08-27 01:55:17 +00:00
David Benjamin
880b14e98c Compute the Channel ID hash after ssl_get_message.
This avoids needing the save the hash on the SSL* (and use some field for two
purposes). Instead, use the new SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag (which
actually was already used here, but at the time, pointlessly). Also fix a minor
bug where the hash would be recomputed in non-blocking mode because init_num
may stay zero for a few state machine iterations.

Change-Id: I3d8331cf3134c5f9a3eda9e988bba5bcebe40933
Reviewed-on: https://boringssl-review.googlesource.com/1631
Reviewed-by: Adam Langley <agl@google.com>
2014-08-27 01:55:06 +00:00
David Benjamin
590cbe970c Introduce a hash_message parameter to ssl_get_message.
This replaces the special-case in ssl3_get_message for Channel ID. Also add
ssl3_hash_current_message to hash the current message, taking TLS vs DTLS
handshake header size into account.

One subtlety with this flag is that a message intended to be processed with
SSL_GET_MESSAGE_DONT_HASH_MESSAGE cannot follow an optional message
(reprocessed with reuse_message, etc.).  There is an assertion to that effect.
If need be, we can loosen it to requiring that the preceeding optional message
also pass SSL_GET_MESSAGE_DONT_HASH_MESSAGE and then maintain some state to
perform the more accurate assertion, but this is sufficient for now.

Change-Id: If8c87342b291ac041a35885b9b5ee961aee86eab
Reviewed-on: https://boringssl-review.googlesource.com/1630
Reviewed-by: Adam Langley <agl@google.com>
2014-08-27 01:54:50 +00:00
Adam Langley
3cb50e0699 Move a variable declaration to function start.
(Fixes the Windows build.)

Change-Id: Ibc7e3fe7cee275c8bc24198e996e4f92100ea9fc
Reviewed-on: https://boringssl-review.googlesource.com/1642
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 22:07:33 +00:00
David Benjamin
8da990677b Rename some message functions for consistency.
Make the get/send functions match.

ssl3_client_hello -> ssl3_send_client_hello.
ssl3_send_newsession_ticket -> ssl3_send_new_session_ticket.
ssl3_send_client_verify -> ssl3_send_cert_verify

Change-Id: Iea5579479b8a8f392167b8fb3b7e9fe961d0f007
Reviewed-on: https://boringssl-review.googlesource.com/1613
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 21:09:40 +00:00
David Benjamin
e58c4f5321 Add a test to ensure False Start occurs.
This adds the missing test coverage for
7e3305eebd.

Change-Id: I8c9f1dc998afa9bb1f6fb2a7872a651037bb4844
Reviewed-on: https://boringssl-review.googlesource.com/1610
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 17:41:53 +00:00
David Benjamin
ef86550537 Remove logic for non-signing client certificates.
Now that only RSA and ECDSA certificates are supported, the server should just
reject non-signing ones outright, rather than allowing them to skip
CertificateVerify.

Change-Id: I7fe5ed3adde14481016ee841ed241faba18c26f0
Reviewed-on: https://boringssl-review.googlesource.com/1609
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 17:41:08 +00:00
David Benjamin
a08e49d17a Add basic TLS Channel ID tests.
Change-Id: I7ccf2b8282dfa8f3985775e8b67edcf3c2949752
Reviewed-on: https://boringssl-review.googlesource.com/1606
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 17:40:36 +00:00
David Benjamin
4e99c52bf6 Fix FalseStart-SessionTicketsDisabled tests.
They weren't inheriting async settings.

Change-Id: I5e9c04914926910dce63f93462cce4024627fb26
Reviewed-on: https://boringssl-review.googlesource.com/1605
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 23:00:59 +00:00
David Benjamin
2561dc335a Introduce a mechanism for base64 options.
We may wish to pass data to the runner that contains NULs.

Change-Id: Id78dad0ad0b5b6d0537481c818e3febdf1740cc9
Reviewed-on: https://boringssl-review.googlesource.com/1603
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 23:00:44 +00:00
David Benjamin
3a66e2838c Update ssl3_get_channel_id for the simpler ECDSA_do_verify.
The return values are now 1/0, not 1/0/-1.

Change-Id: If65bb08a229c7944cb439ec779df461904d0ec19
Reviewed-on: https://boringssl-review.googlesource.com/1607
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 22:54:26 +00:00
David Benjamin
d30a990850 Implement TLS Channel ID in runner.go
Change-Id: Ia349c7a7cdcfd49965cd0c4d6cf81a76fbffb696
Reviewed-on: https://boringssl-review.googlesource.com/1604
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 22:48:18 +00:00
David Benjamin
f9b96fa493 Fix flipped DTLS checks.
09bd58d1f1 flipped a condition. Doing that
memset in the DTLS case breaks retransmits across a CCS and fails to memset in
the TLS case.

Strangely, it didn't break any tests, but I think that's a function of us
lacking renego tests. The sequence number doesn't seem to be used in the
initial handshake for TLS, so it stayed at zero. After a renego, that codepath
is relevant.

Change-Id: I369a524021857a82e181af7798c7a10fe6279550
Reviewed-on: https://boringssl-review.googlesource.com/1601
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 21:49:36 +00:00
David Benjamin
2a0c496ab3 Fix duplicate test name.
Change-Id: I16be575e4a6a13c74bd45a8fe3e1473502a80c86
Reviewed-on: https://boringssl-review.googlesource.com/1600
Reviewed-by: Adam Langley <agl@google.com>
2014-08-25 21:47:25 +00:00
Håvard Molland
9169c96458 Implement client side of TLS signed certificate stamps extension.
https://crbug.com/389420 and 3.3 in rfc6962.

Change-Id: Ib22bcd4e4bde5a314ed33e123e19a76cdb714da4
Reviewed-on: https://boringssl-review.googlesource.com/1491
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-08-21 16:21:46 +00:00
David Benjamin
c44d2f4cb8 Convert all zero-argument functions to '(void)'
Otherwise, in C, it becomes a K&R function declaration which doesn't actually
type-check the number of arguments.

Change-Id: I0731a9fefca46fb1c266bfb1c33d464cf451a22e
Reviewed-on: https://boringssl-review.googlesource.com/1582
Reviewed-by: Adam Langley <agl@google.com>
2014-08-21 01:06:07 +00:00
David Benjamin
5213df4e9e Prefer AES-GCM when hardware support is available.
BUG=396787

Change-Id: I72ddb0ec3c71dbc70054403163930cbbde4b6009
Reviewed-on: https://boringssl-review.googlesource.com/1581
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 20:53:31 +00:00
David Benjamin
92909a6206 Remove MS SGC support on the the server.
It doesn't appear to have ever been implemented on the client. The server code
stopped working anyway because it now skips the ssl_get_message call, so we
never cash in on the reuse_message, attempt to reprocess the repeated
ClientHello, and reject it thinking it's a second MS SGC restart.

Change-Id: Id536846e08460143f6fc0a550bdcc1b26b506b04
Reviewed-on: https://boringssl-review.googlesource.com/1580
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 19:06:01 +00:00
David Benjamin
b52e3dded2 Move common code between two branches.
Some follow-up cleanup to bd30f8e34a.

Change-Id: Id996c6c43f42f4db410e1e612ee1e5e8e0233356
Reviewed-on: https://boringssl-review.googlesource.com/1569
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 19:05:02 +00:00
David Benjamin
422d3a4015 Remove some unused state and code.
Change-Id: I1f917d1fe70ef5046eeea8fed27cc402bcd674b9
Reviewed-on: https://boringssl-review.googlesource.com/1568
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 19:04:32 +00:00
David Benjamin
a7d1363fcb Prune removed key types from SSL_PKEY_*.
Remove all the logic managing key types that aren't being used anymore.

Change-Id: I101369164588048e64ba1c84a6b8aac8f3a221cd
Reviewed-on: https://boringssl-review.googlesource.com/1567
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:15:32 +00:00
David Benjamin
ef2116d33c Remove DSA-based cipher suites and client auth.
DSA is not connected up to EVP, so it wouldn't work anyway. We shouldn't
advertise a cipher suite we don't support. Chrome UMA data says virtually no
handshakes end up negotiating one of these.

Change-Id: I874d934432da6318f05782ebd149432c1d1e5275
Reviewed-on: https://boringssl-review.googlesource.com/1566
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:14:40 +00:00
David Benjamin
cff6472442 Mark some more globals as const.
Change-Id: Ie6f3a3713ce1482a787444678a65daa37bc0b273
Reviewed-on: https://boringssl-review.googlesource.com/1565
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:13:09 +00:00
David Benjamin
0da0e18a60 Remove ECDH_RSA, ECDH_ECDSA, DH_RSA, and DH_DSS.
These are the variants where the CA signs a Diffie-Hellman keypair. They are
not supported by Chrome on NSS.

Change-Id: I569a7ac58454bd3ed1cd5292d1f98499012cdf01
Reviewed-on: https://boringssl-review.googlesource.com/1564
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:12:38 +00:00
David Benjamin
bd30f8e34a Remove support on both sides for *_fixed_(ec)dh client auth.
In the fixed_ecdh case, it wasn't even implemented, but there was stub code for
it. It complicates the ClientKeyExchange (the client parameters become implicit
in the certificate) and isn't used.

Change-Id: I3627a37042539c90e05e59cd0cb3cd6c56225561
Reviewed-on: https://boringssl-review.googlesource.com/1563
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:07:00 +00:00
David Benjamin
d0639af978 Remove single-DES cipher suites.
This also removes the 'LOW' strength class.

Change-Id: Iffd2356dadb4a4875c1547a613d51061101358fd
Reviewed-on: https://boringssl-review.googlesource.com/1562
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:02:57 +00:00
David Benjamin
ebf42b5f6f Remove remnants of now-removed cipher suites.
NULL, SRP, CAMELLIA, export ciphers, SSLv2, IDEA, and SEED are gone. Unknown
directives are silently ignored in the parser, so there is no need to retain
their masks and entries in the cipher suite aliases.

Change-Id: If43b9cbce56b3e1c401db764b88996940452a300
Reviewed-on: https://boringssl-review.googlesource.com/1561
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:02:17 +00:00
Adam Langley
e7bf281be1 Fix (harmless) memory leak in the test harness.
Change-Id: Ia0daaaaf464cfa0e9d563d7f376ce2bb2e338685
Reviewed-on: https://boringssl-review.googlesource.com/1560
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-08-19 19:01:15 +00:00
David Benjamin
ede973a89a Tidy up cipher ordering.
To align with what Chrome sends on NSS, remove all 3DES cipher suites except
RSA_WITH_3DES_EDE_CBC_SHA. This avoids having to order a PFS 3DES cipher
against a non-PFS 3DES cipher.

Remove the strength sort which wanted place AES_256_CBC ahead of AES_128_GCM
and is not especially useful (everything under 128 is either 3DES or DES).
Instead, explicitly order all the bulk ciphers. Continue to prefer PFS over
non-PFS and ECDHE over DHE.

This gives the following order in Chromium. We can probably prune it a bit
(DHE_DSS, DH_*) in a follow-up.

TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc14)   Forward Secrecy	256
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc13)   Forward Secrecy	256
TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc15)   Forward Secrecy	256
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)   Forward Secrecy	128
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b)   Forward Secrecy	128
TLS_DHE_DSS_WITH_AES_128_GCM_SHA256 (0xa2)   Forward Secrecy*	128
TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (0x9e)   Forward Secrecy	128
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014)   Forward Secrecy	256
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a)   Forward Secrecy	256
TLS_DHE_RSA_WITH_AES_256_CBC_SHA (0x39)   Forward Secrecy	256
TLS_DHE_DSS_WITH_AES_256_CBC_SHA (0x38)   Forward Secrecy*	256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013)   Forward Secrecy	128
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009)   Forward Secrecy	128
TLS_DHE_RSA_WITH_AES_128_CBC_SHA (0x33)   Forward Secrecy	128
TLS_DHE_DSS_WITH_AES_128_CBC_SHA (0x32)   Forward Secrecy*	128
TLS_ECDHE_RSA_WITH_RC4_128_SHA (0xc011)   Forward Secrecy	128
TLS_ECDHE_ECDSA_WITH_RC4_128_SHA (0xc007)   Forward Secrecy	128
TLS_DH_DSS_WITH_AES_128_GCM_SHA256 (0xa4)	128
TLS_DH_RSA_WITH_AES_128_GCM_SHA256 (0xa0)	128
TLS_RSA_WITH_AES_128_GCM_SHA256 (0x9c)	128
TLS_DH_RSA_WITH_AES_256_CBC_SHA (0x37)	256
TLS_DH_DSS_WITH_AES_256_CBC_SHA (0x36)	256
TLS_RSA_WITH_AES_256_CBC_SHA (0x35)	256
TLS_DH_RSA_WITH_AES_128_CBC_SHA (0x31)	128
TLS_DH_DSS_WITH_AES_128_CBC_SHA (0x30)	128
TLS_RSA_WITH_AES_128_CBC_SHA (0x2f)	128
TLS_RSA_WITH_RC4_128_SHA (0x5)	128
TLS_RSA_WITH_RC4_128_MD5 (0x4)	128
TLS_RSA_WITH_3DES_EDE_CBC_SHA (0xa)	112

BUG=405091

Change-Id: Ib8dd28469414a4eb496788a57a215e7e21f8c37f
Reviewed-on: https://boringssl-review.googlesource.com/1559
Reviewed-by: Adam Langley <agl@google.com>
2014-08-19 19:00:32 +00:00
David Benjamin
e712778a7b Remove rsa_md5, md5, and sha1 fields from SSL_CTX.
Just use the normal API for them.

Change-Id: Ibb5988611a86e8d39abda1e02087523d98defb51
Reviewed-on: https://boringssl-review.googlesource.com/1555
Reviewed-by: Adam Langley <agl@google.com>
2014-08-19 18:18:36 +00:00
David Benjamin
8bc38f556a DTLS version negotiation doesn't happen at HelloVerifyRequest.
RFC 6347 changed the meaning of server_version in HelloVerifyRequest. It should
now always be 1.0 with version negotiation not happening until ServerHello. Fix
runner.go logic and remove #if-0'd code in dtls1_get_hello_verify.

Enforce this in the runner for when we get DTLS 1.2 tests.

Change-Id: Ice83628798a231df6bf268f66b4c47b14a519386
Reviewed-on: https://boringssl-review.googlesource.com/1552
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 18:07:43 +00:00
David Benjamin
f2fedefdca Simplify HelloVerifyRequest processing.
Rather than switching the order of the ServerHello and HelloVerifyRequest
states and processing each twice, have the states follow the protocol order.
HelloVerifyRequest reading is optional and ServerHello is strict. Use the
send_cookie bit to determine whether we're expecting a cookie or not.

Fix the dtls1_stop_timer call in these states to consistently hit the end of a
server flight; the previous flight should not be cleared from the retransmit
buffer until the entire next flight is received. That said, OpenSSL doesn't
appear to implement the part where, on receipt of the previous peer flight, the
buffered flight is retransmitted. (With the exception of a SSL3_MT_FINISHED
special-case in dtls1_read_bytes.) So if the peer is also OpenSSL, this doesn't
do anything.

Also fix the DTLS test which wasn't actually asserting that the ClientHello
matched.

Change-Id: Ia542190972dbffabb837d32c9d453a243caa90b2
Reviewed-on: https://boringssl-review.googlesource.com/1551
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 18:05:10 +00:00
David Benjamin
cc23df53da Remove SSL_OP_CISCO_ANYCONNECT.
I see no internal users and the existence of a THIRD version encoding
complicates all version-checking logic. Also convert another version check to
SSL_IS_DTLS that was missed earlier.

Change-Id: I60d215f57d44880f6e6877889307dc39dbf838f7
Reviewed-on: https://boringssl-review.googlesource.com/1550
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 17:57:01 +00:00
David Benjamin
6f2600199c Mark all SSL_CIPHERs as const.
This lets us put the SSL_CIPHER table in the data section. For type-checking,
make STACK_OF(SSL_CIPHER) cast everything to const SSL_CIPHER*.

Note that this will require some changes in consumers which weren't using a
const SSL_CIPHER *.

Change-Id: Iff734ac0e36f9e5c4a0f3c8411c7f727b820469c
Reviewed-on: https://boringssl-review.googlesource.com/1541
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 17:55:05 +00:00
David Benjamin
fb4ea28bb8 Tidy DTLS cookie callback types.
const-correctness, unsigned long -> size_t.

Change-Id: Ic0c2685a48a0f98396c5753b6077c6c0c3b92326
Reviewed-on: https://boringssl-review.googlesource.com/1540
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 17:52:21 +00:00
David Benjamin
f4501347c9 Remove default_timeout hook.
Of the remaining implementations left, ssl3_, dtls1_, and ssl23_, dtls1_ is
redundant and can be folded into ssl3_. ssl23_ actually isn't; it sets 5
minutes rather than 2 hours. Two hours seems to be what everything else uses
and seems a saner default. Most consumers seem to override it anyway
(SSL_CTX_set_timeout). But it is a behavior change.

The method is called at two points:
- SSL_get_default_timeout
- SSL_CTX_new

Incidentally, the latter call actually makes the former never called internally
and the value it returns a lie. SSL_get_default_timeout returns the default
timeout of the /current/ method, but in ssl_get_new_session, the timeout is
shadowed by session_timeout on the context. That is initialized when
SSL_CTX_new is called. So, unless you go out of your way to
SSL_CTX_set_timeout(0), it always overrides. (And it actually used to a
difference because, for SSL23, the SSL_CTX's method is SSL23, but, when session
creation happens, the SSL's method is the version-specific one.)

Change-Id: I331d3fd69b726242b36492402717b6d0b521c6ee
Reviewed-on: https://boringssl-review.googlesource.com/1521
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 17:25:20 +00:00
David Benjamin
d4178fd9d8 Remove AES-GCM cipher indices.
Those ciphers go through EVP_AEAD now.

Change-Id: Ia97af9960223724f041dc2c249def9e626fd03f8
Reviewed-on: https://boringssl-review.googlesource.com/1520
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 22:02:42 +00:00
David Benjamin
b00eee1935 Trim unused cipher indices and definitions.
Also remove SSL_eNULL ciphers. They were broken anyway in the initial import
because of a lost 'else', but just remove them altogether.

Change-Id: Ie71cf1b45f8fc6883e209801443eddf7f2d058ba
Reviewed-on: https://boringssl-review.googlesource.com/1518
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 22:01:43 +00:00
David Benjamin
335d10d201 Remove Suite B mode.
It was added in OpenSSL 1.0.2, so nothing can be depending on it yet. If we
really want a Suite B profile, it seems better to generate a configuration for
the rest of the system rather than pepper the codebase with checks.

Change-Id: I1be3ebed0e87cbfe236ade4174dcf5bbc7e10dd5
Reviewed-on: https://boringssl-review.googlesource.com/1517
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 22:00:16 +00:00
David Benjamin
76d8abe7fd Get SSL 3.0 server tests working.
The missing SSL 3.0 client support in runner.go was fairly minor.

Change-Id: Ibbd440c9b6be99be08a214dec6b93ca358d8cf0a
Reviewed-on: https://boringssl-review.googlesource.com/1516
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 21:42:36 +00:00
David Benjamin
6fd297bb62 Add initial DTLS tests.
Change-Id: I7407261bdb2d788c879f2e67e617a615d9ff8f8b
Reviewed-on: https://boringssl-review.googlesource.com/1505
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 16:55:45 +00:00
Adam Langley
82b7da271f Set SSL_SESSION.cipher when parsing.
This was done for the server when parsing a session ticket, but it
wasn't done in the parsing function itself. That caused problems when
high level code used the parsing function directly to set a session for
the client code.

See comments in internal bug 7091840.

Change-Id: Iaa048c3df62cd9fe7a003af33805819e2556960a
2014-08-14 09:42:46 -07:00
Adam Langley
0f4746e207 Add SSL_SESSION_get_version.
Android uses this and added it to their OpenSSL in
  https://android.googlesource.com/platform/external/openssl.git/+/master/patches/0003-jsse.patch

Change-Id: Ib7985d51752ad8e7b75480513c3ab582a30f398a
2014-08-14 09:42:46 -07:00
David Benjamin
1b96526c6f Merge tls1_* method hooks with the ssl3_* versions.
The protocols are pretty similar; they were all basically redundant. The free
of s->tlsext_session_ticket (more fallout from the EAP-FAST patch) was moved to
SSL_free because that object's attached to s, not s->s3. This is relevant if
SSL_set_ssl_method gets called.

Change-Id: I14a896ba8a6a2c34ab1cb5f65311b117051228da
Reviewed-on: https://boringssl-review.googlesource.com/1509
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 00:18:12 +00:00
David Benjamin
5a593af42a Move configuration into a dedicated TestConfig struct.
This removes some duplicate code in parsing command-line flags and, more
importantly, makes configuration available when constructing the SSL_CTX and
avoids a number of globals.

Change-Id: I26e2d2285b732f855a2c82752bc8e0db480c3b30
Reviewed-on: https://boringssl-review.googlesource.com/1502
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 23:44:50 +00:00
David Benjamin
83c0bc94d7 Test-only DTLS implementation in runner.go.
Run against openssl s_client and openssl s_server. This seems to work for a
start, although it may need to become cleverer to stress more of BoringSSL's
implementation for test purposes.

In particular, it assumes a reliable, in-order channel. And it requires that
the peer send handshake fragments in order. Retransmit and whatnot are not
implemented. The peer under test will be expected to handle a lossy channel,
but all loss in the channel will be controlled. MAC errors, etc., are fatal.

Change-Id: I329233cfb0994938fd012667ddf7c6a791ac7164
Reviewed-on: https://boringssl-review.googlesource.com/1390
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 23:43:38 +00:00
David Benjamin
51e3283d62 Port dtls1_get_hello_verify to CBS.
Gives bounds checks and asserts that there's nothing after the cookie.

Change-Id: I8f9753e0c72670e9960f73a5722cefd9c02696a9
Reviewed-on: https://boringssl-review.googlesource.com/1507
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 21:59:51 +00:00
David Benjamin
9174312be6 Update d1_clnt.c to use ssl_cipher_has_server_public_key.
Mirror the changes in s3_clnt.c.

Change-Id: I7af7080c6eea2a67cc994befa11e45d32eaa9615
Reviewed-on: https://boringssl-review.googlesource.com/1506
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 21:59:02 +00:00
David Benjamin
6491e8c267 Allow renewed tickets on session resumption in DTLS.
Analogous fix for DTLS as upstream's c519e89f5c359b8c0f747519773284d9b6382791.

Change-Id: I8a56070ce2a1edf4e9ceb2fd8ce08552e25a1cf3
Reviewed-on: https://boringssl-review.googlesource.com/1504
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 21:58:43 +00:00
David Benjamin
09bd58d1f1 Replace some DTLS version checks with SSL_IS_DTLS.
They weren't updated to account for DTLS 1.2.

Change-Id: I81b3bfcb84a46d7b233bb567976a7de37bc46b92
Reviewed-on: https://boringssl-review.googlesource.com/1503
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 21:58:03 +00:00
Håvard Molland
16c623b83d Fix of handling weak dh server keys.
https://crbug.com/353579

Align behavior with NSS and report SSL_R_BAD_DH_P_LENGTH error
when size of the server's dh group is less than 512 bits.

Change-Id: I09f1828482f40b2283f7c6a69425819379399815
Reviewed-on: https://boringssl-review.googlesource.com/1480
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 17:59:59 +00:00
Håvard Molland
584d28cad6 Use OPENSSL_free instead of free.
Even if OPENSSL_free() now simply is defined to free(), it is
still nice to consistently use OPENSSL_free, so that they can
easily be replaced. Many embedded platforms still have slow
allocation and free functions.

Change-Id: Ie8781591311f12c7f69206dbad6fc4a0c89d88b7
Reviewed-on: https://boringssl-review.googlesource.com/1490
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 17:02:09 +00:00
Adam Langley
f1df2b3fbf Add SSL_CIPHER_get_kx_name.
This is needed by Android because it passes this string to a handshake
callback. It's implemented in Android's OpenSSL in this patch:
  https://android.googlesource.com/platform/external/openssl.git/+/master/patches/0003-jsse.patch

(Note that it's called |SSL_authentication_method| there.)

I didn't format this function in OpenSSL style because it's crazy and
because we'll probably clang-format ssl/ soon.

Change-Id: I865540511b50859c339da5d76ce37810449aa444
2014-08-12 14:13:12 -07:00
Adam Langley
8eaaa86da3 Add mode to disallow session creation.
Android needs this and it was patched into their OpenSSL in
  https://android.googlesource.com/platform/external/openssl.git/+/master/patches/0003-jsse.patch

It appears that this is needed because javax.net.ssl.SSLEngine has it as
part of its interface and thus it's part of the Android API. No idea why
anything would ever want to disable that though.

Change-Id: I9c6279a961637f44936889edbe269b9d5c19746d
2014-08-12 14:11:35 -07:00
David Benjamin
98e882ead1 Clean up s23_srvr.c.
ssl23_get_client_hello has lots of remnants of SSLv2 support and remnants of an
even older SSL_OP_NON_EXPORT_FIRST option (see upstream's
d92f0bb6e9ed94ac0c3aa0c939f2565f2ed95935) which complicates the logic.

Split it into three states and move V2ClientHello parsing into its own
function. Port it to CBS and CBB to give bounds checks on the V2ClientHello
parse.

This fixes a minor bug where, if the SSL_accept call in ssl23_get_client_hello
failed, cb would not be NULL'd and SSL_CB_ACCEPT_LOOP would get reported an
extra time.

It also unbreaks the invariant between s->packet, s->packet_length,
s->s3->rbuf.buf, and s->s3->rbuf.offset at the point the switch, although this
was of no consequence because the first ssl3_read_n call passes extend = 0
which resets s->packet and s->packet_length.

It also makes us tolerant to major version bumps in the ClientHello. Add tests
for TLS tolerance of both minor and major version bumps as well as the HTTP
request error codes.

Change-Id: I948337f4dc483f4ebe1742d3eba53b045b260257
Reviewed-on: https://boringssl-review.googlesource.com/1455
Reviewed-by: Adam Langley <agl@google.com>
2014-08-12 21:10:56 +00:00
Adam Langley
ded93581f1 Windows build fixes.
Windows doesn't have ssize_t, sadly. There's SSIZE_T, but defining an
OPENSSL_SSIZE_T seems worse than just using an int.

Change-Id: I09bb5aa03f96da78b619e551f92ed52ce24d9f3f
Reviewed-on: https://boringssl-review.googlesource.com/1352
Reviewed-by: Adam Langley <agl@google.com>
2014-08-11 22:10:02 +00:00
Håvard Molland
90974e7081 Fixed reporting wrong function to OPENSSL_PUT_ERROR.
ssl3_send_client_key_exchange were wrongly reported
by ssl3_send_client_certificate() and
ssl3_check_cert_and_algorithm()

Change-Id: I244d3d871b6b4f75a188fd386d52ffc4335d1f9b
Reviewed-on: https://boringssl-review.googlesource.com/1460
Reviewed-by: Adam Langley <agl@google.com>
2014-08-11 18:46:56 +00:00
David Benjamin
1e7f8d70ec Move SendV2ClientHello to handshake coverage tests.
It's a different handshake flow with more state machine coverage. We should
make sure to test the asynchronous version.

Change-Id: I0bb79ca7e6a86bd3cac66bac1f795a885d474909
Reviewed-on: https://boringssl-review.googlesource.com/1454
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:40:18 +00:00
David Benjamin
bed9aae757 Add RenewTicketOnResume tests.
Didn't have coverage for abbreviated handshakes with NewSessionTicket. Also add
some missing resumeSession flags so the tests match the comments.

Change-Id: Ie4d76e8764561f3f1f31e1aa9595324affce0db8
Reviewed-on: https://boringssl-review.googlesource.com/1453
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:39:57 +00:00
David Benjamin
9821454f2b Add tests for CVE-2014-3511.
Also change MaxHandshakeRecordLength to 1 in the handshake coverage tests to
better stress the state machine.

Change-Id: I27fce2c000b3d4818fd2e9a47fb09d3f646dd1bd
Reviewed-on: https://boringssl-review.googlesource.com/1452
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:39:47 +00:00
David Benjamin
7e2e6cf1a0 Add test coverage for TLS version negotiation.
Test all pairs of client and server version, except for the ones that require
SSLv3 client support in runner.go. That is, as yet, still missing.

Change-Id: I601ab49c5526cd2eb4f85d5d535570e32f218d5b
Reviewed-on: https://boringssl-review.googlesource.com/1450
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:39:16 +00:00
David Benjamin
0fecacd46d Fix protocol downgrade bug in case of fragmented packets
CVE-2014-3511

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Bodo Möller <bodo@openssl.org>

(Imported from upstream's 280b1f1ad12131defcd986676a8fc9717aaa601b)

Change-Id: I39894b340d2a03751b221631f8cd0ee220cf5b67
Reviewed-on: https://boringssl-review.googlesource.com/1451
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:08:41 +00:00
David Benjamin
a8e3e0e936 Remove SSL_OP_TLS_ROLLBACK_BUG.
It's not part of SSL_OP_ALL and is unused, so remove it. Add a test that
asserts the version check works.

Change-Id: I917516594ec5a4998a8316782f035697c33d99b0
Reviewed-on: https://boringssl-review.googlesource.com/1418
Reviewed-by: Adam Langley <agl@google.com>
2014-08-08 17:04:53 +00:00
Emilia Käsper
c3d79605ab Fix DTLS anonymous EC(DH) denial of service
(This change originally applied to 1.0.1. In the switch to 1.0.2, the
DTLS specific client processing was removed and now the s3_clnt.c
functions are used. This caused most of the patch to be moot. What
remains is still useful however. For the original patch, see the change
against 1.0.1: 88ae012c8092852f03c50f6461175271104b4c8a)

CVE-2014-3510

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>

(Imported from upstream's 1d7d0ed9c21403d79d602b6c7d76fdecf5e737da)

Change-Id: I666f9c48d603f2366cab821ae446a57360c3026b
Reviewed-on: https://boringssl-review.googlesource.com/1439
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:13:45 +00:00
Adam Langley
5ba06a7532 Fix race condition in ssl_parse_serverhello_tlsext
CVE-2014-3509

(Imported from upstream's 92aa73bcbfad44f9dd7997ae51537ac5d7dc201e)

Change-Id: Ibc681897251081ae5ebfea0ff6ca9defd73fe0f5
Reviewed-on: https://boringssl-review.googlesource.com/1441
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:11:23 +00:00
Adam Langley
abae631fb9 Remove some duplicate DTLS code.
In a couple of functions, a sequence number would be calculated twice.

Additionally, in |dtls1_process_out_of_seq_message|, we know that
|frag_len| <= |msg_hdr->msg_len| so the later tests for |frag_len <
msg_hdr->msg_len| can be more clearly written as |frag_len !=
msg_hdr->msg_len|, since that's the only remaining case.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's d345a24569edf0a966b3d6eaae525f0ca4c5e570)

Change-Id: I038f9f01a1d9379f1ee058b231d80e8b9ce6c2d7
Reviewed-on: https://boringssl-review.googlesource.com/1438
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:32 +00:00
Matt Caswell
e24f686e31 Same fix as in dtls1_process_out_of_seq_message.
Applying same fix as in dtls1_process_out_of_seq_message. A truncated
DTLS fragment would cause *ok to be clear, but the return value would
still be the number of bytes read.

Problem identified by Emilia Käsper, based on previous issue/patch by Adam
Langley.

Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 3d5dceac430d7b9b273331931d4d2303f5a2256f)

Change-Id: Ibe30716266e2ee1489c98b922cf53edda096c23c
Reviewed-on: https://boringssl-review.googlesource.com/1437
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:21 +00:00
Adam Langley
8506609ca3 Fix return code for truncated DTLS fragment.
Previously, a truncated DTLS fragment in
|dtls1_process_out_of_seq_message| would cause *ok to be cleared, but
the return value would still be the number of bytes read. This would
cause |dtls1_get_message| not to consider it an error and it would
continue processing as normal until the calling function noticed that
*ok was zero.

I can't see an exploit here because |dtls1_get_message| uses
|s->init_num| as the length, which will always be zero from what I can
see.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's aad61c0a57a3b6371496034db61675abcdb81811.)

Change-Id: I2fb0ea93b6e812e19723ada3351f842cc7b2fa91
Reviewed-on: https://boringssl-review.googlesource.com/1436
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:12 +00:00
Adam Langley
e951ff4fc3 Fix memory leak from zero-length DTLS fragments.
The |pqueue_insert| function can fail if one attempts to insert a
duplicate sequence number. When handling a fragment of an out of
sequence message, |dtls1_process_out_of_seq_message| would not call
|dtls1_reassemble_fragment| if the fragment's length was zero. It would
then allocate a fresh fragment and attempt to insert it, but ignore the
return value, leaking the fragment.

This allows an attacker to exhaust the memory of a DTLS peer.

Fixes CVE-2014-3507

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 8ca4c4b25e050b881f3aad7017052842b888722d.)

Change-Id: I387e3f6467a0041f6367965ed3c1ad4377b9ac08
Reviewed-on: https://boringssl-review.googlesource.com/1435
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:09:00 +00:00
Matt Caswell
2306fe5ff5 Fix DTLS handshake message size checks.
In |dtls1_reassemble_fragment|, the value of
|msg_hdr->frag_off+frag_len| was being checked against the maximum
handshake message size, but then |msg_len| bytes were allocated for the
fragment buffer. This means that so long as the fragment was within the
allowed size, the pending handshake message could consume 16MB + 2MB
(for the reassembly bitmap). Approx 10 outstanding handshake messages
are allowed, meaning that an attacker could consume ~180MB per DTLS
connection.

In the non-fragmented path (in |dtls1_process_out_of_seq_message|), no
check was applied.

Fixes CVE-2014-3506

Wholly based on patch by Adam Langley with one minor amendment.

Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 0598468fc04fb0cf2438c4ee635b587aac1bcce6)

Change-Id: I4849498eabb45ec973fcb988d639b23145891e25
Reviewed-on: https://boringssl-review.googlesource.com/1434
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:08:49 +00:00
Matt Caswell
3873f6f33d Added comment for the frag->reassembly == NULL case as per feedback from Emilia
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's ea7cb5397457c59554155935b677a1dab23bd864)

Change-Id: Idd5ed233028c42d2b921deb424381aad88a0aa84
Reviewed-on: https://boringssl-review.googlesource.com/1433
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 18:20:41 +00:00
Adam Langley
d06afe40ab Avoid double free when processing DTLS packets.
The |item| variable, in both of these cases, may contain a pointer to a
|pitem| structure within |s->d1->buffered_messages|. It was being freed
in the error case while still being in |buffered_messages|. When the
error later caused the |SSL*| to be destroyed, the item would be double
freed.

Thanks to Wah-Teh Chang for spotting that the fix in 1632ef74 was
inconsistent with the other error paths (but correct).

Fixes CVE-2014-3505

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

(Imported from upstream's 49850075555893c9c60d5b981deb697f3b9515ea)

Change-Id: Ie40007184f6194ba032b4213c18d36254e80aaa6
Reviewed-on: https://boringssl-review.googlesource.com/1432
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 18:12:41 +00:00
David Benjamin
67454b6f76 Don't X509_up_ref X509_STOREs.
Change-Id: Ic78bec93aedcc06c1496fe374e1c1c77ef70ea4b
Reviewed-on: https://boringssl-review.googlesource.com/1416
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 02:44:31 +00:00
Kenny Root
7fdeaf1101 Retry sending record split fragment when SSL write fails.
When the write size was exactly SSL3_RT_MAX_PLAIN_LENGTH+1 and record
splitting is needed, an extra byte would be added to the max size of the
message to be written. This would cause the requested size to not exceed
the max. If the SSL_WANT_WRITE error were returned, the next packet
would not get the extra byte added to the max packet size since
record_split_done is set. Since a different set of arguments
(SSL3_RT_MAX_PLAIN_LENGTH+1 vs SSL3_RT_MAX_PLAIN_LENGTH) would be passed
to do_ssl3_write, it would return an "SSL3_WRITE_PENDING:bad write
retry" error.

To avoid a failure in the opposite direction, the max variable increment
is removed as well. This can happen when SSL_MODE_ENABLE_PARTIAL_WRITE
is not enabled and the call to ssl3_write_bytes contains, e.g., a buffer
of 2*SSL3_RT_MAX_PLAIN_LENGTH, where the first call into do_ssl3_write
succeeds writing the first SSL3_RT_MAX_PLAIN_LENGTH bytes, but writing
the second SSL3_RT_MAX_PLAIN_LENGTH bytes fails. This means the first
time the the second section of SSL3_RT_MAX_PLAIN_LENGTH bytes has called
do_ssl3_write with "max" bytes, but next call to ssl3_write_bytes in
turn calls into do_ssl3_write with "max+1" bytes.

Change-Id: Icf8453195c1145a54d31b8e8146801118207df03
Reviewed-on: https://boringssl-review.googlesource.com/1420
Reviewed-by: Kenny Root <kroot@google.com>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:08:44 +00:00
David Benjamin
150c617cfc Add X509_up_ref and use it internally.
Avoid needing to manually increment the reference count and using the right
lock, both here and in Chromium.

Change-Id: If116ebc224cfb1c4711f7e2c06f1fd2c97af21dd
Reviewed-on: https://boringssl-review.googlesource.com/1415
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:06:34 +00:00
David Benjamin
e6e15fc3a1 Use EVP_PKEY_dup instead of manually incrementing the refcount.
Reference counting should be internal to the type, otherwise callers need to
know which lock to use.

Change-Id: If4d805876a321ef6dece115c805e605584ff311e
Reviewed-on: https://boringssl-review.googlesource.com/1414
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:03:56 +00:00
David Benjamin
43ec06f705 Test state machine asynchronous behavior.
Add a framework for testing the asynchronous codepath. Move some handshake
state machine coverage tests to cover a range of record-layer and
handshake-layer asynchronicity.

This adds tests for the previous two async bugs fixed.

Change-Id: I422ef33ba3eeb0ad04766871ed8bc59b677b169e
Reviewed-on: https://boringssl-review.googlesource.com/1410
Reviewed-by: Adam Langley <agl@google.com>
2014-08-05 20:41:58 +00:00
David Benjamin
734fca0890 Fix memory leak in ssl3_get_cert_verify.
Any ssl3_get_* function that takes ownership of something before the
ssl_get_message call can't early-return without cleanup work.

This fixes valgrind on ClientAuth-Server-Async.

Change-Id: Ie7f0b37cac4d4bb7e06c00bae091fee0386c22da
Reviewed-on: https://boringssl-review.googlesource.com/1413
Reviewed-by: Adam Langley <agl@google.com>
2014-08-05 18:07:32 +00:00
David Benjamin
95fcaa4f4b Fix server-side ClientHello state machine.
- DTLS server code didn't account for the new ClientHello state. This looks
  like it only matters if a DTLS server uses select_certificate_cb and returns
  asynchronously.

- State A transitions immediately to B and is redundant. No code distinguishes
  A and B.

- The ssl_get_message call transitions to the second state (originally C). This
  makes the explicit transition to C a no-op. More of a problem,
  ssl_get_message may return asynchronously and remain in its second state if the
  handshake body had not completed yet. Fix this by splitting state C in two.
  Combined with the above change, this results in only the top few states getting
  reshuffled.

This fixes the server async tests.

Change-Id: I46703bcd205988b118217b6424ba4f88e731be5a
Reviewed-on: https://boringssl-review.googlesource.com/1412
Reviewed-by: Adam Langley <agl@google.com>
2014-08-05 18:07:13 +00:00
Alex Chernyakhovsky
6ccf29012c Remove use of freelist_{extract,insert}
With the removal of the freelist itself, these macros are
superfluous. Remove them in favore of OPENSSL_malloc and OPENSSL_free.

Change-Id: I4bfeff8ea087b9e16c7c32d7c1bdb7a07e7dd03e
Reviewed-on: https://boringssl-review.googlesource.com/1389
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:14:51 +00:00
Alex Chernyakhovsky
983f6bdb58 Set OPENSSL_NO_BUF_FREELISTS
The memory freelist maintained by OpenSSL claims to be a performance
optimization for platforms that have a slow malloc/free
implementation. This should not be the case on modern
linux/glibc. Remove the freelist as it poses a potential security
hazard of buffer-reuse that is of "initialized" memory that will not
be caught be tools such as valgrind.

Change-Id: I3cfa6a05f9bdfbbba7820060bae5a673dee43014
Reviewed-on: https://boringssl-review.googlesource.com/1385
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:14:33 +00:00
Alex Chernyakhovsky
75d1589f60 Remove PKCS1_CHECK
The code guarded by PKCS1_CHECK appears to be unhelpful, and the guard
is explicitly undefined in ssl_locl.h Remove both.

Change-Id: I3cd45a744a8f35b02181b1e48fd1ef11af5e6f4a
Reviewed-on: https://boringssl-review.googlesource.com/1383
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:14:21 +00:00
Alex Chernyakhovsky
499b440135 Remove OPENSSL_NO_SHA512
Changes were made by running

find . -type f -name *.c | xargs unifdef -m -U OPENSSL_NO_SHA512
find . -type f -name *.h | xargs unifdef -m -U OPENSSL_NO_SHA512

Change-Id: I5c8a89384c865423aa7b549470068ef18161bce4
Reviewed-on: https://boringssl-review.googlesource.com/1379
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:13:54 +00:00
Alex Chernyakhovsky
69877b6119 Remove OPENSSL_NO_SHA256
Changes were made by running

find . -type f -name *.c | xargs unifdef -m -U OPENSSL_NO_SHA256
find . -type f -name *.h | xargs unifdef -m -U OPENSSL_NO_SHA256

Change-Id: Ia2fda4b1a3a4aad286ced86cf936358fc7a30b15
Reviewed-on: https://boringssl-review.googlesource.com/1378
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:13:46 +00:00
David Benjamin
f4e5c4e106 runner: Implement DHE-RSA.
Use it to test DHE-RSA in BoringSSL.

Change-Id: I88f7bfa76507a6f60234d61d494c9f94b7df4e0a
Reviewed-on: https://boringssl-review.googlesource.com/1377
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:12:58 +00:00