Commit Graph

2365 Commits

Author SHA1 Message Date
Watson Ladd
2f213f643f Update delegated credentials to draft-03
Change-Id: I0c648340ac7bb134fcda42c56a83f4815bbaa557
Reviewed-on: https://boringssl-review.googlesource.com/c/34884
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2019-02-13 20:04:33 +00:00
Adam Langley
2d38b83976 Remove separate default group list for servers.
It's the same as for clients, and we're probably not going to change
that any time soon.

Change-Id: Ic48cb640e98b0957d264267b97b5393f1977c6e6
Reviewed-on: https://boringssl-review.googlesource.com/c/34665
Reviewed-by: David Benjamin <davidben@google.com>
2019-02-06 00:33:29 +00:00
Adam Langley
fcc1ad78f9 Enable all curves (inc CECPQ2) during fuzzing.
Change-Id: I8083e841de135e9ec244609b1c20f0280ce20072
Reviewed-on: https://boringssl-review.googlesource.com/c/34664
Reviewed-by: David Benjamin <davidben@google.com>
2019-02-06 00:32:45 +00:00
David Benjamin
20a9b409bb runner: Don't generate an RSA key on startup.
RSA keygen isn't the fastest. Just use the existing one in
rsaCertificate.

Change-Id: Icd151232928e67e0a7d5becabf9dc96b0e9bfa22
Reviewed-on: https://boringssl-review.googlesource.com/c/34764
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2019-02-04 16:08:41 +00:00
Jesse Selover
d7266ecc9b Enforce key usage for RSA keys in TLS 1.2.
For now, this is off by default and controlled by SSL_set_enforce_rsa_key_usage.
This may be set as late as certificate verification so we may start by enforcing
it for known roots.

Generalizes ssl_cert_check_digital_signature_key_usage to check any part of the
key_usage, and adds a new error KEY_USAGE_BIT_INCORRECT for the generalized
method.

Bug: chromium:795089
Change-Id: Ifa504c321bec3263a4e74f2dc48513e3b895d3ee
Reviewed-on: https://boringssl-review.googlesource.com/c/34604
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2019-01-30 21:28:34 +00:00
Filippo Valsorda
73308b6606 Avoid SCT/OCSP extensions in SH on {Omit|Empty}Extensions
They were causing a "panic: ServerHello unexpectedly contained extensions"
if the client unconditionally signals support for OCSP or SCTs.

Change-Id: Ia60639431daf78679b269dfe337c1af171fd7d8b
Reviewed-on: https://boringssl-review.googlesource.com/c/34644
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2019-01-29 00:51:31 +00:00
Christopher Patton
6c1b376e1d Implement server support for delegated credentials.
This implements the server-side of delegated credentials, a proposed
extension for TLS:
https://tools.ietf.org/html/draft-ietf-tls-subcerts-02

Change-Id: I6a29cf1ead87b90aeca225335063aaf190a417ff
Reviewed-on: https://boringssl-review.googlesource.com/c/33666
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2019-01-24 20:06:58 +00:00
Adam Langley
9801a07145 Tweak some slightly fragile tests.
These tests failed when CECPQ2 was enabled by default. Even if we're
not going to make CECPQ2 the default, it's worth fixing them to be more
robust.

Change-Id: Idef508bca9e17a4ef0e0a8a396755abd975f9908
Reviewed-on: https://boringssl-review.googlesource.com/c/34524
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2019-01-23 22:48:16 +00:00
Adam Langley
4bfab5d9d7 Make 256-bit ciphers a preference for CECPQ2, not a requirement.
If 256-bit ciphers are a requirement for CECPQ2 then that introduces a
link between supported ciphers and supported groups: offering CECPQ2
without a 256-bit cipher is invalid. But that's a little weird since
these things were otherwise independent.

So, rather than require a 256-bit cipher for CECPQ2, just prefer them.

Change-Id: I491749e41708cd9c5eeed5b4ae23c11e5c0b9725
Reviewed-on: https://boringssl-review.googlesource.com/c/34504
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2019-01-23 22:38:56 +00:00
David Benjamin
fa81cc65dd Update comments around JDK11 workaround.
11.0.2 has since been released, but we are now aware of several more
bugs, so the workaround is unlikely to be removable for the foreseeable
future.

Change-Id: I8e7edcba2f002d0558a21e607306ddf9a205bfb3
Reviewed-on: https://boringssl-review.googlesource.com/c/34484
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2019-01-23 20:00:38 +00:00
David Benjamin
14c611cf91 Don't pass NULL,0 to qsort.
qsort shares the same C language bug as mem*. Two of our calls may see
zero-length lists. This trips UBSan.

Change-Id: Id292dd277129881001eb57b1b2db78438cf4642e
Reviewed-on: https://boringssl-review.googlesource.com/c/34447
Reviewed-by: Adam Langley <agl@google.com>
2019-01-22 23:28:38 +00:00
Adam Langley
823effe975 Revert "Fix protos_len size in SSL_set_alpn_protos and SSL_CTX_set_alpn_protos"
This reverts commit 35771ff8af. It breaks
tcnetty, which is tcnetty's fault but we have a large backlog from
Christmas to break with at the moment.

Bug: chromium:879657
Change-Id: Iafe93b335d88722170ec2689a25e145969e19e73
Reviewed-on: https://boringssl-review.googlesource.com/c/34324
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2019-01-16 20:02:16 +00:00
Alessandro Ghedini
3cbb0299a2 Allow configuring QUIC method per-connection
This allows sharing SSL_CTX between TCP and QUIC connections, such that
common settings can be configured without having to duplicate the
context.

Change-Id: Ie920e7f2a772dd6c6c7b63fdac243914ac5b7b26
Reviewed-on: https://boringssl-review.googlesource.com/c/33904
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2019-01-14 19:54:59 +00:00
Steven Valdez
b84674b2d2 Delete the variants/draft code.
Change-Id: I84abfedc30e4c34e42285f3c366c2f504a3b9cf2
Reviewed-on: https://boringssl-review.googlesource.com/c/34144
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2019-01-08 17:38:41 +00:00
Raul Tambre
35771ff8af Fix protos_len size in SSL_set_alpn_protos and SSL_CTX_set_alpn_protos
MakeConstSpan() takes size_t as the second argument, so protos_len ought to also be size_t.

Bug: chromium:879657
Change-Id: I93089ea20ce4b9c2b9d4d954dce807feb5341482
Reviewed-on: https://boringssl-review.googlesource.com/c/34164
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2019-01-07 18:14:42 +00:00
Christopher Patton
9cde848bd1 Use handshake parameters to decide if cert/key are available
Whether the host has a valid certificate or private key may depend on
the handshake parameters and not just its configuration. For example,
negotiating the delegated credential extension (see
https://tools.ietf.org/html/draft-ietf-tls-subcerts) requires an
alternate private key for the handshake.

Change-Id: I11cea1d11e731aa4018d980c010b8d8ebaa64c31
Reviewed-on: https://boringssl-review.googlesource.com/c/33664
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2019-01-04 19:29:33 +00:00
David Benjamin
17d553d299 Add a CFI tester to CHECK_ABI.
This uses the x86 trap flag and libunwind to test CFI works at each
instruction. For now, it just uses the system one out of pkg-config and
disables unwind tests if unavailable. We'll probably want to stick a
copy into //third_party and perhaps try the LLVM one later.

This tester caught two bugs in P-256 CFI annotations already:
I47b5f9798b3bcee1748e537b21c173d312a14b42 and
I9f576d868850312d6c14d1386f8fbfa85021b347

An earlier design used PTRACE_SINGLESTEP with libunwind's remote
unwinding features. ptrace is a mess around stop signals (see group-stop
discussion in ptrace(2)) and this is 10x faster, so I went with it. The
question of which is more future-proof is complex:

- There are two libunwinds with the same API,
  https://www.nongnu.org/libunwind/ and LLVM's. This currently uses the
  system nongnu.org for convenience. In future, LLVM's should be easier
  to bundle (less complex build) and appears to even support Windows,
  but I haven't tested this.  Moreover, setting the trap flag keeps the
  test single-process, which is less complex on Windows. That suggests
  the trap flag design and switching to LLVM later. However...

- Not all architectures have a trap flag settable by userspace. As far
  as I can tell, ARMv8's PSTATE.SS can only be set from the kernel. If
  we stick with nongnu.org libunwind, we can use PTRACE_SINGLESTEP and
  remote unwinding. Or we implement it for LLVM. Another thought is for
  the ptracer to bounce SIGTRAP back into the process, to share the
  local unwinding code.

- ARMv7 has no trap flag at all and PTRACE_SINGLESTEP fails. Debuggers
  single-step by injecting breakpoints instead. However, ARMv8's trap
  flag seems to work in both AArch32 and AArch64 modes, so we may be
  able to condition it on a 64-bit kernel.

Sadly, neither strategy works with Intel SDE. Adding flags to cpucap
vectors as we do with ARM would help, but it would not emulate CPUs
newer than the host CPU. For now, I've just had SDE tests disable these.

Annoyingly, CMake does not allow object libraries to have dependencies,
so make test_support a proper static library. Rename the target to
test_support_lib to avoid
https://gitlab.kitware.com/cmake/cmake/issues/17785

Update-Note: This adds a new optional test dependency, but it's disabled
by default (define BORINGSSL_HAVE_LIBUNWIND), so consumers do not need
to do anything. We'll probably want to adjust this in the future.

Bug: 181
Change-Id: I817263d7907aff0904a9cee83f8b26747262cc0c
Reviewed-on: https://boringssl-review.googlesource.com/c/33966
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2019-01-03 22:01:55 +00:00
Alessandro Ghedini
2cc6f449d7 Use same HKDF label as TLS 1.3 for QUIC as per draft-ietf-quic-tls-17
Change-Id: Ie9825634f0f290aa3af0e88477013f62e2e0c246
Reviewed-on: https://boringssl-review.googlesource.com/c/33724
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-12-19 20:25:34 +00:00
Adam Langley
ba9ad6628c Add |SSL_key_update|.
This function allows a client to send a TLS 1.3 KeyUpdate message.

Change-Id: I69935253795a79d65a8c85b652378bf04b7058e2
Reviewed-on: https://boringssl-review.googlesource.com/c/33706
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-12-19 20:15:24 +00:00
Adam Langley
9700b44ff5 HRSS: omit reconstruction of ciphertext.
In [1], section 5.1, an optimised re-encryption process is given. In the
code, this simplifies to not needing to rebuild the ciphertext at all.

Thanks to John Schanck for pointing this out.

[1] https://eprint.iacr.org/2018/1174.pdf

Change-Id: I807bd509e936b7e82a43e8656444431546e9bbdf
Reviewed-on: https://boringssl-review.googlesource.com/c/33705
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-12-19 20:09:34 +00:00
Adam Langley
a6a049a6fb Add start of infrastructure for checking constant-time properties.
Valgrind's checking of uninitialised memory behaves very much like a
check for constant-time code: branches and memory indexes based on
uninitialised memory trigger warnings. Therefore, if we can tell
Valgrind that some secret is “uninitialised”, it'll give us a warning if
we do something non-constant-time with it.

This was the idea behind https://github.com/agl/ctgrind. But tricks like
that are no longer needed because Valgrind now comes with support for
marking regions of memory as defined or not. Therefore we can use that
API to check constant-time code.

This CL defines |CONSTTIME_SECRET| and |CONSTTIME_DECLASSIFY|, which are
no-ops unless the code is built with
|BORINGSSL_CONSTANT_TIME_VALIDATION| defined, which it isn't by default.
So this CL is a no-op itself so far. But it does show that a couple of
bits of constant-time time are, in fact, constant-time—seemingly even
when compiled with optimisations, which is nice.

The annotations in the RSA code are a) probably not marking all the
secrets as secret, and b) triggers warnings that are a little
interesting:

The anti-glitch check calls |BN_mod_exp_mont| which checks that the
input is less than the modulus. Of course, it is because the input is
the RSA plaintext that we just decrypted, but the plaintext is supposed
to be secret and so branching based on its contents isn't allows by
Valgrind. The answer isn't totally clear, but I've run out of time on
this for now.

Change-Id: I1608ed0b22d201e97595fafe46127159e02d5b1b
Reviewed-on: https://boringssl-review.googlesource.com/c/33504
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2018-12-18 22:43:02 +00:00
David Benjamin
4cce955d14 Fix thread-safety bug in SSL_get_peer_cert_chain.
https://boringssl-review.googlesource.com/12704 pushed it just too far
to the edge. Once we have an established SSL_SESSION, any modifications
need to either be locked or done ahead of time. Do it ahead of time.
session->is_server gives a suitable place to check and X509s are
ref-counted so this should be cheap.

Add a regression test via TSan. Confirmed that TSan indeed catches this.

Change-Id: I30ce7b757d3a44465b318af3c98961ff3667483e
Reviewed-on: https://boringssl-review.googlesource.com/c/33606
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-12-13 19:30:49 +00:00
Adam Langley
200fe6786b Remove HRSS confirmation hash.
Since the underlying operation is deterministic the confirmation hash
isn't needed and SXY didn't use it in their proof.

Change-Id: I3a03c20ee79645cf94b10dbfe654c1b88d9aa416
Reviewed-on: https://boringssl-review.googlesource.com/c/33605
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2018-12-13 18:42:02 +00:00
Adam Langley
d6e1f230b3 Add |SSL_export_traffic_secrets|.
This allows an application to obtain the current TLS 1.3 traffic secrets
for a connection.

Change-Id: I8ad8d0559caba266f74081441dea54b22da3db20
Reviewed-on: https://boringssl-review.googlesource.com/c/33590
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-12-12 22:57:33 +00:00
David Benjamin
43cc9c6e86 Do not allow AES_128_GCM_SHA256 with CECPQ2.
Just forbid it altogether, so we don't need to worry about a mess of
equipreferences.

Change-Id: I4921ff326c6047e50c075d4311dd42219bf8318e
Reviewed-on: https://boringssl-review.googlesource.com/c/33585
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-12-12 20:05:52 +00:00
Adam Langley
7b935937b1 Add initial HRSS support.
This change includes support for a variant of [HRSS], a post-quantum KEM
based on NTRU. It includes changes suggested in [SXY]. This is not yet
ready for any deployment: some breaking changes, like removing the
confirmation hash, are still planned.

(CLA for HRSS's assembly code noted in b/119426559.)

[HRSS] https://eprint.iacr.org/2017/667.pdf
[SXY] https://eprint.iacr.org/2017/1005.pdf

Change-Id: I85d813733b066d5c578484bdd248de3f764194db
Reviewed-on: https://boringssl-review.googlesource.com/c/33105
Reviewed-by: David Benjamin <davidben@google.com>
2018-12-12 17:35:02 +00:00
David Benjamin
602f4669ab Forbid empty CertificateRequestsupported_signature_algorithms in TLS 1.2.
See the IETF thread here:
https://www.ietf.org/mail-archive/web/tls/current/msg27292.html

In particular, although the original publication of RFC 5246 had a
syntax error in the field (the minimum length was unspecified), there is
an errata from 2012 to fix it to be non-empty.
https://www.rfc-editor.org/errata/eid2864

Currently, when empty, we implicitly interpret it as SHA1/*, matching
the server behavior in missing extension in ClientHellos. However that
text does not support doing it for CertificateRequests, and there is not
much reason to. That default (which is in itself confusing and caused
problems such as older OpenSSL only signing SHA-1 given SNI) was
because, at the time, there were concerns over making any ClientHello
extensions mandatory. This isn't applicable for CertificateRequest,
which can freely advertise their true preferences.

Change-Id: I113494d8f66769fde1362795fb08ff2f471ef31d
Reviewed-on: https://boringssl-review.googlesource.com/c/33524
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-12-11 20:08:12 +00:00
Adam Langley
e6ad7a027f Drop some explicit SSLKeyShare destructors.
We zero out memory in |OPENSSL_free| already.

Change-Id: I84a0f3cdfadd4544c0fade1d3d727baa6496ffe5
Reviewed-on: https://boringssl-review.googlesource.com/c/33446
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-12-03 22:51:05 +00:00
David Benjamin
278b3120ee Validate ClientHellos in tests some more.
This way we'll notice if we ever generate a bad padding extension or
duplicate an extension. This did require fixing one of the JDK11 test
vectors. When I manually added a padding extension, I forgot the
contents were all zeros and incorrectly put in "padding" instead.

Change-Id: Ifec5bb01a739014ed0fdf5b49b82a6b514646e9a
Reviewed-on: https://boringssl-review.googlesource.com/c/33444
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-12-03 20:31:55 +00:00
David Benjamin
9113e0996f Satisfy golint.
Errors are supposed to be fragments that go into sentences, rather than
sentences themselves.

Change-Id: I6569fce25535475162c85e7b0db7eeb62c93febd
Reviewed-on: https://boringssl-review.googlesource.com/c/33324
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-11-21 23:31:08 +00:00
David Benjamin
6965d25602 Work around a JDK 11 TLS 1.3 bug.
JDK 11 shipped with a TLS 1.3 implementation enabled by default.
Unfortunately, that implementation does not work and fails to send the
SNI extension on resumption. See
https://bugs.openjdk.java.net/browse/JDK-8211806.

This means servers which enable TLS 1.3 will see JDK 11 clients work on
the first connection and then fail on all subsequent connections. Add
SSL_set_jdk11_workaround which configures a workaround to fingerprint
JDK 11 and disable TLS 1.3 with the faulty clients.

JDK 11 also implemented the downgrade signal, which means that
connections that trigger the workaround also must not send the downgrade
signal. Unfortunately, the downgrade signal's security properties are
sensitive to the existence of any unmarked TLS 1.2 ServerHello paths. To
salvage this, pick a new random downgrade marker for this scenario and
modify the client to treat it as an alias of the standard one.

Per the link above, JDK 11.0.2 will fix this bug. Hopefully the
workaround can be retired sometime after it is released.

Change-Id: I0627609a8cadf7cc214073eb7f1e880acdf613ef
Reviewed-on: https://boringssl-review.googlesource.com/c/33284
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-11-21 18:22:57 +00:00
Adam Langley
c65a1f4949 go fmt
Change-Id: I48a1e9e27013bb91b783949b65463208516bb3d2
Reviewed-on: https://boringssl-review.googlesource.com/c/33265
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-11-19 20:20:01 +00:00
David Benjamin
ce61710062 Move JSON test results code into a common module.
We can actually use modules now.

Change-Id: I0bd8abaf4e3318069f93fa17e89b4804d03944eb
Reviewed-on: https://boringssl-review.googlesource.com/c/33205
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-11-16 20:13:31 +00:00
Jesse Selover
f241a59dcc In 0RTT mode, reverify the server certificate before sending early data.
Bug: chromium:347402
Change-Id: I1442b595ed7296b9d9fe88357565f68e1ab80ffd
Reviewed-on: https://boringssl-review.googlesource.com/c/32644
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>
2018-11-16 19:52:18 +00:00
Steven Valdez
e6eef1ca16 Add post-handshake support for the QUIC API.
Change-Id: I4956efabfb33f7bd60a4743a922c29ee4de18935
Reviewed-on: https://boringssl-review.googlesource.com/c/33004
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-11-14 18:54:36 +00:00
David Benjamin
ce45588695 Speculatively remove __STDC_*_MACROS.
C99 added macros such as PRIu64 to inttypes.h, but it said to exclude them from
C++ unless __STDC_FORMAT_MACROS or __STDC_CONSTANT_MACROS was defined. This
text was never incorporated into any C++ standard and explicitly overruled in
C++11.

Some libc headers followed C99. Notably, glibc prior to 2.18
(https://sourceware.org/bugzilla/show_bug.cgi?id=15366) and old versions of the
Android NDK.

In the NDK, although it was fixed some time ago (API level 20), the NDK used to
use separate headers per API level. Only applications using minSdkVersion >= 20
would get the fix. Starting NDK r14, "unified" headers are available which,
among other things, make the fix available (opt-in) independent of
minSdkVersion. In r15, unified headers are opt-out, and in r16 they are
mandatory.

Try removing these and see if anyone notices. The former is past our five year
watermark. The latter is not and Android has hit
https://boringssl-review.googlesource.com/c/boringssl/+/32686 before, but
unless it is really widespread, it's probably simpler to ask consumers to
define __STDC_CONSTANT_MACROS and __STDC_FORMAT_MACROS globally.

Update-Note: If you see compile failures relating to PRIu64, UINT64_MAX, and
friends, update your glibc or NDK. As a short-term fix, add
__STDC_CONSTANT_MACROS and __STDC_FORMAT_MACROS to your build, but get in touch
so we have a sense of how widespread it is.

Bug: 198
Change-Id: I56cca5f9acdff803de1748254bc45096e4c959c2
Reviewed-on: https://boringssl-review.googlesource.com/c/33146
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-11-14 16:14:37 +00:00
David Benjamin
7d10ab594c Abstract hs_buf a little.
Having to lazily create it is a little wordy, and we append to it in
three places now. V2ClientHello makes this slightly finicky, but I think
this is still clearer.

Change-Id: If931db0b56efd7f0728c0b7d119886864dd7933a
Reviewed-on: https://boringssl-review.googlesource.com/c/32824
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-11-09 19:01:04 +00:00
Steven Valdez
384d0eaf19 Make SSL_get_current_cipher valid during QUIC callbacks.
Update-Note: This effectively reverts https://boringssl-review.googlesource.com/4733,
which was an attempt at a well-defined story during renegotiation and pre-handshake.
This is a behavior change, though one that matches OpenSSL upstream. It is also more
consistent with other functions, such as SSL_get_curve_id. Renegotiation is now
opt-in, so this is less critical, and, if we change the behavior mid-renegotiation,
we should do it consistently to all getters.

Change-Id: Ica6b386fb7c5ac524395de6650642edd27cac36f
Reviewed-on: https://boringssl-review.googlesource.com/c/32904
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>
2018-11-06 19:04:48 +00:00
Matthew Braithwaite
c65eb2ceda Serialize SSL curve list in handoff and check it on application.
A split SSL handshake may involve 2 binaries, potentially built at
different versions: call them the "handoff/handback" binary and the
"handshake" binary.  We would like to guarantee that the
handoff/handback binary does not make any promises that the handshake
binary cannot keep.

d2ed382 serialized |kCiphers|; this commit extends the same approach
to |kNamedGroups|.

Change-Id: Idb13e54e9b189236309f6054a36872c5a4d96985
Reviewed-on: https://boringssl-review.googlesource.com/c/32825
Reviewed-by: David Benjamin <davidben@google.com>
2018-11-06 01:19:10 +00:00
Matthew Braithwaite
d2ed382e64 Serialize SSL configuration in handoff and check it on application.
A split SSL handshake may involve 2 binaries, potentially built at
different versions: call them the "handoff/handback" binary and the
"handshake" binary.  We would like to guarantee that the
handoff/handback binary does not make any promises that the handshake
binary cannot keep.

As a start, this commit serializes |kCiphers| to the handoff message.
When the handoff message is applied to an |SSL|, any configured
ciphers not listed in the handoff message will be removed, in order to
prevent them from being negotiated.

Subsequent commits will apply the same approach to other lists of features.

Change-Id: Idf6dbeadb750c076ab0509c09b9d3f22eb162b9c
Reviewed-on: https://boringssl-review.googlesource.com/c/29264
Reviewed-by: Matt Braithwaite <mab@google.com>
2018-11-02 19:45:42 +00:00
David Benjamin
cc9d935256 Buffer up QUIC data within a level internally.
Avoid forcing the QUIC implementation to buffer this when we already have code
to do it. This also avoids QUIC implementations relying on this hook being
called for each individual message.

Change-Id: If2d70f045a25da1aa2b10fdae262cae331da06b1
Reviewed-on: https://boringssl-review.googlesource.com/c/32785
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-11-01 13:52:43 +00:00
Steven Valdez
c8e0f90f83 Add an interface for QUIC integration.
0-RTT support and APIs to consume NewSessionTicket will be added in a
follow-up.

Change-Id: Ib2b2c6b618b3e33a74355fb53fdbd2ffafcc5c56
Reviewed-on: https://boringssl-review.googlesource.com/c/31744
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>
Reviewed-by: David Benjamin <davidben@google.com>
2018-10-31 20:38:10 +00:00
Jeremy Apthorp
c0c9001440 Implement SSL_get_tlsext_status_type
It's used by Node.js[1], and is simple to implement.

[1]: e2f58c71dd/src/node_crypto.cc (L2390)

Change-Id: Ie5c76b848623d00f7478aeae0214c25472de523c
Reviewed-on: https://boringssl-review.googlesource.com/c/32525
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>
2018-10-19 00:30:32 +00:00
David Benjamin
1eff9482ca Use proper functions for lh_*.
As with sk_*, this. This doesn't fix the function pointer casts. Those
will be done in a follow-up change. Also add a test for lh_*_doall so we
cover both function pointer shapes.

Update-Note: This reworks how LHASH_OF(T) is implemented and also only
pulls in the definitions where used, but LHASH_OF(T) is never used
externally, so I wouldn't expect this to affect things.

Change-Id: I7970ce8c41b8589d6672b71dd03658d0e3bd89a7
Reviewed-on: https://boringssl-review.googlesource.com/c/32119
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-10-15 23:37:04 +00:00
David Benjamin
2d98d49cf7 Add a per-SSL TLS 1.3 downgrade enforcement option and improve tests.
Due to non-compliant middleboxes, it is possible we'll need to do some
surgery to this mechanism. Making it per-SSL is a little more flexible
and also eases some tests in Chromium until we get its SSL_CTX usage
fixed up.

Also fix up BoringSSL tests. We forgot to test it at TLS 1.0 and use the
-expect-tls13-downgrade flag.

Bug: 226
Change-Id: Ib39227e74e2d6f5e1fbc1ebcc091e751471b3cdc
Reviewed-on: https://boringssl-review.googlesource.com/c/32424
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>
2018-10-10 19:50:19 +00:00
David Benjamin
419144adce Fix undefined function pointer casts in {d2i,i2d}_Foo_{bio,fp}
Lacking C++, this instead adds a mess of macros. With this done, all the
function-pointer-munging "_of" macros in asn1.h can also be removed.

Update-Note: A number of *really* old and unused ASN.1 macros were
removed.

Bug: chromium:785442
Change-Id: Iab260d114c7d8cdf0429759e714d91ce3f3c04b2
Reviewed-on: https://boringssl-review.googlesource.com/32106
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-10-01 17:34:53 +00:00
David Benjamin
5b33effa72 Rename OPENSSL_NO_THREADS, part 1.
BoringSSL depends on the platform's locking APIs to make internal global
state thread-safe, including the PRNG. On some single-threaded embedded
platforms, locking APIs may not exist, so this dependency may be disabled
with a build flag.

Doing so means the consumer promises the library will never be used in any
multi-threaded address space. It causes BoringSSL to be globally thread-unsafe.
Setting it inappropriately will subtly and unpredictably corrupt memory and
leak secret keys.

Unfortunately, folks sometimes misinterpreted OPENSSL_NO_THREADS as skipping an
internal thread pool or disabling an optionally extra-thread-safe mode. This is
not and has never been the case. Rename it to
OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED to clarify what
this option does.

Update-Note: As a first step, this CL makes both OPENSSL_NO_THREADS and
OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED work. A later CL
will remove the old name, so migrate callers after or at the same time as
picking up this CL.

Change-Id: Ibe4964ae43eb7a52f08fd966fccb330c0cc11a8c
Reviewed-on: https://boringssl-review.googlesource.com/32084
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-09-26 19:10:02 +00:00
David Benjamin
ca4971cbae Sync bundled bits of golang.org/x/crypto.
We no longer need to fork them. This is in preparation for pulling it
via Go modules, but probably need to figure out the network issue first.
Slightly bad manners for CI to do that.

Change-Id: Ic258264f3c3559817d5e4921e4ad3282e94d05fe
Reviewed-on: https://boringssl-review.googlesource.com/31904
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-09-17 23:14:35 +00:00
David Benjamin
0990a552eb Set up Go modules.
This should make it easier for us to reuse Go code properly.
util/fipstools is kind of a mess. runner has been using relative
imports, but Go seems to prefer this mechanism these days.

Update-Note: The import spelling in ssl/test/runner changes. Also we now
    require Go 1.11. Or you could clone us into GOPATH, but no one does
    that.

Change-Id: I8bf91e1e0345b3d0b3d17f5c642fe78b415b7dde
Reviewed-on: https://boringssl-review.googlesource.com/31884
Reviewed-by: Adam Langley <agl@google.com>
2018-09-17 21:04:17 +00:00
David Benjamin
d1673c2191 Remove the add_alert hook.
This was added to support the no_certificate warning alert in SSLv3. That has
since been removed. In the long run, I would like for ssl_send_alert to go
through a flow similar to add_alert so the BIO-free APIs work right and avoid a
host of strangeness surrounding wpend_buf. For now, remove the unused hook.

Change-Id: I1995028b8af4ffa836028794e6b33b2cd1b2435b
Reviewed-on: https://boringssl-review.googlesource.com/31984
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-09-15 00:55:02 +00:00