ERR_FLAGS_STRING is meaningless and we can use a bitfield for the mark
bit.
Change-Id: I6f677b55b11316147512171629196c651cb33ca9
Reviewed-on: https://boringssl-review.googlesource.com/21084
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Cut down on the number of cases we need to worry about here. In
particular, it would be useful for the handshake to be able to replay an
error.
Change-Id: I2345faaff5503ede1324a5599e680de83f4b106e
Reviewed-on: https://boringssl-review.googlesource.com/21004
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.
Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This loosens the earlier restriction to match Channel ID. Both may be
configured and offered, but the server is obligated to select only one
of them. This aligns with the current tokbind + 0-RTT draft where the
combination is signaled by a separate extension.
Bug: 183
Change-Id: I786102a679999705d399f0091f76da236be091c2
Reviewed-on: https://boringssl-review.googlesource.com/19124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
WatchGuard's bug is very distinctive. Report a dedicated error code out
of BoringSSL so we can better track this.
Bug: chromium:733223
Change-Id: Ia42abd8654e7987b1d43c63a4f454f35f6aa873b
Reviewed-on: https://boringssl-review.googlesource.com/17328
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This imports upstream's scrypt implementation, though it's been heavily
revised. I lost track of words vs. blocks vs. bigger blocks too many
times in the original code and introduced a typedef for the fixed-width
Salsa20 blocks. The downside is going from bytes to blocks is a bit
trickier, so I took advantage of our little-endian assumption.
This also adds an missing check for N < 2^32. Upstream's code is making
this assumption in Integerify. I'll send that change back upstream. I've
also removed the weird edge case where a NULL out_key parameter means to
validate N/r/p against max_mem and nothing else. That's just in there to
get a different error code out of their PKCS#12 code.
Performance-wise, the cleanup appears to be the same (up to what little
precision I was able to get here), but an optimization to use bitwise
AND rather than modulus makes us measurably faster. Though scrypt isn't
a fast operation to begin with, so hopefully it isn't anyone's
bottleneck.
This CL does not route scrypt up to the PKCS#12 code, though we could
write our own version of that if we need to later.
BUG=chromium:731993
Change-Id: Ib2f43344017ed37b6bafd85a2c2b103d695020b8
Reviewed-on: https://boringssl-review.googlesource.com/17084
Reviewed-by: Adam Langley <agl@google.com>
A follow-up change will add a CRYPTO_BUFFER variant. This makes the
naming match the header and doesn't require including x509.h. (Though
like ssl.h and pkcs8.h, some of the functions are implemented with code
that depends on crypto/x509.)
Change-Id: I5a7de209f4f775fe0027893f711326d89699ca1f
Reviewed-on: https://boringssl-review.googlesource.com/15128
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BUG=76
Change-Id: I8b754ba17b3e0beee425929e4b53785b2e95f0ae
Reviewed-on: https://boringssl-review.googlesource.com/15164
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
As a precursor to removing the code entirely later, disable the protocol
by default. Callers must use SSL_CTX_set_min_version to enable it.
This change also makes SSLv3_method *not* enable SSL 3.0. Normally
version-specific methods set the minimum and maximum version to their
version. SSLv3_method leaves the minimum at the default, so we will
treat it as all versions disabled. To help debugging, the error code is
switched from WRONG_SSL_VERSION to a new NO_SUPPORTED_VERSIONS_ENABLED.
This also defines OPENSSL_NO_SSL3 and OPENSSL_NO_SSL3_METHOD to kick in
any no-ssl3 build paths in consumers which should provide a convenient
hook for any upstreaming changes that may be needed. (OPENSSL_NO_SSL3
existed in older versions of OpenSSL, so in principle one may encounter
an OpenSSL with the same settings.)
Change-Id: I96a8f2f568eb77b2537b3a774b2f7108bd67dd0c
Reviewed-on: https://boringssl-review.googlesource.com/14031
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BUG=187
Change-Id: I5775ce0886041b0c12174a7d665f3af1e8bce511
Reviewed-on: https://boringssl-review.googlesource.com/14505
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
It's amazing how short p_ed25519.c is.
BUG=187
Change-Id: Ib2a5fa7a4acf2087ece954506f81e91a1ed483e1
Reviewed-on: https://boringssl-review.googlesource.com/14449
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The resulting EVP_PKEYs do not do anything useful yet, but we are able
to parse them. Teaching them to sign will be done in a follow-up.
Creating these from in-memory keys is also slightly different from other
types. We don't have or need a public ED25519_KEY struct in
curve25519.h, so I've added tighter constructor functions which should
hopefully be easier to use anyway.
BUG=187
Change-Id: I0bbeea37350d4fdca05b6c6c0f152c15e6ade5bb
Reviewed-on: https://boringssl-review.googlesource.com/14446
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Channel ID is incompatible with 0-RTT, so we gracefully decline 0-RTT
as a server and forbid their combination as a client. We'll keep this
logic around until Channel ID is removed.
Channel ID will be replaced by tokbind which currently uses custom
extensions. Those will need additional logic to work with 0-RTT.
This is not implemented yet so, for now, fail if both are ever
configured together at all. A later change will allow the two to
combine.
BUG=183
Change-Id: I46c5ba883ccd47930349691fb08074a1fab13d5f
Reviewed-on: https://boringssl-review.googlesource.com/14370
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This isn't strictly necessary for Chromium yet, but we already have a
decoupled version of hash algorithm parsing available. For now, don't
export it but eventually we may wish to use it for OCSP.
BUG=54
Change-Id: If460d38d48bd47a2b4a853779f210c0cf7ee236b
Reviewed-on: https://boringssl-review.googlesource.com/14211
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This adds support on the server and client to accept data-less early
data. The server will still fail to parse early data with any
contents, so this should remain disabled.
BUG=76
Change-Id: Id85d192d8e0360b8de4b6971511b5e8a0e8012f7
Reviewed-on: https://boringssl-review.googlesource.com/12921
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We forgot to run the script at some point.
Change-Id: I0bd142fdd13d64c1ed81d9b1515449220d1c936b
Reviewed-on: https://boringssl-review.googlesource.com/14329
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
conf has the ability to expand variables in config files. Repeatedly doing
this can lead to an exponential increase in the amount of memory required.
This places a limit on the length of a value that can result from an
expansion.
Credit to OSS-Fuzz for finding this problem.
(Imported from upstream's 6a6213556a80ab0a9eb926a1d6023b8bf44f2afd. This
also import's upstream's ee1ccd0a41ad068957fe65ba7521e593b51bbad4 which
we had previously missed.)
Change-Id: I9be06a7e8a062b5adcd00c974a7b245226123563
Reviewed-on: https://boringssl-review.googlesource.com/14316
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This change adds support for setting an |SSL_TICKET_AEAD_METHOD| which
allows a caller to control ticket encryption and decryption to a greater
extent than previously possible and also permits asynchronous ticket
decryption.
This change only includes partial support: TLS 1.3 work remains to be
done.
Change-Id: Ia2e10ebb3257e1a119630c463b6bf389cf20ef18
Reviewed-on: https://boringssl-review.googlesource.com/14144
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
It's more consistent to have the helper function do the check that
its every caller already performs. This removes the error code
SSL_R_LIBRARY_HAS_NO_CIPHERS in favor of SSL_R_NO_CIPHER_MATCH.
Change-Id: I522239770dcb881d33d54616af386142ae41b29f
Reviewed-on: https://boringssl-review.googlesource.com/13964
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This allows a caller to configure a serving chain without dealing with
crypto/x509.
Change-Id: Ib42bb2ab9227d32071cf13ab07f92d029643a9a6
Reviewed-on: https://boringssl-review.googlesource.com/14126
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This allows us to move the code from Chrome into BoringSSL itself.
BUG=126
Change-Id: I04b4f63008a6de0a58dd6c685c78e9edd06deda6
Reviewed-on: https://boringssl-review.googlesource.com/14028
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BUG=129
Change-Id: I227ffa2da4e220075de296fb5b94d043f4e032e0
Reviewed-on: https://boringssl-review.googlesource.com/13627
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html
Add OPENSSL_memcpy, etc., wrappers which avoid this problem.
BUG=23
Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This change removes the use of |X509_get_pubkey| from the TLS <= 1.2
code. That function is replaced with a shallow parse of the certificate
to extract the public key instead.
Change-Id: I8938c6c5a01b32038c6b6fa58eb065e5b44ca6d2
Reviewed-on: https://boringssl-review.googlesource.com/12707
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Although we ignore all but the first identity, keep clients honest by
parsing the whole thing. Also explicitly check that the binder and
identity counts match.
Change-Id: Ib9c4caae18398360f3b80f8db1b22d4549bd5746
Reviewed-on: https://boringssl-review.googlesource.com/12469
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BUG=101
Change-Id: Ia1edbccee535b0bc3a0e18465286d5bcca240035
Reviewed-on: https://boringssl-review.googlesource.com/12470
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This change causes SSL_CTX_set_signed_cert_timestamp_list to check the
SCT list for shallow validity before allowing it to be set.
Change-Id: Ib8a1fe185224ff02ed4ce53a0109e60d934e96b3
Reviewed-on: https://boringssl-review.googlesource.com/12401
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
As a client, we must tolerate this to avoid interoperability failures
with allowed server behaviors.
BUG=117
Change-Id: I9c40a2a048282e2e63ab5ee1d40773fc2eda110a
Reviewed-on: https://boringssl-review.googlesource.com/12311
Reviewed-by: David Benjamin <davidben@google.com>
This is part of TLS 1.3 draft 16 but isn't much of a wire format change,
so go ahead and add it now. When rolling into Chromium, we'll want to
add an entry to the error mapping.
Change-Id: I8fd7f461dca83b725a31ae19ef96c890d603ce53
Reviewed-on: https://boringssl-review.googlesource.com/11563
Reviewed-by: David Benjamin <davidben@google.com>
BUG=77
Change-Id: If568412655aae240b072c29d763a5b17bb5ca3f7
Reviewed-on: https://boringssl-review.googlesource.com/10840
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
We'd previously been assuming we'd want to predict P-256 and X25519 but,
on reflection, that's nonsense. Although, today, P-256 is widespread and
X25519 is less so, that's not the right question to ask. Those servers
are all 1.2.
The right question is whether we believe enough servers will get to TLS
1.3 before X25519 to justify wasting 64 bytes on all other connections.
Given that OpenSSL has already shipped X25519 and Microsoft was doing
interop testing on X25519 around when we were shipping it, I think the
answer is no.
Moreover, if we are wrong, it will be easier to go from predicting one
group to two rather than the inverse (provided we send a fake one with
GREASE). I anticipate prediction-miss HelloRetryRequest logic across the
TLS/TCP ecosystem will be largely untested (no one wants to pay an RTT),
so taking a group out of the predicted set will likely be a risky
operation.
Only predicting one group also makes things a bit simpler. I haven't
done this here, but we'll be able to fold the 1.2 and 1.3 ecdh_ctx's
together, even.
Change-Id: Ie7e42d3105aca48eb9d97e2e05a16c5379aa66a3
Reviewed-on: https://boringssl-review.googlesource.com/10960
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Found by libFuzzer and then one more mistake caught by valgrind. Add a
test for this case.
Change-Id: I92773bc1231bafe5fc069e8568d93ac0df4c8acb
Reviewed-on: https://boringssl-review.googlesource.com/11129
Reviewed-by: David Benjamin <davidben@google.com>
Changing parameters on renegotiation makes all our APIs confusing. This
one has no reason to change, so lock it down. In particular, our
preference to forbid Token Binding + renego may be overridden at the
IETF, even though it's insane. Loosening it will be a bit less of a
headache if EMS can't change.
https://www.ietf.org/mail-archive/web/unbearable/current/msg00690.html
claims that this is already in the specification and enforced by NSS. I
can't find anything to this effect in the specification. It just says
the client MUST disable renegotiation when EMS is missing, which is
wishful thinking. At a glance, NSS doesn't seem to check, though I could
be misunderstanding the code.
Nonetheless, locking this down is a good idea anyway. Accurate or not,
take the email as an implicit endorsement of this from Mozilla.
Change-Id: I236b05991d28bed199763dcf2f47bbfb9d0322d7
Reviewed-on: https://boringssl-review.googlesource.com/10721
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
nginx consumes these error codes without #ifdefs. Continue to define
them for compatibility, even though we never emit them.
BUG=95
Change-Id: I1e991987ce25fc4952cc85b98ffa050a8beab92e
Reviewed-on: https://boringssl-review.googlesource.com/10446
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I2e1ee319bb9852b9c686f2f297c470db54f72279
Reviewed-on: https://boringssl-review.googlesource.com/10370
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The server should not be allowed select a protocol that wasn't
advertised. Callers tend to not really notice and act as if some default
were chosen which is unlikely to work very well.
Change-Id: Ib6388db72f05386f854d275bab762ca79e8174e6
Reviewed-on: https://boringssl-review.googlesource.com/10284
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This adds the machinery for doing TLS 1.3 1RTT.
Change-Id: I736921ffe9dc6f6e64a08a836df6bb166d20f504
Reviewed-on: https://boringssl-review.googlesource.com/8720
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is the equivalent of FragmentAcrossChangeCipherSuite for DTLS. It
is possible for us to, while receiving pre-CCS handshake messages, to
buffer up a message with sequence number meant for a post-CCS Finished.
When we then get to the new epoch and attempt to read the Finished, we
will process the buffered Finished although it was sent with the wrong
encryption.
Move ssl_set_{read,write}_state to SSL_PROTOCOL_METHOD hooks as this is
a property of the transport. Notably, read_state may fail. In DTLS
check the handshake buffer size. We could place this check in
read_change_cipher_spec, but TLS 1.3 has no ChangeCipherSpec message, so
we will need to implement this at the cipher change point anyway. (For
now, there is only an assert on the TLS side. This will be replaced with
a proper check in TLS 1.3.)
Change-Id: Ia52b0b81e7db53e9ed2d4f6d334a1cce13e93297
Reviewed-on: https://boringssl-review.googlesource.com/8790
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
For now, skip the 1.2 -> 1.1 signal since that will affect shipping
code. We may as well enable it too, but wait until things have settled
down. This implements the version in draft-14 since draft-13's isn't
backwards-compatible.
Change-Id: I46be43e6f4c5203eb4ae006d1c6a2fe7d7a949ec
Reviewed-on: https://boringssl-review.googlesource.com/8724
Reviewed-by: David Benjamin <davidben@google.com>
Rather than blindly select SHA-1 if we can't find a matching one, act as
if the peer advertised rsa_pkcs1_sha1 and ecdsa_sha1. This means that we
will fail the handshake if no common algorithm may be found.
This is done in preparation for removing the SHA-1 default in TLS 1.3.
Change-Id: I3584947909d3d6988b940f9404044cace265b20d
Reviewed-on: https://boringssl-review.googlesource.com/8695
Reviewed-by: David Benjamin <davidben@google.com>
In order to delay the digest of the handshake transcript and unify
around message-based signing callbacks, a copy of the transcript is kept
around until we are sure there is no certificate authentication.
This removes support for SSL_PRIVATE_KEY_METHOD as a client in SSL 3.0.
Change-Id: If8999a19ca021b4ff439319ab91e2cd2103caa64
Reviewed-on: https://boringssl-review.googlesource.com/8561
Reviewed-by: David Benjamin <davidben@google.com>
This also adds a missing check to the C half to ensure fake record types are
always correct, to keep implementations honest.
Change-Id: I1d65272e647ffa67018c721d52c639f8ba47d647
Reviewed-on: https://boringssl-review.googlesource.com/8510
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The case where ec_group_get_mont_data is NULL is only for arbitrary groups
which we now require to be prime order. BN_mod_exp_mont is fine with a NULL
BN_MONT_CTX. It will just compute it. Saves a bit of special-casing.
Also don't mark p-2 as BN_FLG_CONSTTIME as the exponent is public anyway.
Change-Id: Ie868576d52fc9ae5f5c9f2a4039a729151bf84c7
Reviewed-on: https://boringssl-review.googlesource.com/8307
Reviewed-by: Adam Langley <agl@google.com>
There's a __pragma expression which allows this. Android builds us Windows with
MinGW for some reason, so we actually do have to tolerate non-MSVC-compatible
Windows compilers. (Clang for Windows is much more sensible than MinGW and
intentionally mimicks MSVC.)
MinGW doesn't understand MSVC's pragmas and warns a lot. #pragma warning is
safe to suppress, so wrap those to shush them. This also lets us do away with a
few ifdefs.
Change-Id: I1f5a8bec4940d4b2d947c4c1cc9341bc15ec4972
Reviewed-on: https://boringssl-review.googlesource.com/8236
Reviewed-by: Adam Langley <agl@google.com>
Windows SRWLOCK requires you call different functions here. Split
them up in preparation for switching Windows from CRITICAL_SECTION.
BUG=37
Change-Id: I7b5c6a98eab9ae5bb0734b805cfa1ff334918f35
Reviewed-on: https://boringssl-review.googlesource.com/8080
Reviewed-by: Adam Langley <agl@google.com>
