Windows does support anonymous unions but warns about it. Since I'm not
sure what warnings we have enabled in Chromium, this change just drops
the union for Windows.
Change-Id: I914f8cd5855eb07153105250c0f026eaedb35365
Reviewed-on: https://boringssl-review.googlesource.com/6631
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
wpa_supplicant needs access to the internals of SHA_CTX. We supported
this only for builds with ANDROID defined previously but that's a pain
for wpa_supplicant to deal with. Thus this change enables it
unconditionally.
Perhaps in the future we'll be able to get a function to do this into
OpenSSL and BoringSSL.
Change-Id: Ib5d088c586fe69249c87404adb45aab5a7d5cf80
Reviewed-on: https://boringssl-review.googlesource.com/6630
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Right now your options are:
- Bounce on a reference and deal with cleanup needlessly.
- Manually check the type tag and peek into the union.
We probably have no hope of opaquifying this struct, but for new code, let's
recommend using this function rather than the more error-prone thing.
Change-Id: I9b39ff95fe4264a3f7d1e0d2894db337aa968f6c
Reviewed-on: https://boringssl-review.googlesource.com/6551
Reviewed-by: Adam Langley <agl@google.com>
The |ri| field was only used in |BN_MONT_CTX_set|, so make it a local
variable of that function.
Change-Id: Id8c3d44ac2e30e3961311a7b1a6731fe2c33a0eb
Reviewed-on: https://boringssl-review.googlesource.com/6526
Reviewed-by: Adam Langley <agl@google.com>
Trim the cipher table further. Those values are entirely determined by
algorithm_enc.
Change-Id: I355c245b0663e41e54e62d15903a4a9a667b4ffe
Reviewed-on: https://boringssl-review.googlesource.com/6516
Reviewed-by: Adam Langley <agl@google.com>
FIPS is the same as HIGH (but for CHACHA20), so those are redundant.
Likewise, MEDIUM vs HIGH was just RC4. Remove those in favor of
redefining those legacy rules to mean this.
One less field to keep track of in each cipher.
Change-Id: I2b2489cffb9e16efb0ac7d7290c173cac061432a
Reviewed-on: https://boringssl-review.googlesource.com/6515
Reviewed-by: Adam Langley <agl@google.com>
It's redundant with other cipher properties. We can express these in code.
Cipher rule matching gets a little bit complicated due to the confusing legacy
protocol version cipher rules, so add some tests for it. (It's really hard to
grep for uses of them, so I've kept them working to be safe.)
Change-Id: Ic6b3fcd55d76d4a51b31bf7ae629a2da50a7450e
Reviewed-on: https://boringssl-review.googlesource.com/6453
Reviewed-by: Adam Langley <agl@google.com>
The keylog BIO is internally synchronized by the SSL_CTX lock, but an
application may wish to log keys from multiple SSL_CTXs. This is in
preparation for switching Chromium to use a separate SSL_CTX per profile
to more naturally split up the session caches.
It will also be useful for routing up SSLKEYLOGFILE in WebRTC. There,
each log line must be converted to an IPC up from the renderer
processes.
This will require changes in Chromium when we roll BoringSSL.
BUG=458365,webrtc:4417
Change-Id: I2945bdb4def0a9c36e751eab3d5b06c330d66b54
Reviewed-on: https://boringssl-review.googlesource.com/6514
Reviewed-by: Adam Langley <agl@google.com>
Without |EC_POINTs_mul|, there's never more than one variable point
passed to a |EC_METHOD|'s |mul| method. This allows them to be
simplified considerably. In this commit, the p256-x86_64 implementation
has been simplified to eliminate the heap allocation and looping
related that was previously necessary to deal with the possibility of
there being multiple input points. The other implementations were left
mostly as-is; they should be similarly simplified in the future.
Change-Id: I70751d1d5296be2562af0730e7ccefdba7a1acae
Reviewed-on: https://boringssl-review.googlesource.com/6493
Reviewed-by: Adam Langley <agl@google.com>
This moves us closer to having |EC_GROUP| and |EC_KEY| being immutable.
The functions are left as no-ops for backward compatibility.
Change-Id: Ie23921ab0364f0771c03aede37b064804c9f69e0
Reviewed-on: https://boringssl-review.googlesource.com/6485
Reviewed-by: Adam Langley <agl@google.com>
If -mfpu=neon is passed then we don't need to worry about checking for
NEON support at run time. This change allows |CRYPTO_is_NEON_capable| to
statically return 1 in this case. This then allows the compiler to
discard generic code in several cases.
Change-Id: I3b229740ea3d5cb0a304f365c400a0996d0c66ef
Reviewed-on: https://boringssl-review.googlesource.com/6523
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I3072f884be77b8646e90d316154b96448f0cf2a1
Reviewed-on: https://boringssl-review.googlesource.com/6520
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
So long as we're not getting rid of them (the certificate variants may
be useful when we decouple from crypto/x509 anyway), get the types and
bounds checks right.
Also reject trailing data and require the input be a single element.
Note: this is a slight compatibility risk, but we did it for
SSL*_use_RSAPrivateKey_ASN1 previously and I think it's probably worth
seeing if anything breaks here.
Change-Id: I64fa3fc6249021ccf59584d68e56ff424a190082
Reviewed-on: https://boringssl-review.googlesource.com/6490
Reviewed-by: Adam Langley <agl@google.com>
OpenSSH calls |RAND_seed| before jailing in the expectation that that
will be sufficient to ensure that later RAND calls are successful.
See internal bug 25695426.
Change-Id: I9d3f5665249af6610328ac767cb83059bb2953dd
Reviewed-on: https://boringssl-review.googlesource.com/6494
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
dh.c had a 10k-bit limit but it wasn't quite correctly enforced. However,
that's still 1.12s of jank on the IO thread, which is too long. Since the SSL
code consumes DHE groups from the network, it should be responsible for
enforcing what sanity it needs on them.
Costs of various bit lengths on 2013 Macbook Air:
1024 - 1.4ms
2048 - 14ms
3072 - 24ms
4096 - 55ms
5000 - 160ms
10000 - 1.12s
UMA says that DHE groups are 0.2% 4096-bit and otherwise are 5.5% 2048-bit and
94% 1024-bit and some noise. Set the limit to 4096-bit to be conservative,
although that's already quite a lot of jank.
BUG=554295
Change-Id: I8e167748a67e4e1adfb62d73dfff094abfa7d215
Reviewed-on: https://boringssl-review.googlesource.com/6464
Reviewed-by: Adam Langley <agl@google.com>
This exposes the ServerKeyExchange signature hash type used in the most recent
handshake, for histogramming on the client.
BUG=549662
Change-Id: I8a4e00ac735b1ecd2c2df824112c3a0bc62332a7
Reviewed-on: https://boringssl-review.googlesource.com/6413
Reviewed-by: Adam Langley <agl@google.com>
I've used these defines to easy the update of BoringSSL in Android
because Android's external/boringssl is a different git repository from
the rest of Android and thus it's not possible to land changes the
atomically update several things at once.
For this I tended just to add this define in the Android copy of
BoringSSL, but we're starting to see that bleed into other situations
now so it's looking like this will be generally useful.
These defines may be added when useful but shouldn't build up: once the
change has been done, the #if'ed code elsewhere that uses it should be
cleaned up. So far, that's worked ok. (I.e. we've had a BORINGSSL_201509
that correctly disappeared.)
Change-Id: I8cbb4731efe840cc798c970d37bc040b16a4a755
Reviewed-on: https://boringssl-review.googlesource.com/6442
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Not sure if we want to leave bio.h and bytestring.h's instance as-is, but the
evp.h ones are just baffling.
Change-Id: I485c2e355ba93764da0c4c72c48af48b055a8500
Reviewed-on: https://boringssl-review.googlesource.com/6454
Reviewed-by: Adam Langley <agl@google.com>
Most functions can take this in as const. Note this changes an
RSA_METHOD hook, though one I would not expect anyone to override.
Change-Id: Ib70ae65e5876b01169bdc594e465e3e3c4319a8b
Reviewed-on: https://boringssl-review.googlesource.com/6419
Reviewed-by: Adam Langley <agl@google.com>
Later when TLS 1.3 comes around, we'll need SSL_CIPHER_get_max_version too. In
the meantime, hide the SSL_TLSV1_2 messiness behind a reasonable API.
Change-Id: Ibcc17cccf48dd99e364d6defdfa5a87d031ecf0a
Reviewed-on: https://boringssl-review.googlesource.com/6452
Reviewed-by: Adam Langley <agl@google.com>
This removes a sharp corner in the API where |ECDH_compute_key| assumed
that callers were either using ephemeral keys, or else had already
checked that the public key was on the curve.
A public key that's not on the curve can be in a small subgroup and thus
the result can leak information about the private key.
This change causes |EC_POINT_set_affine_coordinates_GFp| to require that
points are on the curve. |EC_POINT_oct2point| already does this.
Change-Id: I77d10ce117b6efd87ebb4a631be3a9630f5e6636
Reviewed-on: https://boringssl-review.googlesource.com/5861
Reviewed-by: Adam Langley <agl@google.com>
This change fixes up several comments (many of which were spotted by
Kenny Root) and also changes doc.go to detect cases where comments don't
start with the correct word. (This is a common error.)
Since we have docs builders now, these errors will be found
automatically in the future.
Change-Id: I58c6dd4266bf3bd4ec748763c8762b1a67ae5ab3
Reviewed-on: https://boringssl-review.googlesource.com/6440
Reviewed-by: Adam Langley <agl@google.com>
This function allows one to extract the current IVs from an SSL
connection. This is needed for the CBC cipher suites with implicit IVs
because, for those, the IV can't be extracted from the handshake key
material.
Change-Id: I247a1d0813b7a434b3cfc88db86d2fe8754344b6
Reviewed-on: https://boringssl-review.googlesource.com/6433
Reviewed-by: Adam Langley <agl@google.com>
They run through completely different logic as only handshake is fragmented.
This'll make it easier to rewrite the handshake logic in a follow-up.
Change-Id: I9515feafc06bf069b261073873966e72fcbe13cb
Reviewed-on: https://boringssl-review.googlesource.com/6420
Reviewed-by: Adam Langley <agl@google.com>
The documentation in md32_common.h is now (more) correct with respect
to the most important details of the layout of |HASH_CTX|. The
documentation explaining why sha512.c doesn't use md32_common.h is now
more accurate as well.
Before, the C implementations of HASH_BLOCK_DATA_ORDER took a pointer
to the |HASH_CTX| and the assembly language implementations took a
pointer to the hash state |h| member of |HASH_CTX|. (This worked
because |h| is always the first member of |HASH_CTX|.) Now, the C
implementations take a pointer directly to |h| too.
The definitions of |MD4_CTX|, |MD5_CTX|, and |SHA1_CTX| were changed to
be consistent with |SHA256_CTX| and |SHA512_CTX| in storing the hash
state in an array. This will break source compatibility with any
external code that accesses the hash state directly, but will not
affect binary compatibility.
The second parameter of |HASH_BLOCK_DATA_ORDER| is now of type
|const uint8_t *|; previously it was |void *| and all implementations
had a |uint8_t *data| variable to access it as an array of bytes.
This change paves the way for future refactorings such as automatically
generating the |*_Init| functions and/or sharing one I-U-F
implementation across all digest algorithms.
Change-Id: I6e9dd09ff057c67941021d324a4fa1d39f58b0db
Reviewed-on: https://boringssl-review.googlesource.com/6405
Reviewed-by: Adam Langley <agl@google.com>
Although those are only created by code owned by RSA_METHOD, custom RSA_METHODs
shouldn't be allowed to squat our internal fields and then change how you free
things.
Remove 'method' from their names now that they're not method-specific.
Change-Id: I9494ef9a7754ad59ac9fba7fd463b3336d826e0b
Reviewed-on: https://boringssl-review.googlesource.com/6423
Reviewed-by: Adam Langley <agl@google.com>
This will allow a static linker (with -ffunction-sections since things aren't
split into files) to drop unused parts of DH and DSA. Notably, the parameter
generation bits pull in primality-checking code.
Change-Id: I25087e4cb91bc9d0ab43bcb267c2e2c164e56b59
Reviewed-on: https://boringssl-review.googlesource.com/6388
Reviewed-by: Adam Langley <agl@google.com>
Removing the function codes continued to sample __func__ for compatibility with
ERR_print_errors_cb, but not ERR_error_string_n. We can just emit
OPENSSL_internal for both. ERR_print_errors_cb already has the file and line
number available which is strictly more information than the function name.
(ERR_error_string_n does not, but we'd already turned that to
OPENSSL_internal.)
This shaves 100kb from a release build of the bssl tool.
In doing so, put an unused function code parameter back into ERR_put_error to
align with OpenSSL. We don't need to pass an additional string in anymore, so
OpenSSL compatibility with anything which uses ERR_LIB_USER or
ERR_get_next_error_library costs nothing. (Not that we need it.)
Change-Id: If6af34628319ade4145190b6f30a0d820e00b20d
Reviewed-on: https://boringssl-review.googlesource.com/6387
Reviewed-by: Adam Langley <agl@google.com>
This option causes clients to ignore HelloRequest messages completely.
This can be suitable in cases where a server tries to perform concurrent
application data and handshake flow, e.g. because they are trying to
“renew” symmetric keys.
Change-Id: I2779f7eff30d82163f2c34a625ec91dc34fab548
Reviewed-on: https://boringssl-review.googlesource.com/6431
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The documentation in md32_common.h is now (more) correct with respect
to the most important details of the layout of |HASH_CTX|. The
documentation explaining why sha512.c doesn't use md32_common.h is now
more accurate as well.
Before, the C implementations of HASH_BLOCK_DATA_ORDER took a pointer
to the |HASH_CTX| and the assembly language implementations tool a
pointer to the hash state |h| member of |HASH_CTX|. (This worked
because |h| is always the first member of |HASH_CTX|.) Now, the C
implementations take a pointer directly to |h| too.
The definitions of |MD4_CTX|, |MD5_CTX|, and |SHA1_CTX| were changed to
be consistent with |SHA256_CTX| and |SHA512_CTX| in storing the hash
state in an array. This will break source compatibility with any
external code that accesses the hash state directly, but will not
affect binary compatibility.
The second parameter of |HASH_BLOCK_DATA_ORDER| is now of type
|const uint8_t *|; previously it was |void *| and all implementations
had a |uint8_t *data| variable to access it as an array of bytes.
This change paves the way for future refactorings such as automatically
generating the |*_Init| functions and/or sharing one I-U-F
implementation across all digest algorithms.
Change-Id: I30513bb40b5f1d2c8932551d54073c35484b3f8b
Reviewed-on: https://boringssl-review.googlesource.com/6401
Reviewed-by: Adam Langley <agl@google.com>
BN_mod_exp_mont_consttime does not modify its |BN_MONT_CTX| so that
value should be const.
Change-Id: Ie74e48eec8061899fd056fbd99dcca2a86b02cad
Reviewed-on: https://boringssl-review.googlesource.com/6403
Reviewed-by: Adam Langley <agl@google.com>
Although the DTLS transport layer logic drops failed writes on the floor, it is
actually set up to work correctly. If an SSL_write fails at the transport,
dropping the buffer is fine. Arguably it works better than in TLS because we
don't have the weird "half-committed to data" behavior. Likewise, the handshake
keeps track of how far its gotten and resumes the message at the right point.
This broke when the buffering logic was rewritten because I didn't understand
what the DTLS code was doing. The one thing that doesn't work as one might
expect is non-fatal write errors during rexmit are not recoverable. The next
timeout must fire before we try again.
This code is quite badly sprinkled in here, so add tests to guard it against
future turbulence. Because of the rexmit issues, the tests need some hacks
around calls which may trigger them. It also changes the Go DTLS implementation
from being completely strict about sequence numbers to only requiring they be
monotonic.
The tests also revealed another bug. This one seems to be upstream's fault, not
mine. The logic to reset the handshake hash on the second ClientHello (in the
HelloVerifyRequest case) was a little overenthusiastic and breaks if the
ClientHello took multiple tries to send.
Change-Id: I9b38b93fff7ae62faf8e36c4beaf848850b3f4b9
Reviewed-on: https://boringssl-review.googlesource.com/6417
Reviewed-by: Adam Langley <agl@google.com>
This is a fairly timid, first step at trying to pack common structures a
little better.
This change reorders a couple of structures a little and turns some
variables into bit-fields. Much more can still be done.
Change-Id: Idbe0f54d66559c0ad654bf7e8dea277a771a568f
Reviewed-on: https://boringssl-review.googlesource.com/6394
Reviewed-by: Adam Langley <agl@google.com>
QUIC code references the TXT macro. Also get rid of
TLS1_TXT_DHE_RSA_WITH_CHACHA20_POLY1305 which wasn't renamed for some reason.
Change-Id: I0308e07104b3cec394d748f3f1146bd786d2ace2
Reviewed-on: https://boringssl-review.googlesource.com/6384
Reviewed-by: Adam Langley <agl@google.com>
WebRTC can't roll into Chromium without picking up the iOS build fix, but we
can't roll BoringSSL forwards because WebRTC also depends on the previously
exposed ChaCha20-Poly1305 cipher suite constants.
Define the old constants again.
Change-Id: If8434a0317e42b3aebe1bc1c5a58ed97a89a0230
Reviewed-on: https://boringssl-review.googlesource.com/6382
Reviewed-by: Adam Langley <agl@google.com>
EVP_aead_chacha20_poly1305_old is listed twice instead of
EVP_aead_chacha20_poly1305.
Change-Id: I281eee7a8359cd2a2b04047c829ef351ea4a7b82
Reviewed-on: https://boringssl-review.googlesource.com/6381
Reviewed-by: Adam Langley <agl@google.com>
The other codepath is Linux-specific. This should get tidied up a bit but, in
the meantime, fix the Chromium iOS (NO_ASM) build. Even when the assembly gets
working, it seems iOS prefers you make fat binaries rather than detect features
at runtime, so this is what we want anyway.
BUG=548539
Change-Id: If19b2e380a96918b07bacc300a3a27b885697b99
Reviewed-on: https://boringssl-review.googlesource.com/6380
Reviewed-by: Adam Langley <agl@google.com>
1. Check for the presence of the private key before allocating or
computing anything.
2. Check the return value of |BN_CTX_get|.
3. Don't bother computing the Y coordinate since it is not used.
4. Remove conditional logic in cleanup section.
Change-Id: I4d8611603363c7e5d16a8e9f1d6c3a56809f27ae
Reviewed-on: https://boringssl-review.googlesource.com/6171
Reviewed-by: Adam Langley <alangley@gmail.com>
These functions ultimately return the result of |BN_num_bits|, and that
function's return type is |unsigned|. Thus, these functions' return
type should also be |unsigned|.
Change-Id: I2cef63e6f75425857bac71f7c5517ef22ab2296b
Reviewed-on: https://boringssl-review.googlesource.com/6170
Reviewed-by: Adam Langley <alangley@gmail.com>
This came up and I wasn't sure which it was without source-diving.
Change-Id: Ie659096e0f42a7448f81dfb1006c125d292fd7fd
Reviewed-on: https://boringssl-review.googlesource.com/6354
Reviewed-by: Adam Langley <alangley@gmail.com>
QUIC has a complex relationship with BoringSSL owing to it living both
in Chromium and the Google-internal repository. In order for it to
handle the ChaCha20-Poly1305 AEAD switch more easily this change gives
the unsuffixed name to the old AEAD, for now.
Once QUIC has moved to the “_old” version the unsuffixed name can be
given to the new version.
Change-Id: Id8a77be6e3fe2358d78e022413fe088e5a274dca
Reviewed-on: https://boringssl-review.googlesource.com/6361
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
This change reduces unnecessary copying and makes the pre-RFC-7539
nonces 96 bits just like the AES-GCM, AES-CCM, and RFC 7539
ChaCha20-Poly1305 cipher suites. Also, all the symbols related to
the pre-RFC-7539 cipher suites now have "_OLD" appended, in
preparation for adding the RFC 7539 variants.
Change-Id: I1f85bd825b383c3134df0b6214266069ded029ae
Reviewed-on: https://boringssl-review.googlesource.com/6103
Reviewed-by: Adam Langley <alangley@gmail.com>
The new function |CRYPTO_chacha_96_bit_nonce_from_64_bit_nonce| can be
used to adapt code from that uses 64 bit nonces, in a way that is
compatible with the old semantics.
Change-Id: I83d5b2d482e006e82982f58c9f981e8078c3e1b0
Reviewed-on: https://boringssl-review.googlesource.com/6100
Reviewed-by: Adam Langley <alangley@gmail.com>
It seems OS X actually cares about symbol resolution and dependencies
when you create a dylib. Probably because they do two-level name
resolution.
(Obligatory disclaimer: BoringSSL does not have a stable ABI and is thus
not suitable for a traditional system-wide library.)
BUG=539603
Change-Id: Ic26c4ad23840fe6c1f4825c44671e74dd2e33870
Reviewed-on: https://boringssl-review.googlesource.com/6131
Reviewed-by: Adam Langley <alangley@gmail.com>
gcm_test.cc needs to access the internal GCM symbols. This is
unfortunate because it means that they have to be marked OPENSSL_EXPORT
just for this.
To compensate, modes.h is removed and its contents copied into
crypto/modes/internal.h.
Change-Id: I1777b2ef8afd154c43417137673a28598a7ec30e
Reviewed-on: https://boringssl-review.googlesource.com/6360
Reviewed-by: Adam Langley <alangley@gmail.com>
This removes the confusion about whether |gcm128_context| copies the
key (it didn't) or whether the caller is responsible for keeping the
key alive for the lifetime of the |gcm128_context| (it was).
Change-Id: Ia0ad0a8223e664381fbbfb56570b2545f51cad9f
Reviewed-on: https://boringssl-review.googlesource.com/6053
Reviewed-by: Adam Langley <alangley@gmail.com>
The key is never modified through the key pointer member, and the
calling code relies on that fact for maintaining its own
const-correctness.
Change-Id: I63946451aa7c400cd127895a61c30d9a647b1b8c
Reviewed-on: https://boringssl-review.googlesource.com/6040
Reviewed-by: Adam Langley <alangley@gmail.com>
∙ host:port parsing, where unavoidable, is now IPv6-friendly.
∙ |BIO_C_GET_CONNECT| is simply removed.
∙ bssl -accept now listens on both IPv6 and IPv4.
Change-Id: I1cbd8a79c0199bab3ced4c4fd79d2cc5240f250c
Reviewed-on: https://boringssl-review.googlesource.com/6214
Reviewed-by: Adam Langley <alangley@gmail.com>
Right whether NPN is advertised can only be configured globally on the SSL_CTX.
Rather than adding two pointers to each SSL*, add an options bit to disable it
so we may plumb in a field trial to disable NPN.
Chromium wants to be able to route a bit in to disable NPN, but it uses SSL_CTX
incorrectly and has a global one, so it can't disconnect the callback. (That
really needs to get fixed. Although it's not clear this necessarily wants to be
lifted up to SSL_CTX as far as Chromium's SSLClientSocket is concerned since
NPN doesn't interact with the session cache.)
BUG=526713
Change-Id: I49c86828b963eb341c6ea6a442557b7dfa190ed3
Reviewed-on: https://boringssl-review.googlesource.com/6351
Reviewed-by: Adam Langley <alangley@gmail.com>
One less exported function. Nothing ever stack-allocates them, within BoringSSL
or in consumers. This avoids the slightly odd mechanism where BN_MONT_CTX_free
might or might not free the BN_MONT_CTX itself based on a flag.
(This is also consistent with OpenSSL 1.1.x which does away with the _init
variants of both this and BIGNUM so it shouldn't be a compatibility concern
long-term either.)
Change-Id: Id885ae35a26f75686cc68a8aa971e2ea6767ba88
Reviewed-on: https://boringssl-review.googlesource.com/6350
Reviewed-by: Adam Langley <alangley@gmail.com>
The internal session cache is keyed on session ID, so this is completely
useless for clients (indeed we never look it up internally). Along the way,
tidy up ssl_update_cache to be more readable. The slight behavior change is
that SSL_CTX_add_session's return code no longer controls the external
callback. It's not clear to me what that could have accomplished. (It can only
fail on allocation error. We only call it for new sessions, so the duplicate
case is impossible.)
The one thing of value the internal cache might have provided is managing the
timeout. The SSL_CTX_flush_sessions logic would flip the not_resumable bit and
cause us not to offer expired sessions (modulo SSL_CTX_flush_sessions's delay
and any discrepancies between the two caches). Instead, just check expiration
when deciding whether or not to offer a session.
This way clients that set SSL_SESS_CACHE_CLIENT blindly don't accidentally
consume gobs of memory.
BUG=531194
Change-Id: If97485beab21874f37737edc44df24e61ce23705
Reviewed-on: https://boringssl-review.googlesource.com/6321
Reviewed-by: Adam Langley <alangley@gmail.com>
In doing so, fix the documentation for SSL_CTX_add_session and
SSL_CTX_remove_session. I misread the code and documented the behavior
on session ID collision wrong.
Change-Id: I6f364305e1f092b9eb0b1402962fd04577269d30
Reviewed-on: https://boringssl-review.googlesource.com/6319
Reviewed-by: Adam Langley <alangley@gmail.com>
A random 32-byte (so 256-bit) session ID is never going to collide with
an existing one. (And, if it does, SSL_CTX_add_session does account for
this, so the server won't explode. Just attempting to resume some
session will fail.)
That logic didn't completely work anyway as it didn't account for
external session caches or multiple connections picking the same ID in
parallel (generation and insertion happen at different times) or
multiple servers sharing one cache. In theory one could fix this by
passing in a sufficiently clever generate_session_id, but no one does
that.
I found no callers of these functions, so just remove them altogether.
Change-Id: I8500c592cf4676de6d7194d611b99e9e76f150a7
Reviewed-on: https://boringssl-review.googlesource.com/6318
Reviewed-by: Adam Langley <alangley@gmail.com>
Private structs shouldn't be shown. Also there's a few sections that are
really more implementation details than anything else.
Change-Id: Ibc5a23ba818ab0531d9c68e7ce348f1eabbcd19a
Reviewed-on: https://boringssl-review.googlesource.com/6313
Reviewed-by: Adam Langley <alangley@gmail.com>
Although Chromium actually uses SSL_(get_)state as part of its fallback
reason heuristic, that function really should go in the deprecated
bucket. I kept SSL_state_string_long since having a human-readable
string is probably useful for logging.
SSL_set_SSL_CTX was only half-documented as the behavior of this
function is very weird. This warrants further investigation and
rethinking.
SSL_set_shutdown is absurd. I added an assert to trip up clearing bits
and set it to a bitwise OR since clearing bits may mess up the state
machine. Otherwise there's enough consumers and it's not quite the same
as SSL_CTX_set_quiet_shutdown that I've left it alone for now.
Change-Id: Ie35850529373a5a795f6eb04222668ff76d84aaa
Reviewed-on: https://boringssl-review.googlesource.com/6312
Reviewed-by: Adam Langley <alangley@gmail.com>
It just calls CRYPTO_library_init and doesn't do anything else. If
anything, I'd like to make CRYPTO_library_init completely go away too.
We have CRYPTO_once now, so I think it's safe to assume that, if ssl/
ever grows initialization needs beyond that of crypto/, we can hide it
behind a CRYPTO_once and not burden callers.
Change-Id: I63dc362e0e9e98deec5516f4620d1672151a91b6
Reviewed-on: https://boringssl-review.googlesource.com/6311
Reviewed-by: Adam Langley <alangley@gmail.com>
SSL_in_connect_init and SSL_in_accept_init are removed as they're unused
both within the library and externally. They're also kind of silly.
Expand on how False Start works at the API level in doing so.
Change-Id: Id2a8e34b5bb8f28329e3b87b4c64d41be3f72410
Reviewed-on: https://boringssl-review.googlesource.com/6310
Reviewed-by: Adam Langley <alangley@gmail.com>
They were since added to crypto.h and implemented in the library proper.
Change-Id: Idaa2fe2d9b213e67cf7ef61ff8bfc636dfa1ef1f
Reviewed-on: https://boringssl-review.googlesource.com/6309
Reviewed-by: Adam Langley <alangley@gmail.com>
They're really not all that helpful, considering they're each used
exactly once. They're also confusing as it is ALMOST the case that
SSL_TXT_FOO expands to "FOO", but SSL_TXT_AES_GCM expand "AESGCM" and
the protocol versions have lowercase v's and dots.
Change-Id: If78ad8edb0c024819219f61675c60c2a7f3a36b0
Reviewed-on: https://boringssl-review.googlesource.com/6307
Reviewed-by: Adam Langley <alangley@gmail.com>
This callback is some combination of arguably useful stuff (bracket
handshakes, alerts) and completely insane things (find out when the
state machine advances). Deprecate the latter.
Change-Id: Ibea5b32cb360b767b0f45b302fd5f1fe17850593
Reviewed-on: https://boringssl-review.googlesource.com/6305
Reviewed-by: Adam Langley <alangley@gmail.com>
Also clean up the code slightly.
Change-Id: I066a389242c46cdc7d41b1ae9537c4b7716c92a2
Reviewed-on: https://boringssl-review.googlesource.com/6302
Reviewed-by: Adam Langley <alangley@gmail.com>
Like tls1.h, ssl3.h is now just a bundle of protocol constants.
Hopefully we can opaquify this struct in due time, but for now it's
still public.
Change-Id: I68366eb233702e149c92e21297f70f8a4a45f060
Reviewed-on: https://boringssl-review.googlesource.com/6300
Reviewed-by: Adam Langley <alangley@gmail.com>
This dates all the way to SSLeay 0.9.0b. At this point the
application/handshake interleave logic in ssl3_read_bytes was already
present:
((
(s->state & SSL_ST_CONNECT) &&
(s->state >= SSL3_ST_CW_CLNT_HELLO_A) &&
(s->state <= SSL3_ST_CR_SRVR_HELLO_A)
) || (
(s->state & SSL_ST_ACCEPT) &&
(s->state <= SSL3_ST_SW_HELLO_REQ_A) &&
(s->state >= SSL3_ST_SR_CLNT_HELLO_A)
)
The comment is attached to SSL3_ST_SR_CLNT_HELLO_A, so I suspect this is
what it was about. This logic is gone now, so let's remove that scary
warning.
Change-Id: I45f13b53b79e35d80e6074b0942600434deb0684
Reviewed-on: https://boringssl-review.googlesource.com/6299
Reviewed-by: Adam Langley <alangley@gmail.com>
Now tls1.h is just a pile of protocol constants with no more circular
dependency problem.
I've preserved SSL_get_servername's behavior where it's simultaneously a
lookup of handshake state and local configuration. I've removed it from
SSL_get_servername_type. It got the logic wrong anyway with the order of
the s->session check.
(Searching through code, neither is used on the client, but the
SSL_get_servername one is easy.)
Change-Id: I61bb8fb0858b07d76a7835bffa6dc793812fb027
Reviewed-on: https://boringssl-review.googlesource.com/6298
Reviewed-by: Adam Langley <alangley@gmail.com>
Some ARM environments don't support |getauxval| or signals and need to
configure the capabilities of the chip at compile time. This change adds
defines that allow them to do so.
Change-Id: I4e6987f69dd13444029bc7ac7ed4dbf8fb1faa76
Reviewed-on: https://boringssl-review.googlesource.com/6280
Reviewed-by: Adam Langley <agl@google.com>
SSL_alert_desc_string_long was kept in the undeprecated bucket and one missing
alert was added. We have some uses and it's not completely ridiculous for
logging purposes.
The two-character one is ridiculous though and gets turned into a stub
that returns a constant string ("!" or "!!") because M2Crypto expects
it.
Change-Id: Iaf8794b5d953630216278536236c7113655180af
Reviewed-on: https://boringssl-review.googlesource.com/6297
Reviewed-by: Adam Langley <alangley@gmail.com>
(Documentation/deprecation will come in later commits.)
Change-Id: I3aba26e32b2e47a1afb5cedd44d09115fc193bce
Reviewed-on: https://boringssl-review.googlesource.com/6296
Reviewed-by: Adam Langley <alangley@gmail.com>
The only reason you'd want it is to tls_unique, and we have a better API
for that. (It has one caller and that is indeed what that caller uses it
for.)
Change-Id: I39f8e353f56f18becb63dd6f7205ad31f4192bfd
Reviewed-on: https://boringssl-review.googlesource.com/6295
Reviewed-by: Adam Langley <alangley@gmail.com>
This is redundant with SSL_get_error. Neither is very good API, but
SSL_get_error is more common. SSL_get_error also takes a return code
which makes it harder to accidentally call it at some a point other than
immediately after an operation. (Any other point is confusing since you
can have SSL_read and SSL_write operations going on in parallel and
they'll get mixed up.)
Change-Id: I5818527c30daac28edb552c6c550c05c8580292d
Reviewed-on: https://boringssl-review.googlesource.com/6294
Reviewed-by: Adam Langley <alangley@gmail.com>
It's pretty clearly pointless to put in the public header.
Change-Id: I9527aba09b618f957618e653c4f2ae379ddd0fdb
Reviewed-on: https://boringssl-review.googlesource.com/6293
Reviewed-by: Adam Langley <alangley@gmail.com>
Also added a SSL_CTX_set_select_certificate_cb setter for
select_certificate_cb so code needn't access SSL_CTX directly. Plus it
serves as a convenient anchor for the documentation.
Change-Id: I23755b910e1d77d4bea7bb9103961181dd3c5efe
Reviewed-on: https://boringssl-review.googlesource.com/6291
Reviewed-by: Adam Langley <alangley@gmail.com>
These are theh two remaining quirks (SSL_OP_LEGACY_SERVER_CONNECT
aside). Add counters so we can determine whether there are still clients
that trip up these cases.
Change-Id: I7e92f42f3830c1df675445ec15a852e5659eb499
Reviewed-on: https://boringssl-review.googlesource.com/6290
Reviewed-by: Adam Langley <alangley@gmail.com>
Start converting the ones we can right now. Some of the messier ones
resize init_buf rather than assume the initial size is sufficient, so
those will probably wait until init_buf is gone and the handshake's
undergone some more invasive surgery. The async ones will also require
some thought. But some can be incrementally converted now.
BUG=468889
Change-Id: I0bc22e4dca37d9d671a488c42eba864c51933638
Reviewed-on: https://boringssl-review.googlesource.com/6190
Reviewed-by: Adam Langley <alangley@gmail.com>
This extends 79c59a30 to |RSA_public_encrypt|, |RSA_private_encrypt|,
and |RSA_public_decrypt|. It benefits Conscrypt, which expects these
functions to have the same signature as |RSA_public_private_decrypt|.
Change-Id: Id1ce3118e8f20a9f43fd4f7bfc478c72a0c64e4b
Reviewed-on: https://boringssl-review.googlesource.com/6286
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It's missing fields and no one ever calls it.
Change-Id: I450edc1e29bb48edffb5fd3df8da19a03e4185ce
Reviewed-on: https://boringssl-review.googlesource.com/5821
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's bf0fc41266f17311c5db1e0541d3dd12eb27deb6.
Change-Id: Ib692b0ad608f2e3291f2aeab2ad98a7e177d5851
Reviewed-on: https://boringssl-review.googlesource.com/6150
Reviewed-by: Adam Langley <agl@google.com>
Grouping along two axes is weird. Doesn't hugely matter which one, but
we should be consistent.
Change-Id: I80fb04d3eff739c08fda29515ce81d101d8542cb
Reviewed-on: https://boringssl-review.googlesource.com/6120
Reviewed-by: Adam Langley <agl@google.com>
The caller obligations for retransmit are messy, so I've peppered a few
other functions with mentions of it. There's only three functions, so
they're lumped in with the other core functions. They're irrelevant for
TLS, but important for DTLS.
Change-Id: Ifc995390952eef81370b58276915dcbe4fc7e3b5
Reviewed-on: https://boringssl-review.googlesource.com/6093
Reviewed-by: Adam Langley <agl@google.com>
Deprecate the client_cert_cb variant since you can't really configure
intermediates with it. (You might be able to by configuring the
intermediates without the leaf or key and leaving the SSL stack to
configure those, but that's really weird. cert_cb is simpler.)
Also document the two functions the callbacks may use to query the
CertificateRequest on the client.
Change-Id: Iad6076266fd798cd74ea4e09978e7f5df5c8a670
Reviewed-on: https://boringssl-review.googlesource.com/6092
Reviewed-by: Adam Langley <agl@google.com>
It doesn't actually do anything.
Change-Id: I8a5748dc86b842406cc656a5b251e1a7c0092377
Reviewed-on: https://boringssl-review.googlesource.com/6090
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: Adam Langley <agl@google.com>
