Commit Graph

187 Commits

Author SHA1 Message Date
David Benjamin
1c2532ffe6 Fix error strings for SSL_R_TLS13_DOWNGRADE.
make_errors.go didn't seem to get run.

Change-Id: I12739fbab75b9f4898f73f206e404d101642b9c0
Reviewed-on: https://boringssl-review.googlesource.com/31184
Reviewed-by: Adam Langley <agl@google.com>
2018-08-22 01:26:47 +00:00
Adam Langley
826ce15092 Support OpenSSL APIs SSL[_CTX]_set1_sigalgs[_list].
These functions can be used to configure the signature algorithms. One
of them is a string mini-languaging parsing function, which we generally
dislike because it defeats static analysis. However, some dependent
projects (in this case TensorFlow) need it and we also dislike making
people patch.

Change-Id: I13f990c896a7f7332d78b1c351357d418ade8d11
Reviewed-on: https://boringssl-review.googlesource.com/30304
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-08-09 16:57:09 +00:00
Adam Langley
4732c544f7 Add ECDH_compute_key_fips inside the module.
This change adds a function so that an ECDH and the hashing of the
resulting 'x' coordinate can occur inside the FIPS boundary.

Change-Id: If93c20a70dc9dcbca49056f10915d3ce064f641f
Reviewed-on: https://boringssl-review.googlesource.com/30104
Reviewed-by: Adam Langley <agl@google.com>
2018-07-30 22:40:31 +00:00
Adam Langley
0080d83b9f Implement the client side of certificate compression.
Change-Id: I0aced480af98276ebfe0970b4afb9aa957ee07cb
Reviewed-on: https://boringssl-review.googlesource.com/29024
Reviewed-by: Adam Langley <agl@google.com>
2018-06-18 22:16:11 +00:00
David Benjamin
5267ef7b4a Reject unexpected application data in bidirectional shutdown.
Update-Note: This tweaks the SSL_shutdown behavior. OpenSSL's original
SSL_shutdown behavior was an incoherent mix of discarding the record and
rejecting it (it would return SSL_ERROR_SYSCALL but retrying the
operation would discard it). SSLeay appears to have intended to discard
it, so we previously "fixed" it actually discard.

However, this behavior is somewhat bizarre and means we skip over
unbounded data, which we typically try to avoid. If you are trying to
cleanly shutdown the TLS portion of your protocol, surely it is at a
point where additional data is a syntax error. I suspect I originally
did not realize that, because the discarded record did not properly
continue the loop, SSL_shutdown would appear as if it rejected the data,
and so it's unlikely anyone was relying on that behavior.

Discussion in https://github.com/openssl/openssl/pull/6340 suggests
(some of) upstream also prefers rejecting.

Change-Id: Icde419049306ed17eb06ce1a7e1ff587901166f3
Reviewed-on: https://boringssl-review.googlesource.com/28864
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-06-04 21:39:58 +00:00
David Benjamin
caf8ddd0ba Add SSL_SESSION_set1_id.
This matches the OpenSSL 1.1.0 spelling. I'd thought we could hide
SSL_SESSION this pass, but I missed one test that messed with session
IDs!

Bug: 6
Change-Id: I84ea113353eb0eaa2b06b68dec71cb9061c047ca
Reviewed-on: https://boringssl-review.googlesource.com/28866
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>
2018-06-04 14:25:28 +00:00
David Benjamin
d12f2ba55e Tweak RSA errors for compatibility.
cryptography.io wants RSA_R_BLOCK_TYPE_IS_NOT_02, only used by the
ancient RSA_padding_check_SSLv23 function. Define it but never emit it.

Additionally, it's rather finicky about RSA_R_TOO_LARGE* errors. We
merged them in BoringSSL because having RSA_R_TOO_LARGE,
RSA_R_TOO_LARGE_FOR_MODULUS, and RSA_R_TOO_LARGE_FOR_KEY_SIZE is a
little silly. But since we don't expect well-behaved code to condition
on error codes anyway, perhaps that wasn't worth it.  Split them back
up.

Looking through OpenSSL, there is a vague semantic difference:

RSA_R_DIGEST_TOO_BIG_FOR_RSA_KEY - Specifically emitted if a digest is
too big for PKCS#1 signing with this key.

RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE - You asked me to sign or encrypt a
digest/plaintext, but it's too big for this key.

RSA_R_DATA_TOO_LARGE_FOR_MODULUS - You gave me an RSA ciphertext or
signature and it is not fully reduced modulo N.
-OR-
The padding functions produced something that isn't reduced, but I
believe this is unreachable outside of RSA_NO_PADDING.

RSA_R_DATA_TOO_LARGE - Some low-level padding function was told to copy
a digest/plaintext into some buffer, but the buffer was too small. I
think this is basically unreachable.
-OR-
You asked me to verify a PSS signature, but I didn't need to bother
because the digest/salt parameters you picked were too big.

Update-Note: This depends on cl/196566462.
Change-Id: I2e539e075eff8bfcd52ccde365e975ebcee72567
Reviewed-on: https://boringssl-review.googlesource.com/28547
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 23:02:49 +00:00
David Benjamin
103ed08549 Implement legacy OCSP APIs for libssl.
Previously, we'd omitted OpenSSL's OCSP APIs because they depend on a
complex OCSP mechanism and encourage the the unreliable server behavior
that hampers using OCSP stapling to fix revocation today. (OCSP
responses should not be fetched on-demand on a callback. They should be
managed like other server credentials and refreshed eagerly, so
temporary CA outage does not translate to loss of OCSP.)

But most of the APIs are byte-oriented anyway, so they're easy to
support. Intentionally omit the one that takes a bunch of OCSP_RESPIDs.

The callback is benign on the client (an artifact of OpenSSL reading
OCSP and verifying certificates in the wrong order). On the server, it
encourages unreliability, but pyOpenSSL/cryptography.io depends on this.
Dcument that this is only for compatibility with legacy software.

Also tweak a few things for compatilibility. cryptography.io expects
SSL_CTX_set_read_ahead to return something, SSL_get_server_tmp_key's
signature was wrong, and cryptography.io tries to redefine
SSL_get_server_tmp_key if SSL_CTRL_GET_SERVER_TMP_KEY is missing.

Change-Id: I2f99711783456bfb7324e9ad972510be8a95e845
Reviewed-on: https://boringssl-review.googlesource.com/28404
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 22:21:26 +00:00
David Benjamin
2e67153de4 Add PKCS12_create.
PyOpenSSL calls this function these days. Tested by roundtripping with
ourselves and also manually confirming our output interoperates with
OpenSSL.  (For anyone repeating this experiment, the OpenSSL
command-line tool has a bug and does not correctly output friendlyName
attributes with non-ASCII characters. I'll send them a PR to fix this
shortly.)

Between this and the UTF-8 logic earlier, the theme of this patch series
seems to be "implement in C something I last implemented in
JavaScript"...

Change-Id: I258d563498d82998c6bffc6789efeaba36fe3a5e
Reviewed-on: https://boringssl-review.googlesource.com/28328
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:59:34 +00:00
David Benjamin
bc2562e50e Treat PKCS#12 passwords as UTF-8.
This aligns with OpenSSL 1.1.0's behavior, which deviated from OpenSSL
1.0.2. OpenSSL 1.0.2 effectively assumed input passwords were always
Latin-1.

Update-Note: If anyone was using PKCS#12 passwords with non-ASCII
characters, this changes them from being encoding-confused to hopefully
interpretting "correctly". If this breaks anything, we can add a
fallback to PKCS12_get_key_and_certs/PKCS12_parse, but OpenSSL 1.1.0
does not have such behavior. It only implements a fallback in the
command-line tool, not the APIs.

Change-Id: I0aa92db26077b07a40f85b89f4d3e0f6b0d7be87
Reviewed-on: https://boringssl-review.googlesource.com/28326
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:58:56 +00:00
David Benjamin
ae153bb9a6 Use new encoding functions in ASN1_mbstring_ncopy.
Update-Note: This changes causes BoringSSL to be stricter about handling
Unicode strings:
  · Reject code points outside of Unicode
  · Reject surrogate values
  · Don't allow invalid UTF-8 to pass through when the source claims to
    be UTF-8 already.
  · Drop byte-order marks.

Previously, for example, a UniversalString could contain a large-valued
code point that would cause the UTF-8 encoder to emit invalid UTF-8.

Change-Id: I94d9db7796b70491b04494be84249907ff8fb46c
Reviewed-on: https://boringssl-review.googlesource.com/28325
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:58:47 +00:00
David Benjamin
5d626b223b Add some more compatibility functions.
Change-Id: I56afcd896cb9de1c69c788b4f6395f4e78140d81
Reviewed-on: https://boringssl-review.googlesource.com/28265
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>
2018-05-08 20:51:15 +00:00
David Benjamin
ed188fd8ef Enforce supported_versions in the second ServerHello.
We forgot to do this in our original implementation on general ecosystem
grounds. It's also mandated starting draft-26.

Just to avoid unnecessary turbulence, since draft-23 is doomed to die
anyway, condition this on our draft-28 implementation. (We don't support
24 through 27.)

We'd actually checked this already on the Go side, but the spec wants a
different alert.

Change-Id: I0014cda03d7129df0b48de077e45f8ae9fd16976
Reviewed-on: https://boringssl-review.googlesource.com/28124
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-05-07 19:05:20 +00:00
David Benjamin
3c37d0aba5 Reland "Fix bssl client/server's error-handling."
Rather than printing the SSL_ERROR_* constants, print the actual error.
This should be a bit more understandable. Debugging this also uncovered
some other issues on Windows:

- We were mixing up C runtime and Winsock errors, which are separate in
  Windows.

- The thread local implementation interferes with WSAGetLastError due to
  a quirk of TlsGetValue. This could affect other Windows consumers.
  (Chromium uses a custom BIO, so it isn't affected.)

- SocketSetNonBlocking also interferes with WSAGetLastError.

- Listen for FD_CLOSE along with FD_READ. Connection close does not
  signal FD_READ. (The select loop only barely works on Windows anyway
  due to issues with stdin and line buffering, but if we take stdin out
  of the equation, FD_CLOSE can be tested.)

Change-Id: Ia8d42b5ac39ebb3045d410dd768f83a3bb88b2cb
Reviewed-on: https://boringssl-review.googlesource.com/28186
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-07 17:19:59 +00:00
Steven Valdez
0cdbc876a2 Revert "Fix bssl client/server's error-handling."
This reverts commit e7ca8a5d78.

Change-Id: Ib2f923760dc54400f45e9327b3a45466be1dd6d1
Reviewed-on: https://boringssl-review.googlesource.com/28184
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>
2018-05-07 16:53:09 +00:00
David Benjamin
e7ca8a5d78 Fix bssl client/server's error-handling.
Rather than printing the SSL_ERROR_* constants, print the actual error.
This should be a bit more understandable. Debugging this also uncovered
some other issues on Windows:

- We were mixing up C runtime and Winsock errors, which are separate in
  Windows.

- The thread local implementation interferes with WSAGetLastError due to
  a quirk of TlsGetValue. This could affect other Windows consumers.
  (Chromium uses a custom BIO, so it isn't affected.)

- SocketSetNonBlocking also interferes with WSAGetLastError.

- Listen for FD_CLOSE along with FD_READ. Connection close does not
  signal FD_READ. (The select loop only barely works on Windows anyway
  due to issues with stdin and line buffering, but if we take stdin out
  of the equation, FD_CLOSE can be tested.)

Change-Id: If991259915acc96606a314fbe795fe6ea1e295e8
Reviewed-on: https://boringssl-review.googlesource.com/28125
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>
2018-05-07 15:44:08 +00:00
David Benjamin
c1c6eeb5e2 Check d is mostly-reduced in RSA_check_key.
We don't check it is fully reduced because different implementations use
Carmichael vs Euler totients, but if d exceeds n, something is wrong.
Note the fixed-width BIGNUM changes already fail operations with
oversized d.

Update-Note: Some blatantly invalid RSA private keys will be rejected at
    RSA_check_key time. Note that most of those keys already are not
    usable with BoringSSL anyway. This CL moves the failure from
    sign/decrypt to RSA_check_key.

Change-Id: I468dbba74a148aa58c5994cc27f549e7ae1486a2
Reviewed-on: https://boringssl-review.googlesource.com/26374
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-03-30 19:54:10 +00:00
David Benjamin
2a19a17ca7 Limit ASN.1 constructed types recursive definition depth
Constructed types with a recursive definition could eventually exceed
the stack given malicious input with excessive recursion. Therefore we
limit the stack depth.

CVE-2018-0739

Credit to OSSFuzz for finding this issue.

(Imported from upstream's 9310d45087ae546e27e61ddf8f6367f29848220d.)

BoringSSL does not contain any such structures, but import this anyway
with a test.

Change-Id: I0e84578ea795134f25dae2ac8b565f3c26ef3204
Reviewed-on: https://boringssl-review.googlesource.com/26844
Commit-Queue: David Benjamin <davidben@google.com>
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>
2018-03-27 15:40:37 +00:00
David Benjamin
fa65113400 Push an error if custom private keys fail.
The private key callback may not push one of its own (it's possible to
register a custom error library and whatnot, but this is tedious). If
the callback does not push any, we report SSL_ERROR_SYSCALL. This is not
completely wrong, as "syscall" really means "I don't know, something you
gave me, probably the BIO, failed so I assume you know what happened",
but most callers just check errno. And indeed cert_cb pushes its own
error, so this probably should as well.

Update-Note: Custom private key callbacks which push an error code on
    failure will report both that error followed by
    SSL_R_PRIVATE_KEY_OPERATION_FAILED. Callbacks which did not push any
    error will switch from SSL_ERROR_SYSCALL to SSL_ERROR_SSL with
    SSL_R_PRIVATE_KEY_OPERATION_FAILED.

Change-Id: I7e90cd327fe0cbcff395470381a3591364a82c74
Reviewed-on: https://boringssl-review.googlesource.com/25544
Reviewed-by: Adam Langley <agl@google.com>
2018-02-01 21:43:42 +00:00
David Benjamin
0ab3f0ca25 Notice earlier if a server echoes the TLS 1.3 compatibility session ID.
Mono's legacy TLS 1.0 stack, as a server, does not implement any form of
resumption, but blindly echos the ClientHello session ID in the
ServerHello for no particularly good reason.

This is invalid, but due to quirks of how our client checked session ID
equality, we only noticed on the second connection, rather than the
first. Flaky failures do no one any good, so break deterministically on
the first connection, when we realize something strange is going on.

Bug: chromium:796910
Change-Id: I1f255e915fcdffeafb80be481f6c0acb3c628846
Reviewed-on: https://boringssl-review.googlesource.com/25424
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-01-26 21:53:27 +00:00
Nick Harper
36fcc4ca5d Implement Token Binding
Update-Note: Token Binding can no longer be configured with the custom
  extensions API. Instead, use the new built-in implementation. (The
  internal repository should be all set.)

Bug: 183

Change-Id: I007523a638dc99582ebd1d177c38619fa7e1ac38
Reviewed-on: https://boringssl-review.googlesource.com/20645
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>
2018-01-22 20:08:28 +00:00
David Benjamin
f88242d1c1 SSL_export_keying_material should work in half-RTT.
QUIC will need to derive keys at this point. This also smooths over a
part of the server 0-RTT abstraction. Like with False Start, the SSL
object is largely in a functional state at this point.

Bug: 221
Change-Id: I4207d8cb1273a1156e728a7bff3943cc2c69e288
Reviewed-on: https://boringssl-review.googlesource.com/24224
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>
2017-12-18 16:53:13 +00:00
David Benjamin
650d8c393e Implement TLS 1.3 early exporters.
Bug: 222
Change-Id: I33ee56358a62afcd9c3921026d55efcc543a5c11
Reviewed-on: https://boringssl-review.googlesource.com/23945
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-11 21:33:26 +00:00
David Benjamin
47b8f00fdc Reimplement OBJ_txt2obj and add a lower-level function.
OBJ_txt2obj is currently implemented using BIGNUMs which is absurd. It
also depends on the giant OID table, which is undesirable. Write a new
one and expose the low-level function so Chromium can use it without the
OID table.

Bug: chromium:706445
Change-Id: I61ff750a914194f8776cb8d81ba5d3eb5eaa3c3d
Reviewed-on: https://boringssl-review.googlesource.com/23364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-11-27 21:29:00 +00:00
David Benjamin
a838f9dc7e Make ECDSA signing 10% faster and plug some timing leaks.
None of the asymmetric crypto we inherented from OpenSSL is
constant-time because of BIGNUM. BIGNUM chops leading zeros off the
front of everything, so we end up leaking information about the first
word, in theory. BIGNUM functions additionally tend to take the full
range of inputs and then call into BN_nnmod at various points.

All our secret values should be acted on in constant-time, but k in
ECDSA is a particularly sensitive value. So, ecdsa_sign_setup, in an
attempt to mitigate the BIGNUM leaks, would add a couple copies of the
order.

This does not work at all. k is used to compute two values: k^-1 and kG.
The first operation when computing k^-1 is to call BN_nnmod if k is out
of range. The entry point to our tuned constant-time curve
implementations is to call BN_nnmod if the scalar has too many bits,
which this causes. The result is both corrections are immediately undone
but cause us to do more variable-time work in the meantime.

Replace all these computations around k with the word-based functions
added in the various preceding CLs. In doing so, replace the BN_mod_mul
calls (which internally call BN_nnmod) with Montgomery reduction. We can
avoid taking k^-1 out of Montgomery form, which combines nicely with
Brian Smith's trick in 3426d10119. Along
the way, we avoid some unnecessary mallocs.

BIGNUM still affects the private key itself, as well as the EC_POINTs.
But this should hopefully be much better now. Also it's 10% faster:

Before:
Did 15000 ECDSA P-224 signing operations in 1069117us (14030.3 ops/sec)
Did 18000 ECDSA P-256 signing operations in 1053908us (17079.3 ops/sec)
Did 1078 ECDSA P-384 signing operations in 1087853us (990.9 ops/sec)
Did 473 ECDSA P-521 signing operations in 1069835us (442.1 ops/sec)

After:
Did 16000 ECDSA P-224 signing operations in 1064799us (15026.3 ops/sec)
Did 19000 ECDSA P-256 signing operations in 1007839us (18852.2 ops/sec)
Did 1078 ECDSA P-384 signing operations in 1079413us (998.7 ops/sec)
Did 484 ECDSA P-521 signing operations in 1083616us (446.7 ops/sec)

Change-Id: I2a25e90fc99dac13c0616d0ea45e125a4bd8cca1
Reviewed-on: https://boringssl-review.googlesource.com/23075
Reviewed-by: Adam Langley <agl@google.com>
2017-11-22 22:51:40 +00:00
David Benjamin
e7c95d91f8 Run TLS 1.3 tests at all variants and fix bugs.
We were only running a random subset of TLS 1.3 tests with variants and
let a lot of bugs through as a result.

- HelloRetryRequest-EmptyCookie wasn't actually testing what we were
  trying to test.

- The second HelloRetryRequest detection needs tweaks in draft-22.

- The empty HelloRetryRequest logic can't be based on non-empty
  extensions in draft-22.

- We weren't sending ChangeCipherSpec correctly in HRR or testing it
  right.

- Rework how runner reads ChangeCipherSpec by setting a flag which
  affects the next readRecord. This cuts down a lot of cases and works
  correctly if the client didn't send early data. (In that case, we
  don't flush CCS until EndOfEarlyData and runner deadlocks waiting for
  the ChangeCipherSpec to arrive.)

Change-Id: I559c96ea3a8b350067e391941231713c6edb2f78
Reviewed-on: https://boringssl-review.googlesource.com/23125
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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>
2017-11-20 18:19:18 +00:00
David Benjamin
b25a8999be Add the ability to save and restore the error state.
This will be useful for the SSL stack to properly resurface handshake
failures. Leave this in a private header and, along the way, hide the
various types.

(ERR_NUM_ERRORS didn't change in meaning. The old documentation was
wrong.)

Bug: 206
Change-Id: I4c6ca98d162d11ad5e17e4baf439a18fbe371018
Reviewed-on: https://boringssl-review.googlesource.com/21284
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-10-09 21:43:13 +00:00
David Benjamin
a65c252f78 Further simplify error queue flags.
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>
2017-10-04 16:02:16 +00:00
David Benjamin
e1c3dad959 Error data is always a NUL-terminated malloced string.
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>
2017-10-02 21:24:08 +00:00
David Benjamin
808f832917 Run the comment converter on libcrypto.
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>
2017-08-18 21:49:04 +00:00
Steven Valdez
f4ecc84644 Prevent both early data and custom extensions from being accepted.
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>
2017-08-14 20:15:54 +00:00
David Benjamin
5aaaa98f8c Detect WatchGuard's TLS 1.3 interference failure mode.
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>
2017-06-22 19:49:23 +00:00
David Benjamin
b529253bea Implement scrypt from RFC 7914.
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>
2017-06-12 20:32:21 +00:00
Steven Valdez
2f3404bb81 Enforce incrementing counter for TLS 1.2 AES-GCM.
Change-Id: I7e790bc176369f2a57cc486c3dc960971faf019d
Reviewed-on: https://boringssl-review.googlesource.com/16625
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>
2017-05-26 20:06:36 +00:00
Steven Valdez
8ebc9eafec Update BN_enhanced_miller_rabin_primality_test to enforce preconditions and accept BN_prime_checks.
Change-Id: Ie4ac57d39bca46db33280c500a2092350ccdae67
Reviewed-on: https://boringssl-review.googlesource.com/15371
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>
2017-04-21 22:24:01 +00:00
David Benjamin
6fdea2aba9 Move PKCS#7 functions into their own directory.
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>
2017-04-19 17:24:51 +00:00
Alessandro Ghedini
de254b4c4e Enforce max_early_data_size on the server.
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>
2017-04-19 17:21:01 +00:00
Steven Valdez
b15143fece Fix check_fips for public keys and synchronize the EC and RSA versions.
Change-Id: Ibebf787445578608845df8861d67cd1e65ed0b35
Reviewed-on: https://boringssl-review.googlesource.com/15004
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>
2017-04-13 17:33:40 +00:00
Steven Valdez
d0b988219f Add RSA_check_fips to support public key validation checks.
Change-Id: I0e00f099a17d88f56b49970e612b0911afd9661e
Reviewed-on: https://boringssl-review.googlesource.com/14866
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>
2017-04-12 20:00:30 +00:00
David Benjamin
3cfeb9522b Disable SSLv3 by default.
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>
2017-04-11 16:38:16 +00:00
David Benjamin
d69d94e7e3 Teach crypto/x509 how to verify an Ed25519 signature.
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>
2017-04-05 23:35:30 +00:00
David Benjamin
417830d981 Support EVP_PKEY_{sign,verify}_message with Ed25519.
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>
2017-04-05 23:05:14 +00:00
David Benjamin
05bb1c5033 Implement draft-ietf-curdle-pkix-04's serialization.
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>
2017-04-05 23:02:22 +00:00
Steven Valdez
2a0707210a Prevent Channel ID and Custom Extensions on 0-RTT.
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>
2017-03-26 18:13:57 +00:00
David Benjamin
3cb047e56c Decouple PKCS#12 hash lookup from the OID table.
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>
2017-03-25 21:22:50 +00:00
Steven Valdez
2d85062c4f Add Data-less Zero-RTT support.
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>
2017-03-25 21:00:18 +00:00
David Benjamin
cfb9d147bb Update pkcs8 error data.
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>
2017-03-23 15:07:28 +00:00
David Benjamin
2d05568a7b Fix out-of-memory condition in conf.
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>
2017-03-21 16:19:22 +00:00
Adam Langley
4c341d0299 Support asynchronous ticket decryption with TLS 1.0–1.2.
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>
2017-03-11 00:04:18 +00:00
Matthew Braithwaite
6ad20dc912 Move error-on-empty-cipherlist into ssl_create_cipher_list().
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>
2017-03-09 17:31:45 +00:00