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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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
