This extends the packet adaptor protocol to send three commands:
type command =
| Packet of []byte
| Timeout of time.Duration
| TimeoutAck
When the shim processes a Timeout in BIO_read, it sends TimeoutAck, fails the
BIO_read, returns out of the SSL stack, advances the clock, calls
DTLSv1_handle_timeout, and continues.
If the Go side sends Timeout right between sending handshake flight N and
reading flight N+1, the shim won't read the Timeout until it has sent flight
N+1 (it only processes packet commands in BIO_read), so the TimeoutAck comes
after N+1. Go then drops all packets before the TimeoutAck, thus dropping one
transmit of flight N+1 without having to actually process the packets to
determine the end of the flight. The shim then sees the updated clock, calls
DTLSv1_handle_timeout, and re-sends flight N+1 for Go to process for real.
When dropping packets, Go checks the epoch and increments sequence numbers so
that we can continue to be strict here. This requires tracking the initial
sequence number of the next epoch.
The final Finished message takes an additional special-case to test. DTLS
triggers retransmits on either a timeout or seeing a stale flight. OpenSSL only
implements the former which should be sufficient (and is necessary) EXCEPT for
the final Finished message. If the peer's final Finished message is lost, it
won't be waiting for a message from us, so it won't time out anything. That
retransmit must be triggered on stale message, so we retransmit the Finished
message in Go.
Change-Id: I3ffbdb1de525beb2ee831b304670a3387877634c
Reviewed-on: https://boringssl-review.googlesource.com/3212
Reviewed-by: Adam Langley <agl@google.com>
This is fatal for TLS but buffered in DTLS. The buffering isn't strictly
necessary (it would be just as valid to drop the record on the floor), but so
long as we want this behavior it should have a test.
Change-Id: I5846bb2fe80d78e25b6dfad51bcfcff2dc427c3f
Reviewed-on: https://boringssl-review.googlesource.com/3029
Reviewed-by: Adam Langley <agl@google.com>
This regressed in e95d20dcb8. EVP_AEAD will push
errors on the error queue (unlike the EVP_CIPHER codepath which checked
everything internally to ssl/ and didn't bother pushing anything). This meant
that a dropped packet would leave junk in the error queue.
Later, when SSL_read returns <= 0 (EOF or EWOULDBLOCK), the non-empty error
queue check in SSL_get_error kicks in and SSL_read looks to have failed.
BUG=https://code.google.com/p/webrtc/issues/detail?id=4214
Change-Id: I1e5e41c77a3e5b71e9eb0c72294abf0da677f840
Reviewed-on: https://boringssl-review.googlesource.com/2982
Reviewed-by: Adam Langley <agl@google.com>
This CL removes the last of the EVP_CIPHER codepath in ssl/. The dead code is
intentionally not pruned for ease of review, except in DTLS-only code where
adding new logic to support both, only to remove half, would be cumbersome.
Fixes made:
- dtls1_retransmit_state is taught to retain aead_write_ctx rather than
enc_write_ctx.
- d1_pkt.c reserves space for the variable-length nonce when echoed into the
packet.
- dtls1_do_write sizes the MTU based on EVP_AEAD max overhead.
- tls1_change_cipher_state_cipher should not free AEAD write contexts in DTLS.
This matches the (rather confused) ownership for the EVP_CIPHER contexts.
I've added a TODO to resolve this craziness.
A follow-up CL will remove all the resultant dead code.
Change-Id: I644557f4db53bbfb182950823ab96d5e4c908866
Reviewed-on: https://boringssl-review.googlesource.com/2699
Reviewed-by: Adam Langley <agl@google.com>
The minimum MTU (not consistently enforced) is just under 256, so it's
difficult to test everything, but this is a basic test. (E.g., without renego,
the only handshake message with encryption is Finished which fits in the MTU.)
It tests the server side because the Certificate message is large enough to
require fragmentation.
Change-Id: Ida11f1057cebae2b800ad13696f98bb3a7fbbc5e
Reviewed-on: https://boringssl-review.googlesource.com/2824
Reviewed-by: Adam Langley <agl@google.com>
Add a dedicated error code to the queue for a handshake_failure alert in
response to ClientHello. This matches NSS's client behavior and gives a better
error on a (probable) failure to negotiate initial parameters.
BUG=https://crbug.com/446505
Change-Id: I34368712085a6cbf0031902daf2c00393783d96d
Reviewed-on: https://boringssl-review.googlesource.com/2751
Reviewed-by: Adam Langley <agl@google.com>
Ensure that both the client and the server emit a protocol_version alert
(except in SSLv3 where it doesn't exist) with a record-layer version which the
peer will recognize.
Change-Id: I31650a64fe9b027ff3d51e303711910a00b43d6f
This makes SSLv23_method go through DTLS_ANY_VERSION's version negotiation
logic. This allows us to get rid of duplicate ClientHello logic. For
compatibility, SSL_METHOD is now split into SSL_PROTOCOL_METHOD and a version.
The legacy version-locked methods set min_version and max_version based this
version field to emulate the original semantics.
As a bonus, we can now handle fragmented ClientHello versions now.
Because SSLv23_method is a silly name, deprecate that too and introduce
TLS_method.
Change-Id: I8b3df2b427ae34c44ecf972f466ad64dc3dbb171
These tests use both APIs. This also modifies the inline version negotiation's
error codes (currently only used for DTLS) to align with SSLv23's error codes.
Note: the peer should send a protocol_version alert which is currently untested
because it's broken.
Upstream would send such an alert if TLS 1.0 was supported but not otherwise,
which is somewhat bizarre. We've actually regressed and never send the alert in
SSLv23. When version negotiation is unified, we'll get the alerts back.
Change-Id: I4c77bcef3a3cd54a039a642f189785cd34387410
Reviewed-on: https://boringssl-review.googlesource.com/2584
Reviewed-by: Adam Langley <agl@google.com>
Amend the version negotiation tests to test this new spelling of max_version.
min_version will be tested in a follow-up.
Change-Id: Ic4bfcd43bc4e5f951140966f64bb5fd3e2472b01
Reviewed-on: https://boringssl-review.googlesource.com/2583
Reviewed-by: Adam Langley <agl@google.com>
else block got lost in a rewrite of this code.
Change-Id: I51f1655474ec8bbd4eccb4297124e8584329444e
Reviewed-on: https://boringssl-review.googlesource.com/2560
Reviewed-by: Adam Langley <agl@google.com>
The record-layer version of the ServerHello should match the final version. The
record-layer version of the ClientHello should be the advertised version, but
clamped at TLS 1.0. This is to ensure future rewrites do not regress this.
Change-Id: I96f1f0674944997ff38b562453a322ce61652635
Reviewed-on: https://boringssl-review.googlesource.com/2540
Reviewed-by: Adam Langley <agl@google.com>
The ClientHello record is padded to 1024 bytes when
fastradio_padding is enabled. As a result, the 3G cellular radio
is fast forwarded to DCH (high data rate) state. This mechanism
leads to a substantial redunction in terms of TLS handshake
latency, and benefits mobile apps that are running on top of TLS.
Change-Id: I3d55197b6d601761c94c0f22871774b5a3dad614
Replace the comment with a clearer one and reimplement it much more tidily. The
mask thing was more complicated than was needed.
This slightly changes behavior on the DTLS_ANY_VERSION side in that, if only
one method is enabled, we no longer short-circuit to the version-locked method
early. This "optimization" seems unnecessary.
Change-Id: I571c8b60ed16bd4357c67d65df0dd1ef9cc5eb57
Reviewed-on: https://boringssl-review.googlesource.com/2451
Reviewed-by: Adam Langley <agl@google.com>
This fixes bugs that kept the tests from working:
- Resolve DTLS version and cookie before the session.
- In DTLS_ANY_VERSION, ServerHello should be read with first_packet = 1. This
is a regression from f2fedefdca. We'll want to
do the same for TLS, but first let's change this to a boolean has_version in a
follow-up.
Things not yet fixed:
- DTLS code is not EVP_AEAD-aware. Those ciphers are disabled for now.
- On the client, DTLS_ANY_VERSION creates SSL_SESSIONs with the wrong
ssl_version. The tests pass because we no longer enforce the match as of
e37216f560. (In fact, we've gone from the server
ignoring ssl_version and client enforcing to the client mostly ignoring
ssl_version and the server enforcing.)
- ssl3_send_client_hello's ssl_version check checks for equality against
s->version rather than >.
Change-Id: I5a0dde221b2009413df9b9443882b9bf3b29519c
Reviewed-on: https://boringssl-review.googlesource.com/2403
Reviewed-by: Adam Langley <agl@google.com>
We forgot to add those when we implemented the features. (Also relevant because
they will provide test coverage later for configuring features when using the
generic method tables rather than *_client_method.)
Change-Id: Ie08b27de893095e01a05a7084775676616459807
Reviewed-on: https://boringssl-review.googlesource.com/2410
Reviewed-by: Adam Langley <agl@google.com>
This implements session IDs in client and server in runner.go.
Change-Id: I26655f996b7b44c7eb56340ef6a415d3f2ac3503
Reviewed-on: https://boringssl-review.googlesource.com/2350
Reviewed-by: Adam Langley <agl@google.com>
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.
(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)
This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.
Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
Just the negotiation portion as everything else is external. This feature is
used in WebRTC.
Change-Id: Iccc3983ea99e7d054b59010182f9a56a8099e116
Reviewed-on: https://boringssl-review.googlesource.com/2310
Reviewed-by: Adam Langley <agl@google.com>
Prior to this change, BoringSSL maintained a 2-byte buffer for alerts,
and would support reassembly of fragmented alerts.
NSS does not support fragmented alerts, nor would any reasonable
implementation produce them. Remove fragmented alert handling and
produce an error if a fragmented alert has ever been encountered.
Change-Id: I31530ac372e8a90b47cf89404630c1c207cfb048
Reviewed-on: https://boringssl-review.googlesource.com/2125
Reviewed-by: Adam Langley <agl@google.com>
All of NSS, upstream OpenSSL, SChannel, and Secure Transport require, on the
client, that the ServerHello version match the session's version on resumption.
OpenSSL's current behavior is incompatible with all of these. Fall back to a
full handshake on the server instead of mismatch.
Add a comment on the client for why we are, as of
30ddb434bfb845356fbacb6b2bd51f8814c7043c, not currently enforcing the same in
the client.
Change-Id: I60aec972d81368c4ec30e2fd515dabd69401d175
Reviewed-on: https://boringssl-review.googlesource.com/2244
Reviewed-by: Adam Langley <agl@google.com>
These'll get removed once most of renego support is gone, but this is to prove
removing the warning alert from the previous commit still prevents legacy
renegotiations.
Change-Id: I7d9d95e1d4c5d23d3b6d170938a5499a65f2d5ea
Reviewed-on: https://boringssl-review.googlesource.com/2236
Reviewed-by: Adam Langley <agl@google.com>
Ensure that the client rejects it with UNEXPECTED_MESSAGE, not by attempting to
decode it.
Change-Id: Ifc5613cf1152e0f7dcbee73e05df1ef367dfbfd5
Reviewed-on: https://boringssl-review.googlesource.com/2232
Reviewed-by: Adam Langley <agl@google.com>
At the record layer, DTLS maintains a window of seen sequence numbers to detect
replays. Add tests to cover that case. Test both repeated sequence numbers
within the window and sequence numbers past the window's left edge. Also test
receiving sequence numbers far past the window's right edge.
Change-Id: If6a7a24869db37fdd8fb3c4b3521b730e31f8f86
Reviewed-on: https://boringssl-review.googlesource.com/2221
Reviewed-by: Adam Langley <agl@google.com>
This change adds support to the Go code for renegotiation as a client,
meaning that we can test BoringSSL's renegotiation as a server.
Change-Id: Iaa9fb1a6022c51023bce36c47d4ef7abee74344b
Reviewed-on: https://boringssl-review.googlesource.com/2082
Reviewed-by: Adam Langley <agl@google.com>
pskKeyAgreement is now a wrapper over a base key agreement.
Change-Id: Ic18862d3e98f7513476f878b8df5dcd8d36a0eac
Reviewed-on: https://boringssl-review.googlesource.com/2053
Reviewed-by: Adam Langley <agl@google.com>
Only the three plain PSK suites for now. ECDHE_PSK_WITH_AES_128_GCM_SHA256 will
be in a follow-up.
Change-Id: Iafc116a5b2798c61d90c139b461cf98897ae23b3
Reviewed-on: https://boringssl-review.googlesource.com/2051
Reviewed-by: Adam Langley <agl@google.com>
Both as client and as server. Also tests that ALPN causes False Start to kick
in.
Change-Id: Ib570346f3c511834152cd2df2ef29541946d3ab4
Reviewed-on: https://boringssl-review.googlesource.com/1753
Reviewed-by: Adam Langley <agl@google.com>
Otherwise the child is busy waiting for its second handshake.
Change-Id: Ic613eeb04c5d6c1ec1e1bbcb13946d3ac31d05f1
Reviewed-on: https://boringssl-review.googlesource.com/1752
Reviewed-by: Adam Langley <agl@google.com>
Notably, this would have caught ed8270a55c
(although, apart from staring at code coverage, knowing to set resumeSession on
the server test isn't exactly obvious). Perhaps we should systematically set it
on all extension server tests; ClientHello extension parsing happens after
resumption has been determined and is often sensitive to it.
Change-Id: Ie83f294a26881a6a41969e9dbd102d0a93cb68b5
Reviewed-on: https://boringssl-review.googlesource.com/1750
Reviewed-by: Adam Langley <agl@google.com>
These were added in TLS 1.2. They are like the standard AES-CBC cipher suites,
but use different HMACs.
Change-Id: Ib89ddebd1aa398b1347f8285f5d827068b1bd181
Reviewed-on: https://boringssl-review.googlesource.com/1730
Reviewed-by: Adam Langley <agl@google.com>
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 226751ae4a.
Change-Id: I5b864d3a696f3187b849c53b872c24fb7df27924
Reviewed-on: https://boringssl-review.googlesource.com/1696
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
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>
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>
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>
David Benjamin
Feng Lu
Adam Langley
Alex Chernyakhovsky