This drops in a copy of a subset of golang.org/x/crypto/poly1305 to implement
Poly1305. Hopefully this will keep them from regression as we rework the record
layer.
Change-Id: Ic1e0d941a0a9e5ec260151ced8acdf9215c4b887
Reviewed-on: https://boringssl-review.googlesource.com/4257
Reviewed-by: Adam Langley <agl@google.com>
Amazingly, asn1_GetSequence isn't completely unused? Keep that around for now
and ditch everything else. This lets us enable C4311 in MSVC which is actually
a pretty reasonable warning.
Change-Id: I43bb9206b1745e8a68224f3a435713d2a74e04ea
Reviewed-on: https://boringssl-review.googlesource.com/4256
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: If15f2f0e2b4627318c9cdfbc76d5ca56a6894e3f
Reviewed-on: https://boringssl-review.googlesource.com/4270
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
MSVC doesn't like unary - on unsigned numbers. Also switch ssl3_read_n's
version to uintptr_t to match the write half. This gets us closer to clearing
through C4311 violations. (The remaining one is in asn1_add_error which can go
after verifying that most of asn1_mac.h is safe to drop.)
Change-Id: Idb33dda8863bf1a3408b14d5513a667338311b6b
Reviewed-on: https://boringssl-review.googlesource.com/4255
Reviewed-by: Adam Langley <agl@google.com>
all_tests.go will still complain if tab_test is missing.
Change-Id: I97c3684a4397caa55aaae2ec6555b16ee8366233
Reviewed-on: https://boringssl-review.googlesource.com/4250
Reviewed-by: Adam Langley <agl@google.com>
The unused ex_data index declarations are commented out instead of
removed so that it is clear which values to avoid for any new ex_data
indexes added in the future.
Change-Id: Ia19da9631324492c5c7eeacc71453e6240c73870
Reviewed-on: https://boringssl-review.googlesource.com/3940
Reviewed-by: Adam Langley <agl@google.com>
Implement ECDSA_SIG_new and ECDSA_SIG_free manually in preparation for
removing all crypto/asn1 dependencies from ECDSA signature verification.
Change-Id: I0e84d74fa8e757af0cfb09daef03d59f428143cc
Reviewed-on: https://boringssl-review.googlesource.com/4153
Reviewed-by: Adam Langley <agl@google.com>
These functions are useful for implementing non-ASN.1-based protocols
like JSON Web Signature (JWS) and they are even already used within
Chromium.
Change-Id: I58f41ca7beedc5a0b7a8c3da53f319aadff4c0e7
Reviewed-on: https://boringssl-review.googlesource.com/3936
Reviewed-by: Adam Langley <agl@google.com>
decrepit will contain algorithms that we really wish didn't exist any
longer. It won't be built by default in Chromium etc, but the code
will exist for crummy code that still needs it.
Change-Id: Ic307f5f0a69efe9e0a5fd54052f49d219e90dcdd
Sadly, it turns out that we have need of this, at least for now. The
code is taken from upstream and changed only as much as needed.
This only imports keys and doesn't know how to actually perform
operations on them for now.
Change-Id: I0db70fb938186cb7a91d03f068b386c59ed90b84
After sharding the session cache for fallbacks, the numbers have been pretty
good; 0.03% on dev and 0.02% on canary. Stable is at 0.06% but does not have
the sharded session cache. Before sharding, stable, beta, and dev had been
fairly closely aligned. Between 0.03% being low and the fallback saving us in
all but extremely contrived cases, I think this should be fairly safe.
Add tests for both the cipher suite and protocol version mismatch checks.
BUG=441456
Change-Id: I2374bf64d0aee0119f293d207d45319c274d89ab
Reviewed-on: https://boringssl-review.googlesource.com/3972
Reviewed-by: Adam Langley <agl@google.com>
This conceivably has a use, but NSS doesn't do this buffer either and it still
suffers from the same problems as the other uses of record_pqueue. This removes
the last use of record_pqueue. It also opens the door to removing pqueue
altogether as it isn't the right data structure for either of the remaining
uses either. (It's not clear it was right for record_pqueue either, but I don't
feel like digging into this code.)
Change-Id: If8a43e7332b3cd11a78a516f3e8ebf828052316f
Reviewed-on: https://boringssl-review.googlesource.com/4239
Reviewed-by: Adam Langley <agl@google.com>
The encoding of an INTEGER should not have leading zeros, except to pad for the
sign bit.
Change-Id: I80d22818cf1d2ca9d27e215620392e1725372aa5
Reviewed-on: https://boringssl-review.googlesource.com/4218
Reviewed-by: Adam Langley <agl@google.com>
It was only ever enabled for handshake and alert messages. The comments cite
renego as a use case though even then I'm not clear on why. The only use I see
is if, say, the Finished message and ClientKeyExchange came in out-of-order.
DTLS is unreliable so leaning on retransmit seems fine, and usually flights
will be packed into one packet where possible. NSS doesn't have any such
buffer and doesn't seem to have problems.
The buffering mechanism is also rather dubious. It stows away the entire packet
and read buffer---all 16K of it---and there may have been other records in that
packet.
Change-Id: Ic3b7bf817be380dc73102eec62c690ed093e6667
Reviewed-on: https://boringssl-review.googlesource.com/4238
Reviewed-by: Adam Langley <agl@google.com>
At this point, has_version has been set and we may even have a non-null cipher.
Trying to assign meaning to the record-layer version number is not worth making
s->version's semantics even more complicated.
Change-Id: Ia1cf341cf7306eb48d2d11241316dc2116306968
Reviewed-on: https://boringssl-review.googlesource.com/4237
Reviewed-by: Adam Langley <agl@google.com>
Compression is gone, so don't allow for compression overhead. With that fixed,
the second rr->length check in ssl3_get_record matches the length computation
which sizes the read buffer. The first is wrong and doesn't account for the
alignment padding. Move the second to the first.
Change-Id: I3f4f05de9fdf5c645ff24493bbfdf303dcc1aa90
Reviewed-on: https://boringssl-review.googlesource.com/4236
Reviewed-by: Adam Langley <agl@google.com>
Also check for overflow, although it really shouldn't happen.
Change-Id: I34dfe8eaf635aeaa8bef2656fda3cd0bad7e1268
Reviewed-on: https://boringssl-review.googlesource.com/4235
Reviewed-by: Adam Langley <agl@google.com>
These are redundant with the lower level ones in s3_pkt.c just before BIO_read.
Only the operation which actually failed an operation on the BIO should set
the wait state.
Not all failure paths in ssl3_read_bytes and dtls1_read_bytes set SSL_READING,
but those that don't leave the BIO in a retry state, so SSL_READING doesn't
matter.
Change-Id: I2ae064ecc8b2946cc8ae8f724be09dfe49e077b5
Reviewed-on: https://boringssl-review.googlesource.com/4230
Reviewed-by: Adam Langley <agl@google.com>
Fix up the variable names. Also avoid the messy logic of checking whether the
label and context collide with the normal key expansion ones in the face of
adverserial inputs. Make that the caller's responsibility, just as it's already
the caller's responsibility to ensure that different calls don't overlap. (The
label should be a constant string in an IANA registry anyway.)
Change-Id: I062fadb7b6a18fa946b883be660ea9b3f0f6277c
Reviewed-on: https://boringssl-review.googlesource.com/4216
Reviewed-by: Adam Langley <agl@google.com>
Separate actually writing the fragment to the network from assembling it so
there is no need for is_fragment. record_split_done also needn't be a global;
as of 7fdeaf1101, it is always reset to 0 whether
or not SSL3_WANT_WRITE occurred, despite the comment.
I believe this is sound, but the pre-7fdeaf1 logic wasn't quiiite right;
ssl3_write_pending allows a retry to supply *additional* data, so not all
plaintext had been commited to before the IV was randomized. We could fix this
by tracking how many bytes were committed to the last time we fragmented, but
this is purely an optimization and doesn't seem worth the complexity.
This also fixes the alignment computation in the record-splitting case. The
extra byte was wrong, as demonstrated by the assert.
Change-Id: Ia087a45a6622f4faad32e501942cc910eca1237b
Reviewed-on: https://boringssl-review.googlesource.com/4234
Reviewed-by: Adam Langley <agl@google.com>
It's still rather a mess, but this is at least somewhat clearer. The old one
had a lot of remnants of compression, etc.
Change-Id: Iffcb4dd4e8c4ab14f60abf917d22b7af960c93ba
Reviewed-on: https://boringssl-review.googlesource.com/4233
Reviewed-by: Adam Langley <agl@google.com>
If the key is missing, it seems the failure is assumed to be expected.
BUG=473924
Change-Id: I62edd9110fa74bee5e6425fd6786badf5398728c
Reviewed-on: https://boringssl-review.googlesource.com/4231
Reviewed-by: Adam Langley <agl@google.com>
Move the state to TestState rather than passing pointers to them everywhere.
Also move SSL_read and SSL_write retry loops into helper functions so they
aren't repeated everywhere. This also makes the SSL_write calls all
consistently account for partial writes.
Change-Id: I9bc083a03da6a77ab2fc03c29d4028435fc02620
Reviewed-on: https://boringssl-review.googlesource.com/4214
Reviewed-by: Adam Langley <agl@google.com>
Extend the False Start tests to optionally send an alert (thus avoiding
deadlock) before waiting for the out-of-order app data. Based on whether the
peer shuts off the connection before or after sending app data, we can
determine whether the peer False Started by observing purely external effects.
Change-Id: I8b9fecc29668e0b0c34b5fd19d0f239545011bae
Reviewed-on: https://boringssl-review.googlesource.com/4213
Reviewed-by: Adam Langley <agl@google.com>
Based on whether -false-start is passed, we expect SSL_CB_HANDSHAKE_DONE to or
not to fire. Also add a flag that asserts SSL_CB_HANDSHAKE_DONE does *not* fire
in any False Start test where the handshake fails after SSL_connect returns.
Change-Id: I6c5b960fff15e297531e15b16abe0b98be95bec8
Reviewed-on: https://boringssl-review.googlesource.com/4212
Reviewed-by: Adam Langley <agl@google.com>
That's a pretty obvious thing to test. I'm not sure how we forgot that one.
Change-Id: I7e1a7df6c6abbdd587e0f7723117f50d09faa5c4
Reviewed-on: https://boringssl-review.googlesource.com/4211
Reviewed-by: Adam Langley <agl@google.com>
This fixes the Windows build. Otherwise this collides with the symbol in
Chromium's //base. (The 'base' suffix is the name of the library, not some
Windows-ism.)
Change-Id: I65d755f08991978bd2040d53c401082b2fee65fa
Reviewed-on: https://boringssl-review.googlesource.com/4217
Reviewed-by: Adam Langley <agl@google.com>
It seems Android's inttypes.h refuses to define those macros on C++ unless
__STDC_FORMAT_MACROS is set. This unbreaks the roll on Android.
Change-Id: Iad6c971b4789f0302534d9e5022534c6124e0ff0
Reviewed-on: https://boringssl-review.googlesource.com/4202
Reviewed-by: Adam Langley <agl@google.com>
This fixes the standalone build on Windows and matches Chromium.
Change-Id: I194f53e0a610c5ae9cef53c826b22f7bded5f357
Reviewed-on: https://boringssl-review.googlesource.com/4201
Reviewed-by: Adam Langley <agl@google.com>
There's multiple sets of APIs for selecting the curve. Fold away
SSL_OP_SINGLE_ECDH_USE as failing to set it is either a no-op or a bug. With
that gone, the consumer only needs to control the selection of a curve, with
key generation from then on being uniform. Also clean up the interaction
between the three API modes in s3_srvr.c; they were already mutually exclusive
due to tls1_check_ec_tmp_key.
This also removes all callers of EC_KEY_dup (and thus CRYPTO_dup_ex_data)
within the library.
Change-Id: I477b13bd9e77eb03d944ef631dd521639968dc8c
Reviewed-on: https://boringssl-review.googlesource.com/4200
Reviewed-by: Adam Langley <agl@google.com>
ASan's own malloc interceptor isn't compatible with this mechanism; it doesn't
see calls to __libc_malloc.
Change-Id: Ibac5aa05c6e40f1c72dcee3a2597e96deffca62c
Reviewed-on: https://boringssl-review.googlesource.com/4191
Reviewed-by: Adam Langley <agl@google.com>
Not actually much of a change, but consistency.
Change-Id: If2ef7a8b698a229f5c494822d870767e1a61476e
Reviewed-on: https://boringssl-review.googlesource.com/4127
Reviewed-by: Adam Langley <agl@google.com>
Along the way, fix a host of missing failure checks. This will save some
headache when it comes time to run these under the malloc failure tests.
Change-Id: I3fd589bd094178723398e793d6bc578884e99b67
Reviewed-on: https://boringssl-review.googlesource.com/4126
Reviewed-by: Adam Langley <agl@google.com>
SSL_CIPHER_get_rfc_name still returns an allocated string.
Change-Id: Ie2f14626c1ff22d0ea613b22439b7de5c04c9062
Reviewed-on: https://boringssl-review.googlesource.com/4190
Reviewed-by: Adam Langley <agl@google.com>
