The SSL23_ST_foo macros are only used in ssl_stat.c.
However, these states are never set and can be removed.
Move the two remaining SSLv2 client hello record macros to ssl3.h
Change-Id: I76055405a9050cf873b4d1cbc689e54dd3490b8a
Reviewed-on: https://boringssl-review.googlesource.com/4160
Reviewed-by: Adam Langley <agl@google.com>
All callers have been moved to EVP_PKEY_up_ref. (Neither spelling exists
upstream so we only had our own callers to move.)
Change-Id: I267f14054780fe3d6dc1170b7b6ae3811a0d1a9a
Reviewed-on: https://boringssl-review.googlesource.com/5291
Reviewed-by: Adam Langley <agl@google.com>
One tedious thing about using CBB is that you can't safely CBB_cleanup
until CBB_init is successful, which breaks the general 'goto err' style
of cleanup. This makes it possible:
CBB_zero ~ EVP_MD_CTX_init
CBB_init ~ EVP_DigestInit
CBB_cleanup ~ EVP_MD_CTX_cleanup
Change-Id: I085ecc4405715368886dc4de02285a47e7fc4c52
Reviewed-on: https://boringssl-review.googlesource.com/5267
Reviewed-by: Adam Langley <agl@google.com>
The name is confusing. EC keys aren't serialized to DER.
DSA keys are also weird, but left alone for now. i2d_DSAPublicKey either
serializes to a DSAPublicKey per RFC 3279 if write_params is 0 or what
seems to be an OpenSSL-specific format that includes the group if
write_params is 1. See upstream's
ea6b07b54c.
Change-Id: I0d15140acc2d688a563b615fc6a9e3abec929753
Reviewed-on: https://boringssl-review.googlesource.com/5261
Reviewed-by: Adam Langley <agl@google.com>
They're all forward-declared. There's no need to use the struct names.
Change-Id: I435ae2f5971128f08c730317ca644d97239f3b54
Reviewed-on: https://boringssl-review.googlesource.com/5260
Reviewed-by: Adam Langley <agl@google.com>
Use more sensible variable names. Also move some work between the helpers and
s3_srvr.c a little; the session lookup functions now only return a new session.
Whether to send a ticket is now an additional output to avoid the enum
explosion around renewal. The actual SSL state is not modified.
This is somewhat cleaner as s3_srvr.c may still reject a session for other
reasons, so we avoid setting ssl->session and ssl->verify_result to a session
that wouldn't be used. (They get fixed up in ssl_get_new_session, so it didn't
actually matter.)
Change-Id: Ib52fabbe993b5e2b7408395a02cdea3dee66df7b
Reviewed-on: https://boringssl-review.googlesource.com/5235
Reviewed-by: Adam Langley <agl@google.com>
This change also switches the behaviour of the client. Previously the
client would send the SCSV rather than the extension, but now it'll only
do that for SSLv3 connections.
Change-Id: I67a04b8abbef2234747c0dac450458deb6b0cd0a
Reviewed-on: https://boringssl-review.googlesource.com/5143
Reviewed-by: Adam Langley <agl@google.com>
Rather than four massive functions that handle every extension,
organise the code by extension with four smaller functions for each.
Change-Id: I876b31dacb05aca9884ed3ae7c48462e6ffe3b49
Reviewed-on: https://boringssl-review.googlesource.com/5142
Reviewed-by: Adam Langley <agl@google.com>
Chromium uses a zygote process and a sandbox on Linux. In order for RAND_bytes
to be functional and guaranteed fork-safe inside the renderers, /dev/urandom
must be prewarmed. Calling RAND_bytes initializes a thread-local ChaCha20 key
when rdrand is available. So that key is fork-safe and to avoid tempting any
dragons by touching pthreads APIs before a non-exec fork, add a
RAND_set_urandom_fd API. It allows the consumer to supply the /dev/urandom fd
and promises to be fork-safe, both in initializing key material and use of
pthreads.
This doesn't affect any current shipping versions of Chrome.
BUG=462040
Change-Id: I1037e21e525918971380e4ea1371703c8237a0b0
Reviewed-on: https://boringssl-review.googlesource.com/5302
Reviewed-by: Adam Langley <agl@google.com>
Having them spread between ssl.h and tls1.h isn't terribly enlightening.
Change-Id: I5fec4b8e5260312b22bcef21bd4db7a8a8149ad8
Reviewed-on: https://boringssl-review.googlesource.com/5234
Reviewed-by: Adam Langley <agl@google.com>
Using the original numerical order made more sense before they were changed to
doesnt_exist.
BUG=404754
Change-Id: I2971eff7c6fbe7c5d340b103de71bbfa180f1f96
Reviewed-on: https://boringssl-review.googlesource.com/5232
Reviewed-by: Adam Langley <agl@google.com>
This removes EVP_PKEY_HMAC and all the support code around it. EVP_MD requires
a lot of extra glue to support HMAC. This lets us prune it all away.
As a bonus, it removes a (minor) dependency from EVP to the legacy ASN.1 stack.
Change-Id: I5a9e3e39f518429828dbf13d14647fb37d9dc35a
Reviewed-on: https://boringssl-review.googlesource.com/5120
Reviewed-by: Adam Langley <agl@google.com>
The callback arguments are required to be NULL.
Change-Id: I266ec46efdaca411a7f0c2b645883b2c5bec1c96
Reviewed-on: https://boringssl-review.googlesource.com/5160
Reviewed-by: Adam Langley <agl@google.com>
They'll probably stay that way too, so document it as being an ignored
parameter.
Change-Id: Iff385715f5413290a7186c38ea9ef2dd4fce9b38
Reviewed-on: https://boringssl-review.googlesource.com/5175
Reviewed-by: Adam Langley <agl@google.com>
Rather than rely on Chromium to query SSL_initial_handshake_complete in the
callback (which didn't work anyway because the callback is called afterwards),
move the logic into BoringSSL. BoringSSL already enforces that clients never
offer resumptions on renegotiation (it wouldn't work well anyway as client
session cache lookup is external), so it's reasonable to also implement
in-library that sessions established on a renegotiation are not cached.
Add a bunch of tests that new_session_cb is called when expected.
BUG=501418
Change-Id: I42d44c82b043af72b60a0f8fdb57799e20f13ed5
Reviewed-on: https://boringssl-review.googlesource.com/5171
Reviewed-by: Adam Langley <agl@google.com>
Also implement it without reference to crypto/asn1 or fake ASN1_INTEGERs and
add a test. Some platform crypto APIs only give back the key size, and not the
encoded signature length. No sense in implementing it twice.
BUG=347404,499653
Change-Id: I9aa27d52674375f8b036e57bb5850f091c9b25dd
Reviewed-on: https://boringssl-review.googlesource.com/5080
Reviewed-by: Adam Langley <agl@google.com>
This adds a new API, SSL_set_private_key_method, which allows the consumer to
customize private key operations. For simplicity, it is incompatible with the
multiple slots feature (which will hopefully go away) but does not, for now,
break it.
The new method is only routed up for the client for now. The server will
require a decrypt hook as well for the plain RSA key exchange.
BUG=347404
Change-Id: I35d69095c29134c34c2af88c613ad557d6957614
Reviewed-on: https://boringssl-review.googlesource.com/5049
Reviewed-by: Adam Langley <agl@google.com>
Turns out the safer/simpler method still wasn't quite right. :-)
session->sess_cert isn't serialized and deserialized, which is poor. Duplicate
it manually for now. Leave a TODO to get rid of that field altogether as it's
not especially helpful. The certificate-related fields should be in the
session. The others probably have no reason to be preserved on resumptions at
all.
Test by making bssl_shim.cc assert the peer cert chain is there or not as
expected.
BUG=501220
Change-Id: I44034167629720d6e2b7b0b938d58bcab3ab0abe
Reviewed-on: https://boringssl-review.googlesource.com/5170
Reviewed-by: Adam Langley <agl@google.com>
To account for the changes in ticket renewal, Chromium will need to listen for
new_session_cb to determine whether the handshake produced a new session.
Chromium currently never caches sessions produced on a renegotiation. To retain
that behavior, it'll need to know whether new_session_cb is initial or not.
Rather than maintain duplicate state and listen for SSL_HANDSHAKE_DONE, it's
simpler to just let it query ssl->s3->initial_handshake_complete.
BUG=501418
Change-Id: Ib2f2541460bd09cf16106388e9cfdf3662e02681
Reviewed-on: https://boringssl-review.googlesource.com/5126
Reviewed-by: Adam Langley <agl@google.com>
Platform crypto APIs for PKCS#1 RSA signatures vary between expecting the
caller to prepend the DigestInfo prefix (RSA_sign_raw) and prepending it
internally (RSA_sign). Currently, Chromium implements sign or sign_raw as
appropriate. To avoid needing both variants, the new asynchronous methods will
only expose the higher-level one, sign.
To satisfy ports which previously implemented sign_raw, expose the DigestInfo
prefix as a utility function.
BUG=347404
Change-Id: I04c397b5e9502b2942f6698ecf81662a3c9282e6
Reviewed-on: https://boringssl-review.googlesource.com/4940
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's 27c76b9b8010b536687318739c6f631ce4194688, CVE-2015-1791.
Rather than write a dup function, serializing and deserializing the object is
simpler. It also fixes a bug in the original fix where it never calls
new_session_cb to store the new session (for clients which use that callback;
how clients should handle the session cache is much less clear).
The old session isn't pruned as we haven't processed the Finished message yet.
RFC 5077 says:
The server MUST NOT assume that the client actually received the updated
ticket until it successfully verifies the client's Finished message.
Moreover, because network messages are asynchronous, a new SSL connection may
have began just before the client received the new ticket, so any such servers
are broken regardless.
Change-Id: I13b3dc986dc58ea2ce66659dbb29e14cd02a641b
Reviewed-on: https://boringssl-review.googlesource.com/5122
Reviewed-by: Adam Langley <agl@google.com>
Mirrors SSL_SESSION_to_bytes. It avoids having to deal with object-reuse, the
non-size_t length parameter, and trailing data. Both it and the object-reuse
variant back onto an unexposed SSL_SESSION_parse which reads a CBS.
Note that this changes the object reuse story slightly. It's now merely an
optional output pointer that frees its old contents. No d2i_SSL_SESSION
consumer in Google that's built does reuse, much less reuse with the assumption
that the top-level object won't be overridden.
Change-Id: I5cb8522f96909bb222cab0f342423f2dd7814282
Reviewed-on: https://boringssl-review.googlesource.com/5121
Reviewed-by: Adam Langley <agl@google.com>
We had aarch64 handled twice, which was a mistake.
Change-Id: Id27fc86cb701a87c11c54b98534108f87e49262d
Reviewed-on: https://boringssl-review.googlesource.com/5131
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
BN_copy can fail on malloc failure. The case in crypto/rsa was causing the
malloc tests in all_tests.go to infinite loop.
Change-Id: Id5900512013fba9960444d78a8c056aa4314fb2d
Reviewed-on: https://boringssl-review.googlesource.com/5110
Reviewed-by: Adam Langley <agl@google.com>
Some of the documentation had the right explanation but the incorrect
function names attached.
Change-Id: I7b479dae6d71a5ac7bc86df5a3890508c3b3d09f
Reviewed-on: https://boringssl-review.googlesource.com/5090
Reviewed-by: Adam Langley <agl@google.com>
If we're going to have PSK and use standard cipher suites, this might be
the best that we can do for the moment.
Change-Id: I35d9831b2991dc5b23c9e24d98cdc0db95919d39
Reviewed-on: https://boringssl-review.googlesource.com/5052
Reviewed-by: Adam Langley <agl@google.com>
This is the best PSK cipher suite, but it's non-standard and nobody is
using it. Trivial to bring back in the future if we have need of it.
Change-Id: Ie78790f102027c67d1c9b19994bfb10a2095ba92
Reviewed-on: https://boringssl-review.googlesource.com/5051
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
We shouldn't have protocol constraints that are sensitive to whether
data is returned synchronously or not.
Per https://boringssl-review.googlesource.com/#/c/4112/, the original
limitation was to avoid OpenSSL ABI changes. This is no longer a
concern.
Add tests for the sync and async case. Send the empty records in two
batches to ensure the count is reset correctly.
Change-Id: I3fee839438527e71adb83d437879bb0d49ca5c07
Reviewed-on: https://boringssl-review.googlesource.com/5040
Reviewed-by: Adam Langley <agl@google.com>
We have need of it internally.
Change-Id: I564af468728b22245e8eab384ea7018b7e88cc86
Reviewed-on: https://boringssl-review.googlesource.com/5022
Reviewed-by: Matt Braithwaite <mab@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Ic82ab5de4e231cdf6230ee7262c3c7539404d4a6
Reviewed-on: https://boringssl-review.googlesource.com/5020
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This change makes |CBS_get_any_asn1_element| only handle DER elements.
Another function, |CBS_get_any_ber_asn1_element| is exposed internally
for the cases where we need to process BER data.
Change-Id: I544141a1a3d7913986352a8fd9a6d00b9f282652
Reviewed-on: https://boringssl-review.googlesource.com/4994
Reviewed-by: Adam Langley <agl@google.com>
The client and server both have to decide on behaviour when resuming a
session where the EMS state of the session doesn't match the EMS state
as exchanged in the handshake.
Original handshake
| No Yes
------+--------------------------------------------------------------
|
R | Server: ok [1] Server: abort [3]
e No | Client: ok [2] Client: abort [4]
s |
u |
m |
e |
Yes | Server: don't resume No problem
| Client: abort; server
| shouldn't have resumed
[1] Servers want to accept legacy clients. The draft[5] says that
resumptions SHOULD be rejected so that Triple-Handshake can't be done,
but we'll rather enforce that EMS was used when using tls-unique etc.
[2] The draft[5] says that even the initial handshake should be aborted
if the server doesn't support EMS, but we need to be able to talk to the
world.
[3] This is a very weird case where a client has regressed without
flushing the session cache. Hopefully we can be strict and reject these.
[4] This can happen when a server-farm shares a session cache but
frontends are not all updated at once. If Chrome is strict here then
hopefully we can prevent any servers from existing that will try to
resume an EMS session that they don't understand. OpenSSL appears to be
ok here: https://www.ietf.org/mail-archive/web/tls/current/msg16570.html
[5] https://tools.ietf.org/html/draft-ietf-tls-session-hash-05#section-5.2
BUG=492200
Change-Id: Ie1225a3960d49117b05eefa5a36263d8e556e467
Reviewed-on: https://boringssl-review.googlesource.com/4981
Reviewed-by: Adam Langley <agl@google.com>
This implementation does not prompt for a password. It's just enough
to ensure that the many functions that take a tuple of
|pem_password_cb| and a |void *| to a password work in a reasonable
way when the latter is non-NULL.
Change-Id: Ic6bfc484630c67b5ede25277e14eb3b00c2024f0
Reviewed-on: https://boringssl-review.googlesource.com/4990
Reviewed-by: Adam Langley <agl@google.com>
The only flag is EVP_MD_CTX_FLAG_NO_INIT and no good can possibly come of
anyone outside EVP_PKEY_HMAC calling it. (And indeed no one calls it.
EVP_MD_CTX_set_flags has a caller in wpa_supplicant, but it uses
EVP_MD_CTX_FLAG_NON_FIPS_ALLOW which we don't define. The call is guarded by a
pair of ifdefs for some FIPS mode wpa_supplicant.)
Change-Id: I70ab8ffa646f3f75dfa4d37c96b9e82448ff1e40
Reviewed-on: https://boringssl-review.googlesource.com/4971
Reviewed-by: Adam Langley <agl@google.com>
It's never called externally and for good reason; the only flag to set is
EVP_MD_CTX_FLAG_NO_INIT which is an implementation detail of EVP_PKEY_HMAC
(hopefully to be removed eventually). Indeed, only EVP_PKEY_HMAC ever calls
this function. Except there's no need to because the HMAC_CTX has already been
initialized at that point. (And were it not initialized, that call would not
bode well for the poor HMAC_CTX.)
The legacy EVP_PKEY_HMAC API has test coverage and still works after this
change.
Change-Id: I2fb0bede3c24ad1519f9433f957606de15ba86c7
Reviewed-on: https://boringssl-review.googlesource.com/4970
Reviewed-by: Adam Langley <agl@google.com>
Matt Braithwaite
Håvard Molland
David Benjamin
Adam Langley
Adam Langley
Kenny Root