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>
