Probably best to keep the original format, trailing whitespace and all.
Change-Id: I81a0ac46fd4ab4bb9d2b03d930b191024971447c
Reviewed-on: https://boringssl-review.googlesource.com/2473
Reviewed-by: Adam Langley <agl@google.com>
This got reset at some point, but not the files generated from it.
obj_mac.num is an input/output parameter to objects.pl and used to keep the
NIDs stable.
Imported from f2d678e6e89b6508147086610e985d4e8416e867, the point at which we
forked.
Change-Id: Ifd52b1aaa55054d37bc1217f2375a93302839e23
Reviewed-on: https://boringssl-review.googlesource.com/2472
Reviewed-by: Adam Langley <agl@google.com>
Make the commands print a short usage summary and add a README file that
explains the dependencies.
Change-Id: I0c3f0713749ecfca23afaa2b536ac70dbdd7db0a
Reviewed-on: https://boringssl-review.googlesource.com/2471
Reviewed-by: Adam Langley <agl@google.com>
Upstream (impressively quickly) fixed the missing intrinsic. Switch Windows
clang back to building the same code as MSVC. Also include the intrin.h header
rather than forward-declare the intrinsic. clang only works if the header is
explicitly included. Chromium forcibly includes it to work around these kinds
of issues, but we shouldn't rely on it.
BUG=crbug.com/438382
Change-Id: I0ff6d48e1a3aa455cff99f8dc4c407e88b84d446
Reviewed-on: https://boringssl-review.googlesource.com/2461
Reviewed-by: Adam Langley <agl@google.com>
MSVC doesn't like it when you compare the two.
Change-Id: I03c5ff2e2668ac2e536de8278e3a7c98a3dfd117
Reviewed-on: https://boringssl-review.googlesource.com/2460
Reviewed-by: Adam Langley <agl@google.com>
It just inserts extra flushes everywhere and isn't used.
Change-Id: I082e4bada405611f4986ba852dd5575265854036
Reviewed-on: https://boringssl-review.googlesource.com/2456
Reviewed-by: Adam Langley <agl@google.com>
Use it in ssl3_cert_verify_hash so signing a pre-TLS-1.2 handshake hash can go
through RSA_sign and be intercepted via RSA_METHOD appropriately. This avoids
Windows needing to intercept sign_raw. (CAPI keys cannot provide sign_raw,
unless the input size happens to be that of NID_md5_sha1.)
Also use it in processing ServerKeyExchange to avoid special-casing RSA.
BUG=crbug.com/437023
Change-Id: Ia07433f468b75fdf7bfc8fa90c9751639b2478e6
Reviewed-on: https://boringssl-review.googlesource.com/2420
Reviewed-by: David Benjamin <davidben@google.com>
Windows clang lacks _umul128, but it has inline assembly so just use
that.
Change-Id: I6ff5d2465edc703a4d47ef0efbcea43d6fcc79fa
Reviewed-on: https://boringssl-review.googlesource.com/2454
Reviewed-by: Adam Langley <agl@google.com>
Don't link with dl, except on Linux where we have malloc tests.
Change-Id: I7b23acc854172e64628a55acecfaa9a661f74f77
Reviewed-on: https://boringssl-review.googlesource.com/2453
Reviewed-by: Adam Langley <agl@google.com>
The comment has it right, but the rewritten code was wrong.
Change-Id: I450193c39fb62eae32aae090a3834dd83db53421
Reviewed-on: https://boringssl-review.googlesource.com/2444
Reviewed-by: Adam Langley <agl@google.com>
This probably snuck in when adapting the code from upstream. There's a header
file for it now. (Also it's uint32_t now rather than unsigned int.)
Change-Id: Ie8f45bc7a88988744174182a70512c0eff37cc1c
Reviewed-on: https://boringssl-review.googlesource.com/2441
Reviewed-by: Adam Langley <agl@google.com>
Replace the comment with a clearer one and reimplement it much more tidily. The
mask thing was more complicated than was needed.
This slightly changes behavior on the DTLS_ANY_VERSION side in that, if only
one method is enabled, we no longer short-circuit to the version-locked method
early. This "optimization" seems unnecessary.
Change-Id: I571c8b60ed16bd4357c67d65df0dd1ef9cc5eb57
Reviewed-on: https://boringssl-review.googlesource.com/2451
Reviewed-by: Adam Langley <agl@google.com>
It should be set correctly prior to entering the handshake. Don't mask bugs by
assigning it.
Change-Id: Ib9bca8fad68916b3b242aad8819e3760e59e777a
Reviewed-on: https://boringssl-review.googlesource.com/2443
Reviewed-by: Adam Langley <agl@google.com>
first_packet is a temporary connection-global flag set for the duration of some
call and then queried from other code. This kind of logic is too difficult to
reason through. It also incorrectly treats renegotiate ClientHellos as
pre-version-negotiation records. This eliminates the need to query
enc_write_ctx (which wasn't EVP_AEAD-aware anyway).
Instead, take a leaf from Go TLS's book and add a have_version bit. This is
placed on s->s3 as it is connection state; s->s3 automatically gets reset on
SSL_clear while s doesn't.
This new flag will also be used to determine whether to do the V2ClientHello
sniff when the version-locked methods merge into SSLv23_method. It will also
replace needing to condition s->method against a dummy DTLS_ANY_VERSION value
to determine whether DTLS version negotiation has happened yet.
Change-Id: I5c8bc6258b182ba4ab175a48a84eab6d3a001333
Reviewed-on: https://boringssl-review.googlesource.com/2442
Reviewed-by: Adam Langley <agl@google.com>
Supporting both schemes seems pointless. Now that s->server and s->state are
set appropriately late and get_ssl_method is gone, the only difference is that
the client/server ones have non-functional ssl_accept or ssl_connect hooks. We
can't lose the generic ones, so let's unify on that.
Note: this means a static linker will no longer drop the client or server
handshake code if unused by a consumer linking statically. However, Chromium
needs the server half anyway for DTLS and WebRTC, so that's probably a lost
cause. Android also exposes server APIs.
Change-Id: I290f5fb4ed558f59fadb5d1f84e9d9c405004c23
Reviewed-on: https://boringssl-review.googlesource.com/2440
Reviewed-by: Adam Langley <agl@google.com>
If SSL_clear is called before SSL_set_{connect,accept}_state (as SSL_new does
internally), s->state will get set prematurely. Likewise, s->server is set
based on the method's ssl_accept hook, but client SSL's may be initialized from
a generic SSL_METHOD too.
Since we can't easily get rid of the generic SSL_METHODs, defer s->state and
s->server initialization until the side is known.
Change-Id: I0972e17083df22a3c09f6f087011b54c699a22e7
Reviewed-on: https://boringssl-review.googlesource.com/2439
Reviewed-by: Adam Langley <agl@google.com>
It's unused. Also per the previous commit message, it historically had a bug
anyway.
Change-Id: I5868641e7938ddebbc0ffd72d218c81cd17c7739
Reviewed-on: https://boringssl-review.googlesource.com/2437
Reviewed-by: Adam Langley <agl@google.com>
If the state is SSL_ST_BEFORE, the SSL* was just initialized. Otherwise, we
don't want to call SSL_clear. The one case I found where we do is if a
handshake message is received and someone sets
SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS. This is apparently intended for external
consumers to set, but I see no code in Google that does.
Which is fortunate because it'll trigger SSL_clear. This retains the BIOs but
drops all connection state, including the record. If the client just initiated
renego, that's the ClientHello that's lost. The connection then hangs: the now
reset SSL* wants a ClientHello (under the null cipher because that too's been
dropped) while the peer wants an encrypted ServerHello.
Change-Id: Iddb3e0bb86d39d98155b060f9273a0856f2d1409
Reviewed-on: https://boringssl-review.googlesource.com/2436
Reviewed-by: Adam Langley <agl@google.com>
SSL_clear sets s->state and dtls1_clear sets cookie_len on the server. Setting
cookie_len on the server seems to serve no purpose but to let the callback know
how large the buffer is. This can be done just before calling the callback.
It also avoids a bug where the cookie check can be bypassed, should the server
not specify an app_verify_cookie_cb, by supplying a cookie of all zeros of the
maximum size. (Zero is fine because an empty cookie is rejected.)
The goal here is to avoid needing the SSL_clear calls in the handshake
functions. They are currently needed to fix the cookie_len setting when using
the generic method. (They get set wrong and then flipped back.)
Change-Id: I5095891bc0f7df62d83a9c84312fcf0b84826faa
Reviewed-on: https://boringssl-review.googlesource.com/2435
Reviewed-by: Adam Langley <agl@google.com>
s->server's value isn't final until SSL_connect or SSL_accept is called when
using the generic SSLv23_method or DTLS_method rather than the version-locked
ones. This makes the tests pass if bssl_shim uses those methods.
It would be nicer if the generic methods were gone and an SSL* could know from
creation which half it's destined for. Unfortunately, there's a lot of code
that uses those generic methods, so we probably can't get rid of them. If they
have to stay, it seems better to standardize on only having those, rather than
support both, even if standardizing on the side-specific ones would be
preferable.
Change-Id: I40e65a8842cd6706da92263a263f664336a7f3b3
Reviewed-on: https://boringssl-review.googlesource.com/2434
Reviewed-by: Adam Langley <agl@google.com>
SSL_ST_BEFORE is never standalone. As of upstream's
413c4f45ed0508d2242638696b7665f499d68265, SSL_ST_BEFORE is only ever set paired
with SSL_ST_ACCEPT or SSL_ST_CONNECT.
Conversely, SSL_ST_OK is never paired with SSL_ST_ACCEPT or SSL_ST_CONNECT. As
far as I can tell, this combination has never been possible.
Change-Id: Ifbc8f147be821026cf59f3d5038f0dbad3b0a1d2
Reviewed-on: https://boringssl-review.googlesource.com/2433
Reviewed-by: Adam Langley <agl@google.com>
It should already be assigned, as of upstream's
b31b04d951. I believe these assignments are part
of the reason it used to appear to work. Replace them with assertions. So the
assertions are actually valid, check in SSL_connect / SSL_accept that they are
never called if the socket had been placed in the opposite state. (Or we'd be
in another place where it would have appeared to work with the handshake
functions fixing things afterwards.)
Now the only places handshake_func is set are in SSL_set_{connect,accept}_state
and the method switches.
Change-Id: Ib249212bf4aa889b94c35965a62ca06bdbcf52e1
Reviewed-on: https://boringssl-review.googlesource.com/2432
Reviewed-by: Adam Langley <agl@google.com>
This comment is no longer true. It dates from OpenSSL's initial commit, but
stopped being true in upstream's 413c4f45ed.
Change-Id: I47377d992a00e3d57c795fef893e19e109dd6945
Reviewed-on: https://boringssl-review.googlesource.com/2431
Reviewed-by: Adam Langley <agl@google.com>
We intend to deprecate the version-locked methods and unify them. Don't expose
that there's a method swap. (The existing version-locked methods will merely be
a shorthand for configuring minimum/maximum versions.)
There is one consumer of SSL_get_ssl_method in internal code, but it's just
some logging in test-only code. All it's doing is getting the version as a
string which should be SSL_get_version instead.
While here, also remove dead ssl_bad_method function. Also the bogus
ssl_crock_st forward-declaration. The forward declaration in base.h should be
perfectly sufficient.
Change-Id: I50480808f51022e05b078a285f58ec85d5ad7c8e
Reviewed-on: https://boringssl-review.googlesource.com/2408
Reviewed-by: Adam Langley <agl@google.com>
b9cc33a4d6 deleted its documentation rather than
SSL_OP_EPHEMERAL_RSA's.
Change-Id: I2e099a2dc498f145c5a3ccaac824edbda27f7e89
Reviewed-on: https://boringssl-review.googlesource.com/2407
Reviewed-by: Adam Langley <agl@google.com>
They're mapped to the same value, which is the only reason the tests work right
now.
Change-Id: I22f6e3a6b3a2c88b0f92b6d261e86111b4172cd6
Reviewed-on: https://boringssl-review.googlesource.com/2406
Reviewed-by: Adam Langley <agl@google.com>
When not offering to resume a session, the client populates s->session with a
fresh SSL_SESSION before the ServerHello is processed and, in DTLS_ANY_VERSION,
before the version is even determined. Don't create a fresh SSL_SESSION until
we know we are doing a full handshake.
This brings ssl3_send_client_hello closer to ssl23_client_hello in behavior. It
also fixes ssl_version in the client in DTLS_ANY_VERSION.
SSLv23_client_method is largely unchanged. If no session is offered, s->session
continues to be NULL until the ServerHello is received. The one difference is
that s->session isn't populated until the entire ServerHello is received,
rather than just the first half, in the case of a fragmented ServerHello. Apart
from info_callback, no external hooks get called between those points, so this
shouldn't expose new missing NULL checks.
The other client methods change significantly to match SSLv23_client_method's
behavior. For TLS, any exposed missing NULL checks are also in
SSLv23_client_method (and version-specific methods are already weird), so that
should be safe. For DTLS, I've verified that accesses in d1_*.c either handle
NULL or are after the ServerHello.
Change-Id: Idcae6bd242480e28a57dbba76ce67f1ac1ae1d1d
Reviewed-on: https://boringssl-review.googlesource.com/2404
Reviewed-by: Adam Langley <agl@google.com>
This fixes bugs that kept the tests from working:
- Resolve DTLS version and cookie before the session.
- In DTLS_ANY_VERSION, ServerHello should be read with first_packet = 1. This
is a regression from f2fedefdca. We'll want to
do the same for TLS, but first let's change this to a boolean has_version in a
follow-up.
Things not yet fixed:
- DTLS code is not EVP_AEAD-aware. Those ciphers are disabled for now.
- On the client, DTLS_ANY_VERSION creates SSL_SESSIONs with the wrong
ssl_version. The tests pass because we no longer enforce the match as of
e37216f560. (In fact, we've gone from the server
ignoring ssl_version and client enforcing to the client mostly ignoring
ssl_version and the server enforcing.)
- ssl3_send_client_hello's ssl_version check checks for equality against
s->version rather than >.
Change-Id: I5a0dde221b2009413df9b9443882b9bf3b29519c
Reviewed-on: https://boringssl-review.googlesource.com/2403
Reviewed-by: Adam Langley <agl@google.com>
This is a bit of cleanup that probably should have been done at the same time
as 30ddb434bf.
For now, version negotiation is implemented with a method swap. It also
performs this swap on SSL_set_session, but this was neutered in
30ddb434bf. Rather than hackishly neuter it,
remove it outright. In addition, remove SSL_set_ssl_method. Now all method
swaps are internal: SSLv23_method switch to a version-specific method and
SSL_clear undoing it.
Note that this does change behavior: if an SSL* is created with one
version-specific method and we SSL_set_session to a session from a /different/
version, we would switch to the /other/ version-specific method. This is
extremely confusing, so it's unlikely anyone was actually expecting it.
Version-specific methods in general don't work well.
Change-Id: I72a5c1f321ca9aeb1b52ebe0317072950ba25092
Reviewed-on: https://boringssl-review.googlesource.com/2390
Reviewed-by: Adam Langley <agl@google.com>
We forgot to add those when we implemented the features. (Also relevant because
they will provide test coverage later for configuring features when using the
generic method tables rather than *_client_method.)
Change-Id: Ie08b27de893095e01a05a7084775676616459807
Reviewed-on: https://boringssl-review.googlesource.com/2410
Reviewed-by: Adam Langley <agl@google.com>
Although the comment suggests this was added with an s->session check to
account for SSL_set_session switching methods (which we will remove in the next
commit) and to account for SSLv23_method switching methods (which we hope to
remove after a long tower of cleanup), the current codepath never runs and
can't work:
If it is called prior to handshaking or setting a session, no method switch has
happened so that codepath is dead. If it is called after setting a session, the
s->session check will keep it from running. If it is called after a handshake,
we will have established a session so that check will again keep it from
running. (Finally, if it is called during the handshake, the in_handshake check
will stop; that there is an SSL_clear call in the handshake state machine at
all is a bug that will be addressed once more things are disentangled. See
upstream's 979689aa5cfa100ccbc1f25064e9398be4b7b05c.)
Were that code to ever run, the SSL* would be in an inconsistent state. It
switches the method, but not the handshake_func. The handshake_func isn't
switched to NULL, so that will keep the SSL_connect and SSL_accept code from fixing it.
It seems the intent was that the caller would always call
SSL_set_{connect,accept}_state to fix this. But as of upstream's
b31b04d951e9b65bde29657e1ae057b76f0f0a73, this is not necessary and indeed
isn't called by a lot of consumer code.
Change-Id: I710652b1d565b77bc26f913c2066ce749a9025c9
Reviewed-on: https://boringssl-review.googlesource.com/2430
Reviewed-by: Adam Langley <agl@google.com>
The data is owned by the SSL_SESSION, so the caller should not modify it. This
will require changes in Chromium, but they should be trivial.
Change-Id: I314718530c7d810f7c7b8852339b782b4c2dace1
Reviewed-on: https://boringssl-review.googlesource.com/2409
Reviewed-by: Adam Langley <agl@google.com>
MSVC does not allow pointer arithmetic on void* pointers. Also
fix some style issues around whether * hugs the type or the
variable name.
Change-Id: I40cc1627830b37879fd70e2b688a42df62b6c62a
Reviewed-on: https://boringssl-review.googlesource.com/2452
Reviewed-by: Adam Langley <agl@google.com>
Don't use |BIO_set_foo_buffer_size| when setting the
sizes of the buffers while making buffer pair. Since it
happens in pair.c we know the BIOs are BIO pairs and using
bio_ctrl here complicates setting external buffers. Also
zero out bio_bio_st during construction.
This fixes a problem that would happen if the default buffer
sizes were not set, since buf_externally_allocated was
not yet initialized.
Remove BIO_C_SET_BUFF_SIZE and BIO_CTRL_RESET which are
not used for bio pairs.
Change-Id: I365091d5f44f6f1c5522c325a771bdf03d8fe950
Reviewed-on: https://boringssl-review.googlesource.com/2370
Reviewed-by: Adam Langley <agl@google.com>
This is only used for EAP-FAST which we apparently don't need to support.
Remove it outright. We broke it in 9eaeef81fa by
failing to account for session misses.
If this changes and we need it later, we can resurrect it. Preferably
implemented differently: the current implementation is bolted badly onto the
handshake. Ideally use the supplied callbacks to fabricate an appropriate
SSL_SESSION and resume that with as much of the normal session ticket flow as
possible.
The one difference is that EAP-FAST seems to require the probing mechanism for
session tickets rather than the sane session ID echoing version. We can
reimplement that by asking the record layer to probe ahead for one byte.
Change-Id: I38304953cc36b2020611556a91e8ac091691edac
Reviewed-on: https://boringssl-review.googlesource.com/2360
Reviewed-by: Adam Langley <agl@google.com>
This implements session IDs in client and server in runner.go.
Change-Id: I26655f996b7b44c7eb56340ef6a415d3f2ac3503
Reviewed-on: https://boringssl-review.googlesource.com/2350
Reviewed-by: Adam Langley <agl@google.com>
The ex_data index may fail to be allocated. Also don't leave a dangling pointer
in handshake_dgst if EVP_DigestInit_ex fails and check a few more init function
failures.
Change-Id: I2e99a89b2171c9d73ccc925a2f35651af34ac5fb
Reviewed-on: https://boringssl-review.googlesource.com/2342
Reviewed-by: Adam Langley <agl@google.com>
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.
(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)
This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.
Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
`uname -p` is still i386 on OS X for some reason, which causes cmake 2.8 to set
CMAKE_SYSTEM_PROCESSOR to i386, making the build think it's doing a 32-bit
build. However, since the system is almost completely 64-bit these days, clang
defaults to producing 64-bit object files unless told otherwise. As a result,
the produced .o files are all 64-bit except for the .o files from assembly, and
then linking fails.
Fix this by forcing ARCH to 64-bit on OS X. This matches the default behavior
of cmake 3.0, where CMAKE_SYSTEM_PROCESSOR is x86_64.
Change-Id: I7a2abc4cef84dfbaf205852a9d7b647e83dd249f
Reviewed-on: https://boringssl-review.googlesource.com/2330
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Piotr Sikora <piotr@cloudflare.com>
tls1_process_sigalgs now only determines the intersection between the peer
algorithms and those configured locally. That list is queried later to
determine the hash algorithm to use when signing CertificateVerify or
ServerKeyExchange.
This is needed to support client auth on Windows where smartcards or CAPI may
not support all hash functions.
As a bonus, this does away with more connection-global state. This avoids the
current situation where digests are chosen before keys are known (for
CertificateVerify) or for slots that don't exist.
Change-Id: Iec3619a103d691291d8ebe08ef77d574f2faf0e8
Reviewed-on: https://boringssl-review.googlesource.com/2280
Reviewed-by: Adam Langley <agl@google.com>
CERT_PKEY_SIGN isn't meaningful since, without strict mode, we always fall back
to SHA-1 anyway. So the digest is never NULL when CERT_PKEY_SIGN is computed.
The entire valid_flags is now back to it's pre-1.0.2 check of seeing if the
certificate and key are configured.
This finally removes the sensitivity between valid_flags and selecting the
digest, so we can defer choosing the digest all we like.
Change-Id: I9f9952498f512d7f0cc799497f7c5b52145a48af
Reviewed-on: https://boringssl-review.googlesource.com/2288
Reviewed-by: Adam Langley <agl@google.com>
It doesn't depend on the cipher now that export ciphers are gone. It need only
be called once. Also remove the valid bit; nothing ever reads it. Its output is
also only used within a function, so make mask_k and mask_a local variables.
So all the configuration-based checks are in one place, change the input
parameter from CERT to SSL and move the PSK and ECDHE checks to the mask
computation. This avoids having to evaluate the temporary EC key for each
cipher.
The remaining uses are on the client which uses them differently (disabled
features rather than enabled ones). Those too may as well be local variables,
so leave a TODO.
Change-Id: Ibcb574341795d4016ea749f0290a793eed798874
Reviewed-on: https://boringssl-review.googlesource.com/2287
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Håvard Molland
Adam Langley
Nico Weber