SSL3_VERSION_MAJOR is the only MAJOR/MINOR number used internally or
externally.
Change-Id: I3f17175e73fd89887665accf1bfa680581f42dfe
Reviewed-on: https://boringssl-review.googlesource.com/4790
Reviewed-by: Adam Langley <agl@google.com>
Chromium's session cache has since been rewritten and no longer needs to
muck with those functions in tests.
Change-Id: I2defad81513210dca5e105757e04cbb677583251
Reviewed-on: https://boringssl-review.googlesource.com/4788
Reviewed-by: Adam Langley <agl@google.com>
Current thought is to organize this by:
- Core SSL_CTX APIs (creating, destroying)
- Core SSL APIs (creating destroying, maybe handshake, read, write as
well)
- APIs to configure SSL_CTX/SSL, roughly grouped by feature. Probably
options and modes are the first two sections. SSL_TXT_* constants can
be part of documenting cipher suite configuration.
- APIs to query state from SSL_CTX/SSL, roughly grouped by feature. (Or
perhaps these should be folded into the configuration sections?)
The functions themselves aren't reordered or reorganized to match the
eventual header order yet. Though I did do the s -> ssl rename on the
ones I've touched.
Also formally deprecate SSL_clear. It would be a core SSL API
except it's horrible.
Change-Id: Ia7e4fdcb7bad4e9ccdee8cf8c3136dc63aaaa772
Reviewed-on: https://boringssl-review.googlesource.com/4784
Reviewed-by: Adam Langley <agl@google.com>
The type names are perfectly serviceable. Most of them are
forward-declared in base.h.
Change-Id: Id03f5039a2d1bab82c68ade074a0e26cd3ab5ad9
Reviewed-on: https://boringssl-review.googlesource.com/4783
Reviewed-by: Adam Langley <agl@google.com>
Looks like it was the use in type_check.h that was still causing
problems, not that MSVC doesn't short-circuit #if statements.
Change-Id: I574e8dd463c46b0133a989b221a7bb8861b3eed9
At this point, none of these functions or macros are used so they can
just be deleted.
Change-Id: I8ed1aae7a252e886864bf43e3096eff2228183cd
Reviewed-on: https://boringssl-review.googlesource.com/4777
Reviewed-by: Adam Langley <agl@google.com>
These ASN.1 macros are the last references to the old-style OpenSSL
locks that remain. The ASN.1 reference count handling was changed in a
previous commit to use |CRYPTO_refcount_*| so these lock references were
unused anyway.
Change-Id: I1b27eef140723050a8e6878a1bea11da3409d0eb
Reviewed-on: https://boringssl-review.googlesource.com/4776
Reviewed-by: Adam Langley <agl@google.com>
|SSL_CTX| and |X509_STORE| have grown their own locks. Several static
locks have been added to hack around not being able to use a
|CRYPTO_once_t| in public headers. Lastly, support for calling
|SSL_CTX_set_generate_session_id| concurrently with active connections
has been removed. No other property of an |SSL_CTX| works like that.
Change-Id: Iff5fe3ee3fdd6ea9c9daee96f850b107ad8a6bca
Reviewed-on: https://boringssl-review.googlesource.com/4775
Reviewed-by: Adam Langley <agl@google.com>
It's no longer needed after the conversion to |CRYPTO_refcount_t|.
Change-Id: Ied129c4c247fcd426745fa016350528b7571aaaa
Reviewed-on: https://boringssl-review.googlesource.com/4774
Reviewed-by: Adam Langley <agl@google.com>
Convert reference counts in ssl/ to use |CRYPTO_refcount_t|.
Change-Id: I5d60f641b0c89b1ddfe38bfbd9d7285c60377f4c
Reviewed-on: https://boringssl-review.googlesource.com/4773
Reviewed-by: Adam Langley <agl@google.com>
This change converts the reference counts in crypto/ to use
|CRYPTO_refcount_t|. The reference counts in |X509_PKEY| and |X509_INFO|
were never actually used and so were dropped.
Change-Id: I75d572cdac1f8c1083c482e29c9519282d7fd16c
Reviewed-on: https://boringssl-review.googlesource.com/4772
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL has traditionally done reference counting with |int|s and the
|CRYPTO_add| function. Unless a special callback is installed (rare),
this is implemented by doing the reference count operations under a
lock.
This change adds infrastructure for handling reference counts and uses
atomic operations when C11 support is available.
Change-Id: Ia023ce432319efd00f77a7340da27d16ee4b63c3
Reviewed-on: https://boringssl-review.googlesource.com/4771
Reviewed-by: Adam Langley <agl@google.com>
OPENSSL_COMPILE_ASSERT implements a static assertion, but the error
message is a little weird because it's a hack around the fact that C,
traditionally, doesn't have static assertions.
C11 now does have _Static_assert (a.k.a. static_assert when one includes
assert.h) so we can use that when provided to get cleaner error
messages.
Change-Id: Ia3625dfb2988de11fd95ddba957f118c0d3183ff
Reviewed-on: https://boringssl-review.googlesource.com/4770
Reviewed-by: Adam Langley <agl@google.com>
DH groups less than 1024 bits are clearly not very safe. Ideally servers
would switch to ECDHE because 1024 isn't great either, but this will
serve for the short term.
BUG=490240
Change-Id: Ic9aac714cdcdcbfae319b5eb1410675d3b903a69
Reviewed-on: https://boringssl-review.googlesource.com/4813
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
SSLeay is a compatibility function for OpenSSL, but I got it wrong. It
doesn't return a string, it returns a number. This doesn't end up making
any difference, but it fixes a warning when building OpenSSH.
Change-Id: I327ab4f70313c93c18f81d8804ba4acdc3bc1a4a
Reviewed-on: https://boringssl-review.googlesource.com/4811
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This change exposes the functions needed to support arbitrary elliptic
curve groups. The Java API[1] doesn't allow a provider to only provide
certain elliptic curve groups. So if BoringSSL is an ECC provider on
Android, we probably need to support arbitrary groups because someone
out there is going to be using it for Bitcoin I'm sure.
Perhaps in time we can remove this support, but not yet.
[1] https://docs.oracle.com/javase/7/docs/api/java/security/spec/ECParameterSpec.html
Change-Id: Ic1d76de96f913c9ca33c46b451cddc08c5b93d80
Reviewed-on: https://boringssl-review.googlesource.com/4740
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
SSL_get_current_cipher is documented by upstream to return the cipher actually
being used. However, because it reads s->session, it returns information
pertaining to the session to be offered if queried before ServerHello or early
in an abbreviated handshake.
Logic around s->session needs more comprehensive cleanup but for just this
function, defining it to be the current outgoing cipher is close to the current
semantics but for fixing the initial state (s->session->cipher is populated
when sending CCS). Store it in the SSL_AEAD_CTX which seems a natural place to
associate state pertaining to a connection half.
BUG=484744
Change-Id: Ife8db27a16615d0dbb2aec65359537243e08af7c
Reviewed-on: https://boringssl-review.googlesource.com/4733
Reviewed-by: Adam Langley <agl@google.com>
This cuts down on one config knob as well as one case in the renego
combinatorial explosion. Since the only case we care about with renego
is the client auth hack, there's no reason to ever do resumption.
Especially since, no matter what's in the session cache:
- OpenSSL will only ever offer the session it just established,
whether or not a newer one with client auth was since established.
- Chrome will never cache sessions created on a renegotiation, so
such a session would never make it to the session cache.
- The new_session + SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
logic had a bug where it would unconditionally never offer tickets
(but would advertise support) on renego, so any server doing renego
resumption against an OpenSSL-derived client must not support
session tickets.
This also gets rid of s->new_session which is now pointless.
BUG=429450
Change-Id: I884bdcdc80bff45935b2c429b4bbc9c16b2288f8
Reviewed-on: https://boringssl-review.googlesource.com/4732
Reviewed-by: Adam Langley <agl@google.com>
We have a lot of options that don't do anything.
Change-Id: I1681fd07d1272547d4face87917ce41029bbf0de
Reviewed-on: https://boringssl-review.googlesource.com/4731
Reviewed-by: Adam Langley <agl@google.com>
There's multiple different versions of this check, between
s->s3->have_version (only works at some points), s->new_session (really
weird and not actually right), s->renegotiate (fails on the server
because it's always 2 after ClientHello), and s->s3->tmp.finish_md_len
(super confusing). Add an explicit bit with clear meaning. We'll prune
some of the others later; notably s->renegotiate can go away when
initiating renegotiation is removed.
This also tidies up the extensions to be consistent about whether
they're allowed during renego:
- ALPN failed to condition when accepting from the server, so even
if the client didn't advertise, the server could.
- SCTs now *are* allowed during renego. I think forbidding it was a
stray copy-paste. It wasn't consistently enforced in both ClientHello
and ServerHello, so the server could still supply it. Moreover, SCTs
are part of the certificate, so we should accept it wherever we accept
certificates, otherwise that session's state becomes incomplete. This
matches OCSP stapling. (NB: Chrome will never insert a session created
on renego into the session cache and won't accept a certificate
change, so this is moot anyway.)
Change-Id: Ic9bd1ebe2a2dbe75930ed0213bf3c8ed8170e251
Reviewed-on: https://boringssl-review.googlesource.com/4730
Reviewed-by: Adam Langley <agl@google.com>
As of crbug.com/484543, Chromium's SSLClientSocket is not sensitive to whether
renegotiation is enabled or not. Disable it by default and require consumers to
opt into enabling this protocol mistake.
BUG=429450
Change-Id: I2329068284dbb851da010ff1fd398df3d663bcc3
Reviewed-on: https://boringssl-review.googlesource.com/4723
Reviewed-by: Adam Langley <agl@google.com>
This change makes it safe to call EVP_AEAD_CTX_cleanup after a failed
EVP_AEAD_CTX_init.
Change-Id: I608ed550e08d638cd7e941f5067edd3da4c850ab
Reviewed-on: https://boringssl-review.googlesource.com/4692
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
There's no real need to ever disable it, so this is one fewer configuration to
test. It's still disabled for DTLS, but a follow-up will resolve that.
Change-Id: Ia95ad8c17ae8236ada516b3968a81c684bf37fd9
Reviewed-on: https://boringssl-review.googlesource.com/4683
Reviewed-by: Adam Langley <agl@google.com>
inttypes.h kindly requires a feature macro in C++ on some platforms, due
to a bizarre footnote in C99 (see footnote 191 in section 7.8.1). As
bn.h is a public header, we must leak this wart to the consumer. On
platforms with unfriendly inttypes.h headers, using BN_DEC_FMT1 and
friends now require the feature macro be defined externally.
This broke the Chromium Android Clang builder:
http://build.chromium.org/p/chromium.linux/builders/Android%20Clang%20Builder%20%28dbg%29/builds/59288
Change-Id: I88275a6788c7babd0eae32cae86f115bfa93a591
Reviewed-on: https://boringssl-review.googlesource.com/4688
Reviewed-by: Adam Langley <agl@google.com>
Change |EC_KEY_new_by_curve_name| to report |ERR_R_MALLOC_FAILURE|
itself, so that reporting of |EC_R_UNKNOWN_GROUP| is not confused by
the caller's addition of a spurious |ERR_R_MALLOC_FAILURE|.
Change-Id: Id3f5364f01eb8e3597bcddd6484bc03d5578befb
Reviewed-on: https://boringssl-review.googlesource.com/4690
Reviewed-by: Adam Langley <agl@google.com>
Trusty doesn't have setjmp.h and nor does it have threads.
Change-Id: I005f7a009a13e6632513be9fab2bbe62294519a4
Reviewed-on: https://boringssl-review.googlesource.com/4660
Reviewed-by: Adam Langley <agl@google.com>
With DTLSv1_get_timeout de-ctrl-ified, the type checker complains about
OPENSSL_timeval. Existing callers all use the real timeval.
Now that OPENSSL_timeval is not included in any public structs, simply
forward-declare timeval itself in ssl.h and pull in winsock2.h in internal
headers.
Change-Id: Ieaf110e141578488048c28cdadb14881301a2ce1
Reviewed-on: https://boringssl-review.googlesource.com/4682
Reviewed-by: Adam Langley <agl@google.com>
Nothing ever uses those structs. This to avoid having any structs in the
public header which use struct timeval.
In doing so, move the protocol version constants up to ssl.h so dtls1.h
may be empty. This also removes TLS1_get_version and TLS1_get_client_version
as they're unused and depend on TLS1_VERSION_MAJOR. This still lets tls1.h
be included independently from ssl.h (though I don't think anyone ever includes
it...).
Change-Id: Ieac8b90cf94f7f1e742a88bb75c0ee0aa4b1414c
Reviewed-on: https://boringssl-review.googlesource.com/4681
Reviewed-by: Adam Langley <agl@google.com>
The interface for this is very similar to upstream, but the code is
quite different.
Support for “resuming” (i.e. calling |CMAC_Final| and then computing the
CMAC for an extension of the message) has been dropped. Also, calling
|CMAC_Init| with magic argument to reset it has been replaced with
|CMAC_Reset|.
Lastly, a one-shot function has been added because it can save an
allocation and that's what most callers actually appear to want to do.
Change-Id: I9345220218bdb16ebe6ca356928d7c6f055d83f6
Reviewed-on: https://boringssl-review.googlesource.com/4630
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The only place using it is export keying material which can do the
version check inline.
Change-Id: I1893966c130aa43fa97a6116d91bb8b04f80c6fb
Reviewed-on: https://boringssl-review.googlesource.com/4615
Reviewed-by: Adam Langley <agl@google.com>
It's only called for client certificates with NULL. The interaction with
extra_certs is more obvious if we handle that case externally. (We
shouldn't attach extra_certs if there is no leaf.)
Change-Id: I9dc26f32f582be8c48a4da9aae0ceee8741813dc
Reviewed-on: https://boringssl-review.googlesource.com/4613
Reviewed-by: Adam Langley <agl@google.com>
Next batch. Mostly a bunch of deprecated things. This switches
SSL_CTX_set_tmp_rsa from always failing to always succeeding. The latter
is probably a safer behavior; a consumer may defensively set a temporary
RSA key. We'll successfully "set it" and just never use the result.
Change-Id: Idd3d6bf4fc1a20bc9a26605bb9c77c9f799f993c
Reviewed-on: https://boringssl-review.googlesource.com/4566
Reviewed-by: Adam Langley <agl@google.com>
This is an API wart that makes it easy to accidentally reuse the server
DHE half for every handshake. It's much simpler to have only one mode.
This mirrors the change made to the ECDHE code; align with that logic.
Change-Id: I47cccbb354d70127ab458f99a6d390b213e4e515
Reviewed-on: https://boringssl-review.googlesource.com/4565
Reviewed-by: Adam Langley <agl@google.com>
The only difference is SSL_clear_num_renegotiations which is never
called.
Change-Id: Id661c71e89d34d834349ad1f1a296e332606e6cc
Reviewed-on: https://boringssl-review.googlesource.com/4564
Reviewed-by: Adam Langley <agl@google.com>
The API is unused and rather awkward (mixes output parameters with
return values, special-case for NULL).
Change-Id: I4396f98534bf1271e53642f255e235cf82c7615a
Reviewed-on: https://boringssl-review.googlesource.com/4560
Reviewed-by: Adam Langley <agl@google.com>
Also size them based on the limits in the quantities they control (after
checking bounds at the API boundary).
BUG=404754
Change-Id: Id56ba45465a473a1a793244904310ef747f29b63
Reviewed-on: https://boringssl-review.googlesource.com/4559
Reviewed-by: Adam Langley <agl@google.com>
Not going to bother adding the compatibility macros. If they get ifdef'd
out, all the better.
BUG=404754
Change-Id: I26414d2fb84ee1f0b15a3b96c871949fe2bb7fb1
Reviewed-on: https://boringssl-review.googlesource.com/4558
Reviewed-by: Adam Langley <agl@google.com>
This is a bitmask, so the number of bits available should be the same
across all platforms.
Change-Id: I98e8d375fc7d042aeae1270174bc8fc63fba5dfc
Reviewed-on: https://boringssl-review.googlesource.com/4556
Reviewed-by: Adam Langley <agl@google.com>
Document them while I'm here. This adds a new 'preprocessor
compatibility section' to avoid breaking #ifdefs. The CTRL values
themselves are defined to 'doesnt_exist' to catch anything calling
SSL_ctrl directly until that function can be unexported completely.
BUG=404754
Change-Id: Ia157490ea8efe0215d4079556a0c7643273e7601
Reviewed-on: https://boringssl-review.googlesource.com/4553
Reviewed-by: Adam Langley <agl@google.com>
Probably we'll want some simpler server-side API later. But, as things
stand, all consumers of these functions are #ifdef'd out and have to be
because the requisite OCSP_RESPONSE types are gone.
Change-Id: Ic82b2ab3feca14c56656da3ceb3651819e3eb377
Reviewed-on: https://boringssl-review.googlesource.com/4551
Reviewed-by: Adam Langley <agl@google.com>
It's unused, but for some old #ifdef branch in wpa_supplicant's EAP-FAST
hack, before SSL_set_session_ticket_ext_cb existed.
Change-Id: Ifc11fea2f6434354f756e04e5fc3ed5f1692025e
Reviewed-on: https://boringssl-review.googlesource.com/4550
Reviewed-by: Adam Langley <agl@google.com>
This avoids callers having to worry about |CRYPTO_add| and what the
correct lock to use it with is. (Esp since we'll probably change the way
that reference counts work in the future.)
Change-Id: I972bf0cc3be6099e0255e64a0fd50249062d1eb4
Reviewed-on: https://boringssl-review.googlesource.com/4623
Reviewed-by: Adam Langley <agl@google.com>
BoringSSL always uses uncompressed points. This function aborts if
another form is requested or does nothing if uncompressed points are
requested.
Change-Id: I80bc01444cdf9c789c9c75312b5527bf4957361b
I tried so hard to get rid of AES-192, but it's called from too many
places. I suspect that those places don't actually use it, but it's
dangerous to assume that.
Change-Id: I6208b64a463e3539973532abd21882e0e4c55a1c
“ECDHE-PSK-WITH-AES-128-GCM-SHA256” doesn't follow the standard naming
for OpenSSL: it was “-WITH-” in it and has a hyphen between “AES” and
“128”. This change fixes that.
Change-Id: I7465b1ec83e7d5b9a60d8ca589808aeee10c174e
Reviewed-on: https://boringssl-review.googlesource.com/4601
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Android uses BIO reference counting.
This reverts commit 9bde6aeb76.
Change-Id: Ibf4a7f42477549d10829a424ea3b52f09098666c
Reviewed-on: https://boringssl-review.googlesource.com/4472
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
These functions were #if 0'ed out in the code, which is a distraction.
Change-Id: I186196ab512565507476f9b56682bf59d003d85f
Reviewed-on: https://boringssl-review.googlesource.com/4604
Reviewed-by: Adam Langley <agl@google.com>
These are never used and no flags are defined anyway.
Change-Id: I206dc2838c5f68d87559a702dcb299b208cc7e1e
Reviewed-on: https://boringssl-review.googlesource.com/4493
Reviewed-by: Adam Langley <agl@google.com>
This is a really dumb API wart. Now that we have a limited set of curves that
are all reasonable, the automatic logic should just always kick in. This makes
set_ecdh_auto a no-op and, instead of making it the first choice, uses it as
the fallback behavior should none of the older curve selection APIs be used.
Currently, by default, server sockets can only use the plain RSA key exchange.
BUG=481139
Change-Id: Iaabc82de766cd00968844a71aaac29bd59841cd4
Reviewed-on: https://boringssl-review.googlesource.com/4531
Reviewed-by: Adam Langley <agl@google.com>
054e682675 removed the compatibility include of
mem.h in crypto.h. mem.h doesn't exist in upstream which defines these
functions in crypto.h instead. The compatibility include should probably be
restored to avoid causing all kinds of grief when porting consumers over.
Change-Id: Idfe0f9b43ebee5df22bebfe0ed6dc85ec98b4de0
Reviewed-on: https://boringssl-review.googlesource.com/4530
Reviewed-by: Adam Langley <agl@google.com>
CRYPTO_MUTEX was the wrong size. Fortunately, Apple was kind enough to define
pthread_rwlock_t unconditionally, so we can be spared fighting with feature
macros. Some of the stdlib.h removals were wrong and clang is pick about
multiply-defined typedefs. Apparently that's a C11 thing?
BUG=478598
Change-Id: Ibdcb8de9e5d83ca28e4c55b2979177d1ef0f9721
Reviewed-on: https://boringssl-review.googlesource.com/4404
Reviewed-by: Adam Langley <agl@google.com>
This is taken from upstream, although it originally came from us. This
will only take effect on 64-bit systems (x86-64 and aarch64).
Before:
Did 1496 ECDH P-256 operations in 1038743us (1440.2 ops/sec)
Did 2783 ECDSA P-256 signing operations in 1081006us (2574.5 ops/sec)
Did 2400 ECDSA P-256 verify operations in 1059508us (2265.2 ops/sec)
After:
Did 4147 ECDH P-256 operations in 1061723us (3905.9 ops/sec)
Did 9372 ECDSA P-256 signing operations in 1040589us (9006.4 ops/sec)
Did 4114 ECDSA P-256 verify operations in 1063478us (3868.4 ops/sec)
Change-Id: I11fabb03239cc3a7c4a97325ed4e4c97421f91a9
We don't support the SSL BIO so this is a no-op change.
Change-Id: Iba9522b837ebb0eb6adc80d5df6dcac99abf2552
Reviewed-on: https://boringssl-review.googlesource.com/4360
Reviewed-by: Adam Langley <agl@google.com>
Instead, each module defines a static CRYPTO_EX_DATA_CLASS to hold the values.
This makes CRYPTO_cleanup_all_ex_data a no-op as spreading the
CRYPTO_EX_DATA_CLASSes across modules (and across crypto and ssl) makes cleanup
slightly trickier. We can make it do something if needbe, but it's probably not
worth the trouble.
Change-Id: Ib6f6fd39a51d8ba88649f0fa29c66db540610c76
Reviewed-on: https://boringssl-review.googlesource.com/4375
Reviewed-by: Adam Langley <agl@google.com>
No functions for using it were ever added.
Change-Id: Iaee6e5bc8254a740435ccdcdbd715b851d8a0dce
Reviewed-on: https://boringssl-review.googlesource.com/4374
Reviewed-by: Adam Langley <agl@google.com>
No wrappers were ever added and codesearch confirms no one ever added to it
manually. Probably anyone doing complex things with BIOs just made a custom
BIO_METHOD. We can put it back with proper functions if the need ever arises.
Change-Id: Icb5da7ceeb8f1da6d08f4a8854d53dfa75827d9c
Reviewed-on: https://boringssl-review.googlesource.com/4373
Reviewed-by: Adam Langley <agl@google.com>
Callers are required to use the wrappers now. They still need OPENSSL_EXPORT
since crypto and ssl get built separately in the standalone shared library
build.
Change-Id: I61186964e6099b9b589c4cd45b8314dcb2210c89
Reviewed-on: https://boringssl-review.googlesource.com/4372
Reviewed-by: Adam Langley <agl@google.com>
It's unused and requires ex_data support a class number per type.
Change-Id: Ie1fb55053631ef00c3318f3253f7c9501988f522
Reviewed-on: https://boringssl-review.googlesource.com/4371
Reviewed-by: Adam Langley <agl@google.com>
This is never used and we can make the built-in one performant.
Change-Id: I6fc7639ba852349933789e73762bc3fa1341b2ff
Reviewed-on: https://boringssl-review.googlesource.com/4370
Reviewed-by: Adam Langley <agl@google.com>
OpenSSH, especially, does some terrible things that mean that it needs
the EVP_CIPHER structure to be exposed ☹. Damian is open to a better API
to replace this, but only if OpenSSL agree too. Either way, it won't be
happening soon.
Change-Id: I393b7a6af6694d4d2fe9ebcccd40286eff4029bd
Reviewed-on: https://boringssl-review.googlesource.com/4330
Reviewed-by: Adam Langley <agl@google.com>
This introduces a per-RSA/DSA/DH lock. This is good for lock contention,
although pthread locks are depressingly bloated.
Change-Id: I07c4d1606fc35135fc141ebe6ba904a28c8f8a0c
Reviewed-on: https://boringssl-review.googlesource.com/4324
Reviewed-by: Adam Langley <agl@google.com>
Prior to this, BoringSSL was using OpenSSL's technique of having users
register a callback for locking operation. This change adds native mutex
support.
Since mutexes often need to be in objects that are exposed via public
headers, the non-static mutexes are defined in thread.h. However, on
Windows we don't want to #include windows.h for CRITICAL_SECTION and, on
Linux, pthread.h doesn't define pthread_rwlock_t unless the feature
flags are set correctly—something that we can't control in general
for public header files. Thus, on both platforms, the mutex is defined
as a uint8_t[] of equal or greater size and we depend on static asserts
to ensure that everything works out ok.
Change-Id: Iafec17ae7e3422325e587878a5384107ec6647ab
Reviewed-on: https://boringssl-review.googlesource.com/4321
Reviewed-by: Adam Langley <agl@google.com>
It appears that this reference “count” is set to one at creation and
never touched after that.
Change-Id: I3238a6d3dd702953771b8ec725c1c5712c648fba
Reviewed-on: https://boringssl-review.googlesource.com/4320
Reviewed-by: Adam Langley <agl@google.com>
This causes any unexpected handshake records to be met with a fatal
no_renegotiation alert.
In addition, restore the redundant version sanity-checks in the handshake state
machines. Some code would zero the version field as a hacky way to break the
handshake on renego. Those will be removed when switching to this API.
The spec allows for a non-fatal no_renegotiation alert, but ssl3_read_bytes
makes it difficult to find the end of a ClientHello and skip it entirely. Given
that OpenSSL goes out of its way to map non-fatal no_renegotiation alerts to
fatal ones, this seems probably fine. This avoids needing to account for
another source of the library consuming an unbounded number of bytes without
returning data up.
Change-Id: Ie5050d9c9350c29cfe32d03a3c991bdc1da9e0e4
Reviewed-on: https://boringssl-review.googlesource.com/4300
Reviewed-by: Adam Langley <agl@google.com>
There's this giant "Underdocumented functions" section in the middle, but it
doesn't look too silly once the "Deprecated methods" section is merged in with
the other deprecated functions.
Change-Id: Ib97d88b0f915f60e9790264474a9e4aa3e115382
Reviewed-on: https://boringssl-review.googlesource.com/4291
Reviewed-by: Adam Langley <agl@google.com>
Just about everything depends on SSL_CIPHER. Move it to the top as the first
section in ssl.h. Match the header order and the source file order and document
everything. Also make a couple of minor style guide tweaks.
Change-Id: I6a810dbe79238278ac480e5ced1447055715a79f
Reviewed-on: https://boringssl-review.googlesource.com/4290
Reviewed-by: Adam Langley <agl@google.com>
Mostly stuff that doc.go was grumpy about. The main change is to move the
version-specific headers to the bottom. Injecting them in the middle makes it
seem as if the definitions above the #include and those below are somehow
different, but it compiles fine with them at the bottom. (They have to be at
the bottom because those headers depend on ssl.h.)
Change-Id: Iaa4139d2f157c7a3fd0ea609b78ff11d2edfc7b0
Reviewed-on: https://boringssl-review.googlesource.com/4289
Reviewed-by: Adam Langley <agl@google.com>
It's no longer needed to distinguish ciphers from fake ciphers.
Change-Id: I1ad4990ba936b1059eb48f3d2f309eb832dd1cb5
Reviewed-on: https://boringssl-review.googlesource.com/4285
Reviewed-by: Adam Langley <agl@google.com>
Avoiding superflous references to MD5 makes it easier to audit the code
to find unsafe uses of it. It also avoids subtly encouraging users to
choose MD5 instead of a better alternative.
Change-Id: Ic78eb5dfbf44aac39e4e4eb29050e3337c4445cc
Reviewed-on: https://boringssl-review.googlesource.com/3926
Reviewed-by: Adam Langley <agl@google.com>
Beyond generally eliminating unnecessary includes, eliminate as many
includes of headers that declare/define particularly error-prone
functionality like strlen, malloc, and free. crypto/err/internal.h was
added to remove the dependency on openssl/thread.h from the public
openssl/err.h header. The include of <stdlib.h> in openssl/mem.h was
retained since it defines OPENSSL_malloc and friends as macros around
the stdlib.h functions. The public x509.h, x509v3.h, and ssl.h headers
were not changed in order to minimize breakage of source compatibility
with external code.
Change-Id: I0d264b73ad0a720587774430b2ab8f8275960329
Reviewed-on: https://boringssl-review.googlesource.com/4220
Reviewed-by: Adam Langley <agl@google.com>
The only dependency the low-level crypto modules have on code in
crypto/obj is their use of OBJ_nid2sn, which is trivial to avoid.
This facilitates future simplification of crypto/obj, including
possibly the removal of functions like OBJ_nid2sn and the complex
build infrastructure that supports them.
This change also removes EVP_CIPHER_name and EVP_MD_name.
Change-Id: I34ce7dc7e58d5c08b52f95d25eba3963590cf2f7
Reviewed-on: https://boringssl-review.googlesource.com/3932
Reviewed-by: Adam Langley <agl@google.com>
A previous change in BoringSSL renamed ERR_print_errors_fp to
BIO_print_errors_fp as part of refactoring the code to improve the
layering of modules within BoringSSL. Rename it back for better
compatibility with code that was using the function under the original
name. Move its definition back to crypto/err using an implementation
that avoids depending on crypto/bio.
Change-Id: Iee7703bb1eb4a3d640aff6485712bea71d7c1052
Reviewed-on: https://boringssl-review.googlesource.com/4310
Reviewed-by: Adam Langley <agl@google.com>
Avoiding superflous references to RC4 makes it easier to audit the code
to find unsafe uses of it. It also avoids subtly encouraging users to
choose RC4 instead of a better alternative.
Change-Id: Ia27d7f4cd465e143d30a28b36c7871f7c30411ea
Reviewed-on: https://boringssl-review.googlesource.com/3990
Reviewed-by: Adam Langley <agl@google.com>
These are all masks of some sort (except id which is a combined version and
cipher), so they should use fixed-size unsigned integers.
Change-Id: I058dd8ad231ee747df4b4fb17d9c1e2cbee21918
Reviewed-on: https://boringssl-review.googlesource.com/4283
Reviewed-by: Adam Langley <agl@google.com>
We shouldn't be wrapping system headers.
Change-Id: I77498f4ec869797050b276eb764d892f73782f9f
Reviewed-on: https://boringssl-review.googlesource.com/4282
Reviewed-by: Adam Langley <agl@google.com>
Match the other internal headers.
Change-Id: Iff7e2dd06a1a7bf993053d0464cc15638ace3aaa
Reviewed-on: https://boringssl-review.googlesource.com/4280
Reviewed-by: Adam Langley <agl@google.com>
These are the remaining untested cipher suites. Rather than add support in
runner.go, just remove them altogether. Grepping for this is a little tricky,
but nothing enables aNULL (all occurrences disable it), and all occurrences of
["ALL:] seem to be either unused or explicitly disable anonymous ciphers.
Change-Id: I4fd4b8dc6a273d6c04a26e93839641ddf738343f
Reviewed-on: https://boringssl-review.googlesource.com/4258
Reviewed-by: Adam Langley <agl@google.com>
Now that ERR is using thread-local storage, there's very little that the
THREADID code is doing and it can be turned into stub functions.
Change-Id: I668613fec39b26c894d029b10a8173c3055f6019
Since ERR will soon have thread-local storage, we don't need to worry
about high-performance implementations and thus don't need to be able to
switch two different implementations at run-time.
Change-Id: I0598054ee8a8b499ac686ea635a96f5d03c754e0
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>
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>
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>
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>
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>
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>
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>
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>
MIPS64 confusingly sets __mips__, but it's not a 32-bit platform. This
change updates the defines in base.h to recognise MIPS64 based on both
__mips__ and __LP64__ being defined.
Change-Id: I220f5d9c8f1cd7d3089cc013348e6f95cdee76d9
Reviewed-on: https://boringssl-review.googlesource.com/4093
Reviewed-by: Adam Langley <agl@google.com>
(system/keymaster is using them now.)
Change-Id: I8fba501005b9318b7d3a76bf1715fb772b23c49d
Reviewed-on: https://boringssl-review.googlesource.com/4092
Reviewed-by: Adam Langley <agl@google.com>
It's not actually CRYPTO_add_locked, despite the name. I guess they just needed
a name that didn't clash with CRYPTO_add.
Change-Id: I3fdee08bf75e9a4e1b5e75630707c0be5792599b
Reviewed-on: https://boringssl-review.googlesource.com/4102
Reviewed-by: Adam Langley <agl@google.com>
Within the library, only ssl_update_cache read them, so add a dedicated field
to replace that use.
The APIs have a handful of uninteresting callers so I've left them in for now,
but they now always return zero.
Change-Id: Ie4e36fd4ab18f9bff544541d042bf3c098a46933
Reviewed-on: https://boringssl-review.googlesource.com/4101
Reviewed-by: Adam Langley <agl@google.com>
Align with upstream's renames from a while ago. These names are considerably
more standard. This also aligns with upstream in that both "ECDHE" and "EECDH"
are now accepted in the various cipher string parsing bits.
Change-Id: I84c3daeacf806f79f12bc661c314941828656b04
Reviewed-on: https://boringssl-review.googlesource.com/4053
Reviewed-by: Adam Langley <agl@google.com>
(Thanks to William Hesse.)
Change-Id: I8479663250546a5ec0a024f80e50541f91d833bc
Reviewed-on: https://boringssl-review.googlesource.com/4020
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
They do not quite measure the same value for EC keys. "size" is a really weird
notion to generalize on so we should document what it means for each key type.
EVP_PKEY_size's meaning is most tied to signatures, thanks to EVP_SignFinal
implicitly using it as output bounds.
Change-Id: I7504c142818f8f90f8bcf6891c97a6adaf2d574e
Reviewed-on: https://boringssl-review.googlesource.com/4000
Reviewed-by: Adam Langley <agl@google.com>
C99 doesn't, technically, allow empty statements. Thus if a #define'ed
function ends in a semicolon, and the use of it also ends in a
semicolon, then the compiler sees “;;” at the end.
Since a choice has to be made, I prefer that the semicolon exist at the
“callsite” of a #define'ed fuction. But I haven't gone and changed
everything to follow that in this patch.
Change-Id: I1343e52a5ac6255db49aa053048d0df3225bcf43
Reviewed-on: https://boringssl-review.googlesource.com/3890
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
At least the linker can discard this function in the cases where nobody
is calling it.
Change-Id: I30050e918e6bc1dd9c97cc70f3a56408701abebc
Reviewed-on: https://boringssl-review.googlesource.com/3724
Reviewed-by: Adam Langley <agl@google.com>
Because NTLM authentication is still a thing.
Change-Id: I3308a8431c82f0b614e09ce3e5efac1526881f1e
Reviewed-on: https://boringssl-review.googlesource.com/3723
Reviewed-by: Adam Langley <agl@google.com>
This allows the current RC4 state of an SSL* to be extracted. We have
internal uses for this functionality.
Change-Id: Ic124c4b253c8325751f49e7a4c021768620ea4b7
Reviewed-on: https://boringssl-review.googlesource.com/3722
Reviewed-by: Adam Langley <agl@google.com>
This callback receives information about the ClientHello and can decide
whether or not to allow the handshake to continue.
Change-Id: I21be28335fa74fedb5b73a310ee24310670fc923
Reviewed-on: https://boringssl-review.googlesource.com/3721
Reviewed-by: Adam Langley <agl@google.com>
* Eliminate the possibility of multiple lock IDs having the same
value (CRYPTO_LOCK_FIPS2 and CRYPTO_LOCK_OBJ were both 40 prior to
this commit).
* Remove unused lock IDs.
* Automatically guarantee that lock IDs and lock names stay in sync.
Change-Id: If20e462db1285fa891595a7e52404ad011ff16f6
Reviewed-on: https://boringssl-review.googlesource.com/3923
Reviewed-by: Adam Langley <agl@google.com>
No code within BoringSSL or Google (grep for EVP_PKEY_CTX_(ctrl|get|set)) is
sensitive to the various failure cases. Normalize it all to 0/1 for simplicity.
This does carry a slight risk: any new ctrl hooks we import from upstream that,
like EVP_PKEY_CTX_get_rsa_oaep_md, return something other than success/failure
cannot be called directly via EVP_PKEY_CTX_ctrl. They instead need to
internally be routed through a struct like CBS and only called through the
wrappers. To that end, unexport EVP_PKEY_CTX_ctrl and require that callers use
the wrappers. No code in Google uses it directly and, if need be, switching to
the wrapper would be an incredibly upstreamable patch.
Change-Id: I3fd4e5a1a0f3d4d1c4122c52d4c74a5105b99cd5
Reviewed-on: https://boringssl-review.googlesource.com/3874
Reviewed-by: Adam Langley <agl@google.com>
This is the only EVP_PKEY ctrl hook which returns something other than a
boolean.
Change-Id: Ic226aef168abdf72e5d30e8264a559ed5039a055
Reviewed-on: https://boringssl-review.googlesource.com/3873
Reviewed-by: Adam Langley <agl@google.com>
This removes another place where we're internally sensitive to the
success/failure conditions.
Change-Id: I18fecf6457e841ba0afb718397b9b5fd3bbdfe4c
Reviewed-on: https://boringssl-review.googlesource.com/3872
Reviewed-by: Adam Langley <agl@google.com>
All EVP_PKEY types return 1 on that. (It can go away entirely when
EVP_PKEY_HMAC is gone.) This removes a place internally where we're sensitive
to the failure code.
Change-Id: Ic6cda2da9337ba7ef1c66a18e40c5dcc44fcf840
Reviewed-on: https://boringssl-review.googlesource.com/3871
Reviewed-by: Adam Langley <agl@google.com>
Turn them into static functions that take in an hm_fragment. It's not
immediately obvious that the frag_off/frag_len bounds checks and the msg_len
consistency check are critical to avoiding an out-of-bounds write. Better to
have dtls1_hm_fragment_mark also check internally.
Also rework the bitmask logic to be clearer and avoid a table.
Change-Id: Ica54e98f66295efb323e033cb6c67ab21e7d6cbc
Reviewed-on: https://boringssl-review.googlesource.com/3765
Reviewed-by: Adam Langley <agl@google.com>
Replace unsigned long with the appropriate sized integer type.
Change-Id: I7b4641d84568f6c11efa25350a9e488a556fc92e
Reviewed-on: https://boringssl-review.googlesource.com/3766
Reviewed-by: Adam Langley <agl@google.com>
Notably, drop all special cases around receiving a message in order and
receiving a full message. It makes things more complicated and was the source
of bugs (the MixCompleteMessageWithFragments tests added in this CL did not
pass before). Instead, every message goes through an hm_fragment, and
dtls1_get_message always checks buffered_messages to see if the next is
complete.
The downside is that we pay one more copy of the message data in the common
case. This is only during connection setup, so I think it's worth the
simplicity. (If we want to optimize later, we could either tighten
ssl3_get_message's interface to allow the handshake data being in the
hm_fragment's backing store rather than s->init_buf or swap out s->init_buf
with the hm_fragment's backing store when a mesasge completes.
This CL does not address ssl_read_bytes being an inappropriate API for DTLS.
Future work will revise the handshake/transport boundary to align better with
DTLS's needs. Also other problems that I've left as TODOs.
Change-Id: Ib4570d45634b5181ecf192894d735e8699b1c86b
Reviewed-on: https://boringssl-review.googlesource.com/3764
Reviewed-by: Adam Langley <agl@google.com>
It happens to give the same value anyway (64 + 16), but only on accident.
Change-Id: I1415f4015e3de472dbeb9ada0d92607c9d1bcd40
Reviewed-on: https://boringssl-review.googlesource.com/3780
Reviewed-by: Adam Langley <agl@google.com>
This transcription bug comes from the start of BoringSSL and, as you can
imagine, was a complete delight to track down.
Change-Id: I3051934195098a1d3bf893b154389ec7f14d3609
Reviewed-on: https://boringssl-review.googlesource.com/3740
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Instead, add a separate init_with_direction hook. Normal AEADs ignore the
direction, while legacy AEADs must be initialized with it. This avoids
maintaining extra state to support the delayed initialization.
Change-Id: I25271f0e56ee2783a2fd4d4026434154d58dc0a8
Reviewed-on: https://boringssl-review.googlesource.com/3731
Reviewed-by: Adam Langley <agl@google.com>
There's no good reason to do this, and it doesn't work; HMAC checks the length
of the key and runs it through the hash function if too long. The reuse occurs
after this check.
This allows us to shave 132 bytes off HMAC_CTX as this was the only reason it
ever stored the original key. It also slightly simplifies HMAC_Init_ex's
logic.
Change-Id: Ib56aabc3630b7178f1ee7c38ef6370c9638efbab
Reviewed-on: https://boringssl-review.googlesource.com/3733
Reviewed-by: Adam Langley <agl@google.com>
(There are times when I actually miss C++ templates.)
Change-Id: I3db56e4946ae4fb919105fa33e2cfce3c7542d37
Reviewed-on: https://boringssl-review.googlesource.com/3700
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I7b6acc9004beb7b7090de1837814ccdff2e9930e
Reviewed-on: https://boringssl-review.googlesource.com/3680
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
EC_GROUP_copy is an rather unfriendly function; it doesn't work if the groups
have different[*] underlying EC_METHODs, but this notion is not exposed through
the API. I found no callers of EC_GROUP_copy in external code.
This leaves the precompute_mult functions as the remaining mutable API exposed
through EC_GROUP.
[*] Though, of the two EC_METHODs right now, simple.c is entirely unused.
Change-Id: Iabb52518005250fb970e12b3b0ea78b4f6eff4a0
Reviewed-on: https://boringssl-review.googlesource.com/3631
Reviewed-by: Adam Langley <agl@google.com>
Test both asynchronous and synchronous versions. This callback is somewhat
different from others. It's NOT called a second time when the handshake is
resumed. This appears to be intentional and not a mismerge from the internal
patch. The caller is expected to set up any state before resuming the handshake
state machine.
Also test the early callback returning an error.
Change-Id: If5e6eddd7007ea5cdd7533b4238e456106b95cbd
Reviewed-on: https://boringssl-review.googlesource.com/3590
Reviewed-by: Adam Langley <agl@google.com>
(I got this wrong when reading the OpenSSL code.)
Change-Id: Ib289ef41d0ab5a3157ad8b9454d2de96d1f86c22
Reviewed-on: https://boringssl-review.googlesource.com/3620
Reviewed-by: Adam Langley <agl@google.com>
There is exactly one implementation and it doesn't fail. Plus a cleanup
function that can fail is very bad manners; the caller has no choice but to
leak at that point.
Change-Id: I5b524617ef37bc7d92273472fa742416ea7dfd43
Reviewed-on: https://boringssl-review.googlesource.com/3564
Reviewed-by: Adam Langley <agl@google.com>
RC4_CHAR is a bit in the x86(-64) CPUID information that switches the
RC4 asm code from using an array of 256 uint32_t's to 256 uint8_t's. It
was originally written for the P4, where the uint8_t style was faster.
(On modern chips, setting RC4_CHAR took RC4-MD5 from 458 to 304 MB/s.
Although I wonder whether, on a server with many connections, using less
cache wouldn't be better.)
However, I'm not too worried about a slowdown of RC4 on P4 systems these
days (the last new P4 chip was released nine years ago) and I want the
code to be simplier.
Also, RC4_CHAR was set when the CPUID family was 15, but Intel actually
lists 15 as a special code meaning "also check the extended family
bits", which the asm didn't do.
The RC4_CHAR support remains in the RC4 asm code to avoid drift with
upstream.
Change-Id: If3febc925a83a76f453b9e9f8de5ee43759927c6
Reviewed-on: https://boringssl-review.googlesource.com/3550
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(Which is just an exported wrapper around ssl3_get_cipher_by_value.)
Change-Id: Ibba166015ce59e337ff50963ba20237ac4949aaf
Reviewed-on: https://boringssl-review.googlesource.com/3543
Reviewed-by: Adam Langley <agl@google.com>
Upstream settled in this API, and it's also the one that we expect
internally and that third_party code will expect.
Change-Id: Id7af68cf0af1f2e4d9defd37bda2218d70e2aa7b
Reviewed-on: https://boringssl-review.googlesource.com/3542
Reviewed-by: Adam Langley <agl@google.com>
It was a mistake to remove this in the first place.
Change-Id: Icd97b4db01e49151daa41dd892f9da573ddc2842
Reviewed-on: https://boringssl-review.googlesource.com/3541
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This empty header file exists only to make older code compile. But I
named it incorrectly! Upstream doesn't have the underscore in the name.
Change-Id: I96654b7e17d84a5f2810e6eb20fe7bfb22f855fd
Reviewed-on: https://boringssl-review.googlesource.com/3540
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It's never called in outside code. This too seems to be a remnant of the DSA
PKIX optional parameter stuff. This is confirmed both by a removed comment and
by the brief documentation at http://www.umich.edu/~x509/ssleay/x509_pkey.html
RFC 5480 does not allow ECDSA keys to be missing parameters, so this logic is
incorrect for ECDSA anyway. It was also failing to check
EVP_PKEY_copy_parameters' return value. And that logic looks pretty suspect if
you have a chain made up multiple certificate types.
Change-Id: Id6c60659a0162356c7f3eae5c797047366baae1c
Reviewed-on: https://boringssl-review.googlesource.com/3485
Reviewed-by: Adam Langley <agl@google.com>
Found while diagnosing some crashes and hangs in the malloc tests. This (and
the follow-up) get us further but does not quite let the malloc tests pass
quietly, even without valgrind. DTLS silently ignores some malloc failures
(confusion with silently dropping bad packets) which then translate to hangs.
Change-Id: Ief06a671e0973d09d2883432b89a86259e346653
Reviewed-on: https://boringssl-review.googlesource.com/3482
Reviewed-by: Adam Langley <agl@google.com>
False Start is the name it's known by now. Deprecate the old API and expose new
ones with the new name.
Change-Id: I32d307027e178fd7d9c0069686cc046f75fdbf6f
Reviewed-on: https://boringssl-review.googlesource.com/3481
Reviewed-by: Adam Langley <agl@google.com>
I found no users of this. We can restore it if needbe, but I don't expect
anyone to find it useful in its current form. The API is suspect for the same
reasons DTLSv1_listen was. An SSL object is stateful and assumes you already
have the endpoint separated out.
If we ever need it, server-side HelloVerifyRequest and DTLSv1_listen should be
implemented by a separate stateless listener that statelessly handles
cookieless ClientHello + HelloVerifyRequest. Once a ClientHello with a valid
cookie comes in, it sets up a stateful SSL object and passes control along to
that.
Change-Id: I86adc1dfb6a81bebe987784c36ad6634a9a1b120
Reviewed-on: https://boringssl-review.googlesource.com/3480
Reviewed-by: Adam Langley <agl@google.com>
Reduces number of silly casts in OpenSSL code and likely most
applications. Consistent with (char *) for "peername" value from
X509_check_host() and X509_VERIFY_PARAM_get0_peername().
(Imported from upstream's e83c913723fac7432a7706812f12394aaa00e8ce.)
Change-Id: Id0fc11773a0cee8933978cd4bdbd8251fd7cfb5f
Pass address of X509_VERIFY_PARAM_ID peername to X509_check_host().
(Imported from upstream's 55fe56837a65ff505b492aa6aee748bf5fa91fec.)
Change-Id: Ic21bfb361b8eb25677c4c2175882fa95ea44fc31
(Imported from upstream's 8abffa4a73fcbf6536e0a42d736ed9211a8204ea,
9624b50d51de25bb2e3a72e81fe45032d80ea5c2 and
41e3ebd5abacfdf98461cdeb6fa97a4175b7aad3.)
Change-Id: Ic9099eb5704b19b4500229e89351371cc6184f9d
clang-format has changed a little. This is a semantic no-op but it makes
the diff in the next change smaller.
Change-Id: Ia492a81340a868b888d619a1c7740d1a86845e92
This saves about 6-7k of error data.
Change-Id: Ic28593d4a1f5454f00fb2399d281c351ee57fb14
Reviewed-on: https://boringssl-review.googlesource.com/3385
Reviewed-by: Adam Langley <agl@google.com>
Some files in crypto/x509 were moved from crypto/asn1, so they emit errors from
another module. Fix make_errors.go to account for this: cross module errors
must use the foreign module as the first argument to OPENSSL_PUT_ERROR. Both
the function code and the error code should be declared in the foreign module.
Update make_errors.go to ignore cross-module error lines when deciding which
function tokens to emit.
Change-Id: Ic38377ddd56e22d033ef91318c30510762f6445d
Reviewed-on: https://boringssl-review.googlesource.com/3383
Reviewed-by: Adam Langley <agl@google.com>
Also, Clang doesn't like static asserts with the same message and
ERR_free_strings should still free the error queues, although it's badly
misnamed now.
Change-Id: Ibff8eb50f93c0b56c3eeb17a300e8501a31c3ab8
Reviewed-on: https://boringssl-review.googlesource.com/3370
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Previously, error strings were kept in arrays for each subdirectory and
err.c would iterate over them all and insert them at init time to a hash
table.
This means that, even if you have a shared library and lots of processes
using that, each process has ~30KB of private memory from building that
hash table.
This this change, all the error strings are built into a sorted list and
are thus static data. This means that processes can share the error
information and it actually saves binary space because of all the
pointer overhead in the old scheme. Also it saves the time taken
building the hash table at startup.
This removes support for externally-supplied error string data.
Change-Id: Ifca04f335c673a048e1a3e76ff2b69c7264635be
OpenSSL's internal names for the ciphers are not the standard ones and are not
easy to consistently map to the standard ones. Add an API to get the real names
out. (WebRTC wants an API to get the standard names out.)
Also change some incorrect flags on SHA-256 TLS 1.2 ciphers;
SSL_HANDSHAKE_MAC_DEFAULT and SSL_HANDSHAKE_MAC_SHA256 are the same after TLS
1.2. A TLS 1.2 cipher should be tagged explicitly with SHA-256. (This avoids
tripping a check in SSL_CIPHER_get_rfc_name which asserts that default-hash
ciphers only ever use SHA-1 or MD5 for the bulk cipher MAC.)
Change-Id: Iaec2fd4aa97df29883094d3c2ae60f0ba003bf07
The fact that an SSL_SESSION is reference-counted is already part of the API.
If an external application (like, say, the test code) wishes to participate, we
should let it.
Change-Id: If04d26a35141da14fd8d917de6cc1c10537ad11a
Reviewed-on: https://boringssl-review.googlesource.com/3344
Reviewed-by: Adam Langley <agl@google.com>
This makes the following changes:
- SSL_cutthrough_complete no longer rederives whether cutthrough happened and
just maintains a handshake bit.
- SSL_in_init no longer returns true if we are False Starting but haven't
completed the handshake. That logic was awkward as it depended on querying
in_read_app_data to force SSL_read to flush the entire handshake. Defaulting
SSL_in_init to continue querying the full handshake and special-casing
SSL_write is better. E.g. the check in bidirectional SSL_shutdown wants to know
if we're in a handshake. No internal consumer of
SSL_MODE_HANDSHAKE_CUTTHROUGH ever queries SSL_in_init directly.
- in_read_app_data is gone now that the final use is dead.
Change-Id: I05211a116d684054dfef53075cd277b1b30623b5
Reviewed-on: https://boringssl-review.googlesource.com/3336
Reviewed-by: Adam Langley <agl@google.com>
The ENGINE code had a concept of a stable-ABI for METHODs, because that
might be a useful thing in the future when people want to have blobs
that wrap PKCS#11 or something.
However, at the moment nobody uses this feature and it didn't work very
well anyway: I hadn't updated |ENGINE_free| to free them all and
|set_method| was copying the methods, but not resetting the |is_static|
flag.
This change removes support for non-static methods. We can always put it
back later if we need.
Change-Id: Ic7401c8cb1cadd46b26a215f85bc48562efe9919
Reviewed-on: https://boringssl-review.googlesource.com/3300
Reviewed-by: Adam Langley <agl@google.com>
SSL_AEAD_CTX ownership is currently too confusing. Instead, rely on the lack of
renego, so the previous epoch always uses the NULL cipher. (Were we to support
DTLS renego, we could keep track of s->d1->last_aead_write_ctx like
s->d1->last_write_sequence, but it isn't worth it.)
Buffered messages also tracked an old s->session, but this is unnecessary. The
s->session NULL check in tls1_enc dates to the OpenSSL initial commit and is
redundant with the aead NULL check.
Change-Id: I9a510468d95934c65bca4979094551c7536980ae
Reviewed-on: https://boringssl-review.googlesource.com/3234
Reviewed-by: Adam Langley <agl@google.com>
Nothing recognized through those codepaths is fragmentable in DTLS. Also remove
an unnecessary epoch check. It's not possible to process a record from the
wrong epoch.
Change-Id: I9d0f592860bb096563e2bdcd2c8e50a0d2b65f59
Reviewed-on: https://boringssl-review.googlesource.com/3232
Reviewed-by: Adam Langley <agl@google.com>
We will not support any form of DTLS renego.
Change-Id: I6eab4ed12a131ad27fdb9b5ea7cc1f35d872cd43
Reviewed-on: https://boringssl-review.googlesource.com/3230
Reviewed-by: Adam Langley <agl@google.com>
It has no callers in internal code.
Change-Id: I53cf1769b71be6a0441533b6af7d3f64aab5098a
Reviewed-on: https://boringssl-review.googlesource.com/3219
Reviewed-by: Adam Langley <agl@google.com>
Tag number 31 is a long form tag that requires multiple octets. It
cannot be handled by adding a single uint8. Changed CBB_add_asn1()
to return 0 when it is passed in the extension for tag 31.
Change-Id: Ia33936d4f174d1a7176eb11da0b5c7370efb9416
CBS_get_asn1() and CBS_get_any_asn1_element() only support the single
byte ASN.1 identifier octets (aka short form tags). Tag number 31 is
the start of the multi-byte long form per X.690 section 8.1.2.4.
Change-Id: I892600f4946e880a4ff03d219181116ef04f5372
Reviewed-on: https://boringssl-review.googlesource.com/3241
Reviewed-by: Adam Langley <agl@google.com>
All but one field is a no-op.
Change-Id: Ib7bc59a12ce792d5e42fb6e04a4aff54f42643a9
Reviewed-on: https://boringssl-review.googlesource.com/3213
Reviewed-by: Adam Langley <agl@google.com>
This is so the tests needn't be sensitive to the clock. It is, unfortunately, a
test-only hook, but the DTLS retransmit/timeout logic more-or-less requires it
currently. Use this hook to, for now, freeze the clock at zero. This makes the
tests deterministic.
It might be worth designing a saner API in the future. The current one,
notably, requires that the caller's clock be compatible with the one we
internally use. It's also not clear whether the caller needs to call
DTLSv1_handle_timeout or can just rely on the state machine doing it internally
(as it does do). But mock clocks are relatively tame and WebRTC wants to
compile against upstream OpenSSL for now, so we're limited in how much new API
we can build.
Change-Id: I7aad51570596f69275ed0fc1a8892393e4b7ba13
Reviewed-on: https://boringssl-review.googlesource.com/3210
Reviewed-by: Adam Langley <agl@google.com>
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.
This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.
Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Forgot to export those when adding them.
Change-Id: I206f488eb38e5ff55b8c212911aced0cf28b7664
Reviewed-on: https://boringssl-review.googlesource.com/3090
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 2747d73c1466c487daf64a1234b6fe2e8a62ac75.)
Also fix up some stylistic issues in conf.c and clarify empty case in
documentation.
Change-Id: Ibacabfab2339d7566d51db4b3ac4579aec0d1fbf
Reviewed-on: https://boringssl-review.googlesource.com/3023
Reviewed-by: Adam Langley <agl@google.com>
Since we can't update wpa_supplicant nearly as fast as we would like, we
need to try and keep it happy. Unfortunately, the recent switch to
EVP_AEAD breaks it so this dismal change adds some dummy variables that
will allow it to compile.
Change-Id: I03d6b81c30bbebc07af3af0d6cda85a26b461edf
Reviewed-on: https://boringssl-review.googlesource.com/2960
Reviewed-by: Adam Langley <agl@google.com>
