The value returned by |SSL_get_servername| is owned by the |SSL*|, which
might be surprising if someone stashes it away and expects to be able to
use it later.
Change-Id: I7b61d1dd0d3d0bf035bbcc9ffdbea10c33296f59
Reviewed-on: https://boringssl-review.googlesource.com/18444
Reviewed-by: David Benjamin <davidben@google.com>
X.509 functions and the like should not vary their behaviour based on
the configured locale, but tolower(3), strcasecmp(3) and strncasecmp(3)
change behaviour based on that.
For example, with tr_TR.utf8, 'I' is not the upper-case version of 'i'.
Change-Id: I896a285767ae0c22e6ce06b9908331c625e90af2
Reviewed-on: https://boringssl-review.googlesource.com/18412
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 TLS standard suggests[1] that no_renegotation should be a warning alert
and that a client be able to decide whether to continue. This change
documents in PORTING.md that BoringSSL responds with a fatal alert
instead.
This is because we do not want to have any messages that are absorbed
without limit in the TLS layer because they may bypass limits
implemented at a higher level. We could limit the number of ClientHello
messages in the same way that we limit empty records, but we have had
this fatal behaviour for a long time without issue and it's simple.
(Technically this violates the RFC because the RFC says that
no_renegotation is always a warning.)
[1] https://tools.ietf.org/html/rfc5246#section-7.2.2
Change-Id: I4d4a696114f7e2b85f39e3fcb7b2c914cef661f2
Reviewed-on: https://boringssl-review.googlesource.com/18409
Reviewed-by: David Benjamin <davidben@google.com>
OpenSSL allows spaces, commas and semi-colons to be used as separators
in cipher strings, in addition to the usual colons.
This change documents that spaces cannot be used in equal-preference
groups and forbids these alternative separators in strict mode.
Change-Id: I3879e25aed54539c281511627e6a282e9463bdc3
Reviewed-on: https://boringssl-review.googlesource.com/18424
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
A follow-up change will tweak linux_shared to run this tool on
libcrypto.so and libssl.so.
Bug: 132
Change-Id: I868551cebdc308829dee3dca12a39395c4a251ee
Reviewed-on: https://boringssl-review.googlesource.com/18407
Reviewed-by: Steven Valdez <svaldez@google.com>
This allows us to avoid omitting all the silly abort() flags in
reasonable downstreams like Chromium, while the holdouts are fixed. It
also means that we still get the compiler checking that we've
implemented all pure virtuals in some build configurations, which we'll
put on a bot somewhere.
Bug: 132
Change-Id: If500749f7100bb22bb8e828e8ecf38a992ae9fe5
Reviewed-on: https://boringssl-review.googlesource.com/18406
Reviewed-by: Steven Valdez <svaldez@google.com>
SSL_get0_peer_certificates is documented to return NULL if the peer was
anonymous, but it actually returns a non-NULL empty list (except in SSL
3.0 where the Certificate message and thus ssl_parse_cert_chain is
skipped).
Make the implementation match the documentation.
Change-Id: Ib3e25d2155f316cc5e9eb3ab7f74b78e08b8a86b
Reviewed-on: https://boringssl-review.googlesource.com/18226
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>
Looks like they're using the pool now.
Change-Id: Ieeb1cacb9cb039d35ff091bc9742262f0fc5b146
Reviewed-on: https://boringssl-review.googlesource.com/18364
Reviewed-by: Adam Langley <agl@google.com>
This is a utility to switch comments from /* C-style */ to // C++-style.
It's purely aesthetic, but it matches how most of Google C++ looks.
Running it over libssl, the script seems to get all but one or two cases
right.
We may also wish to convert the C code for consistency while we're here.
We've accidentally put both styles of comments all over the place, so
our toolchains can tolerate // in C.
Bug: 132
Change-Id: If2f4d58c0a4ad8f9a2113705435bff90e0dabcc3
Reviewed-on: https://boringssl-review.googlesource.com/18064
Reviewed-by: David Benjamin <davidben@google.com>
This is a C++ interface for encrypting and decrypting TLS application
data records in-place, wrapping the existing C API in tls_record.cc.
Also add bssl::Span, a non-owning reference to a contiguous array of
elements which can be used as a common interface over contiguous
container types (like std::vector), pointer-length-pairs, arrays, etc.
Change-Id: Iaa2ca4957cde511cb734b997db38f54e103b0d92
Reviewed-on: https://boringssl-review.googlesource.com/18104
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Short-term, we will need to use these macros and build without RTTI when
defining any virtual base class. Long-term, it would be good to remove
these constraints, but it will require some downstream work.
Bug: 132
Change-Id: I3bc65bb12d7653978612b7d1bf06f772a2f3b1cd
Reviewed-on: https://boringssl-review.googlesource.com/18344
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
SSLECDHContext has the acronyms problem, so I went with SSLKeyShare to
match the TLS 1.3 terminology. It's also a little shorter. Accept and
Finish, for now, take raw output pointers in anticipation of some
bssl::Array and maybe bssl::CleansedArray types.
Bug: 132
Change-Id: I427c7c0eac95704f3ad093676c504c2848f5acb9
Reviewed-on: https://boringssl-review.googlesource.com/18265
Reviewed-by: Steven Valdez <svaldez@google.com>
This should make it a little easier to write C++-only public headers.
Change-Id: Ie5bff241c810cb5330f66d8a4dc1dd8b2d69c7c9
Reviewed-on: https://boringssl-review.googlesource.com/18225
Reviewed-by: David Benjamin <davidben@google.com>
Originally GREASE was a client-only thing but, in TLS 1.3, we send some
bogus extensions in NewSessionTicket and CertificateRequest. Sampling
from the client_random works fine, but better to use our own entropy
rather than the peer's.
Change-Id: Ic7317eb75a9024c677fcde8e62c73aff380294e4
Reviewed-on: https://boringssl-review.googlesource.com/18144
Reviewed-by: Steven Valdez <svaldez@google.com>
I started by switching a couple fields to SSL_HANDSHAKE and then kept
following transitive bits.
Bug: 132
Change-Id: I640dadd3558615fa38c7e8498d4efe7449b0658f
Reviewed-on: https://boringssl-review.googlesource.com/18245
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
SSL_HANDSHAKE is large so I have not attempted to fully switch it to
scopers in this CL. This is just a preparatory step so that we can start
switching its fields to scopers.
(I also anticipate we'll want a bssl::Array<uint8_t> to replace the
pointer/length pairs.)
Bug: 132
Change-Id: I1538d3fc7f9c7385cd8c44a7b99b5c76e8a8768c
Reviewed-on: https://boringssl-review.googlesource.com/18244
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
The previous attempt around the 'struct ssl_st' compatibility mess
offended OSS-Fuzz and UBSan because one compilation unit passed a
function pointer with ssl_st* and another called it with
bssl::SSLConnection*.
Linkers don't retain such types, of course, but to silence this alert,
instead make C-visible types be separate from the implementation and
subclass the public type. This does mean we risk polluting the symbol
namespace, but hopefully the compiler is smart enough to inline the
visible struct's constructor and destructor.
Bug: 132
Change-Id: Ia75a89b3a22a202883ad671a630b72d0aeef680e
Reviewed-on: https://boringssl-review.googlesource.com/18224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Clear out some of the easy cases.
Bug: 132
Change-Id: Icd5c246cb6bec4a96c72eccd6569235c3d030ebd
Reviewed-on: https://boringssl-review.googlesource.com/18204
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>
At this point, the security policy document will be maintained in the
BoringSSL repo for change control.
Change-Id: I9ece51a0e9a506267e2f3b5215fb0d516d0d834b
Reviewed-on: https://boringssl-review.googlesource.com/18184
Reviewed-by: David Benjamin <davidben@google.com>
This adds several utilities as replacements for new and delete and makes
bssl::UniquePtr work with our private types.
Later work can convert more incrementally. I did this one more
aggressively to see how it'd work. Unfortunately, in doing so, I needed
to remove the NULL SSL_AEAD_CTX "method" receiver trick to appease
clang. The null cipher is now represented by a concrete SSL_AEAD_CTX.
The long-lived references to SSL_AEAD_CTX are not yet in types with
constructors, so they still bare Delete rather than UniquePtr for now.
Though this does mean we may be able to move the sequence number into
SSLAEADContext later which is one less object for DTLS to carry around.
Bug: 132
Change-Id: I506b404addafb692055d5709b0ca6d5439a4e6be
Reviewed-on: https://boringssl-review.googlesource.com/18164
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This is horrible, but everything else I tried was worse. The goal with
this CL is to take the extern "C" out of ssl/internal.h and move most
symbols to namespace bssl, so we can start using C++ helpers and
destructors without worry.
Complications:
- Public API functions must be extern "C" and match their declaration in
ssl.h, which is unnamespaced. C++ really does not want you to
interleave namespaced and unnamespaced things. One can actually write
a namespaced extern "C" function, but this means, from C++'s
perspective, the function is namespaced. Trying to namespace the
public header would worked but ended up too deep a rabbithole.
- Our STACK_OF macros do not work right in namespaces.
- The typedefs for our exposed but opaque types are visible in the
header files and copied into consuming projects as forward
declarations. We ultimately want to give SSL a destructor, but
clobbering an unnamespaced ssl_st::~ssl_st seems bad manners.
- MSVC complains about ambiguous names if one typedefs SSL to bssl::SSL.
This CL opts for:
- ssl/*.cc must begin with #define BORINGSSL_INTERNAL_CXX_TYPES. This
informs the public headers to create forward declarations which are
compatible with our namespaces.
- For now, C++-defined type FOO ends up at bssl::FOO with a typedef
outside. Later I imagine we'll rename many of them.
- Internal functions get namespace bssl, so we stop worrying about
stomping the tls1_prf symbol. Exported C functions are stuck as they
are. Rather than try anything weird, bite the bullet and reorder files
which have a mix of public and private functions. I expect that over
time, the public functions will become fairly small as we move logic
to more idiomatic C++.
Files without any public C functions can just be written normally.
- To avoid MSVC troubles, some bssl types are renamed to CPlusPlusStyle
in advance of them being made idiomatic C++.
Bug: 132
Change-Id: Ic931895e117c38b14ff8d6e5a273e868796c7581
Reviewed-on: https://boringssl-review.googlesource.com/18124
Reviewed-by: David Benjamin <davidben@google.com>
This serves two purposes:
1. Make sure we don't accidentally write "throw" or "catch" in our code.
2. If a C project statically links to our libssl.a, they'll use gcc
rather than g++ as the linker which does not pull in the C++ runtime.
-fno-exceptions cuts out the C++ runtime dependency. (Though we will
need to give up on this should we attempt to allow a runtime
dependency in the future.)
Bug: 132
Change-Id: I9fc81f034833ec9ed38b98d98df7b45c32cfa30c
Reviewed-on: https://boringssl-review.googlesource.com/18084
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>
The former is defined by the kernel and is a straightforward number. The
latter is defined by glibc as:
#define SYS_getrandom __NR_getrandom
which does not work when kernel headers are older than glibc headers.
Instead, use the kernel values.
Bug: chromium:742260
Change-Id: Id162f125db660643269e0b1329633437048575c4
Reviewed-on: https://boringssl-review.googlesource.com/17864
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 is needed to switch Chromium's SSLServerSocket and parts of
Conscrypt to CRYPTO_BUFFER.
Bug: 54
Change-Id: Iacd417970607bc1a162057676b576956a3bdfa3f
Reviewed-on: https://boringssl-review.googlesource.com/17965
Reviewed-by: Adam Langley <agl@google.com>
This also serves as a certificate verification callback for
CRYPTO_BUFFER-based consumers. Remove the silly
SSL_CTX_i_promise_to_verify_certs_after_the_handshake placeholder.
Bug: 54, chromium:347402
Change-Id: I4c6b445cb9cd7204218acb2e5d1625e6f37aff6f
Reviewed-on: https://boringssl-review.googlesource.com/17964
Reviewed-by: David Benjamin <davidben@google.com>
This would also have fixed the Windows clang issues. Those kicked in
because Windows clang defines __clang__ and not __GNUC__, but
OPENSSL_UNUSED accounts for this. It's also shorter.
Change-Id: I75bc17bbb789c5b78a7a369c43194e146739f574
Reviewed-on: https://boringssl-review.googlesource.com/18004
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This will catch failures standalone before they reach consumers.
Change-Id: I1957df92e02ef7b4c609cd85420535c94ae3a04c
Reviewed-on: https://boringssl-review.googlesource.com/18005
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>
In some configurations, Clang will warn about all unannotated
fall-throughs in C++. This change adds the needed annotation for Clang
in the single place where we appear to have this.
Change-Id: I25a9069e659ce278d3cd24bf46f667324b3d5146
Reviewed-on: https://boringssl-review.googlesource.com/18024
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Record splitting is a send-side only behaviour and supporting it in
fuzzer mode was messy.
Change-Id: I406d2cc77f1d83ed2039a85b95acdfbc815f5a44
Reviewed-on: https://boringssl-review.googlesource.com/17944
Reviewed-by: Adam Langley <agl@google.com>
Clang for Windows does not like OPENSSL_COMPILE_ASSERT inside a function
in C++. It complains that the struct is unused. I think we worked around
this in C previously by making it expand to C11 _Static_assert when
available.
But libssl is now C++ and assumes a C++11-capable compiler. Use real
static_assert.
Bug: 132
Change-Id: I6aceb95360244bd2c80d194b80676483abb60519
Reviewed-on: https://boringssl-review.googlesource.com/17924
Reviewed-by: Adam Langley <agl@google.com>
This plumbs EVP_AEAD_CTX_seal_scatter all the way through to
tls_record.c, so we can add a new zero-copy record sealing method on top
of the existing code.
Change-Id: I01fdd88abef5442dc16605ea31b29b4b1231c073
Reviewed-on: https://boringssl-review.googlesource.com/17684
Reviewed-by: Adam Langley <agl@google.com>
Due to SSL 3.0 legacy, TLS 1.0 through 1.2 allow ClientHello and
ServerHello messages to omit the extensions field altogether, rather
than write an empty field. We broke this in
https://boringssl-review.googlesource.com/c/17704/ when we needed to a
second ServerHello parsing path.
Fix this and add some regression tests to explicitly test both the
omitted and empty extensions ClientHello and ServerHello cases.
Bug: chromium:743218
Change-Id: I8297ba608570238e19f12ea44a9fe2fe9d881d28
Reviewed-on: https://boringssl-review.googlesource.com/17904
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This change updates the ChaCha20-Poly1305 AEAD to be able to process
|extra_in| data. It does this by encrypting the extra data byte-by-byte
(because extra data should be very small). Both the generic and assembly
code is updated to be able to include this extra ciphertext in the
Poly1305 calculation.
Change-Id: I751ed31fb7e1f4db6974e9ed31721a43177cf8cb
Reviewed-on: https://boringssl-review.googlesource.com/17465
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This is in an attempt to debug the Mac flakiness. The timestamps will
hopefully help narrow down the order of operations here.
Bug: 199
Change-Id: I8b8dd7222e3a57a8b055b8bc1b7731334e0fcdf0
Reviewed-on: https://boringssl-review.googlesource.com/17886
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>
This was broken when we added the API to SSL.
Change-Id: I92d4330b0d70f655c9a9ad33898d6b84704e915c
Reviewed-on: https://boringssl-review.googlesource.com/17884
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
There's not much point in enforcing this on these files, and clang
complains about LLVMFuzzerTestOneInput.
Change-Id: Ieae3287d7f7cd9736efd2b9fc11d0e6d89fcfa43
Reviewed-on: https://boringssl-review.googlesource.com/17806
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
This implements PR #1051
(https://github.com/tlswg/tls13-spec/pull/1051).
Local experiments were not able to replicate the claims in the PR, but
implement this anyway for comparison purposes.
Change-Id: Ic9baf5e671f9a44565020466a553dd08f5ec0f1b
Reviewed-on: https://boringssl-review.googlesource.com/17844
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>
Change-Id: Iefe4790094e2b2c6f9ef0833c00ab37707b7b19a
Reviewed-on: https://boringssl-review.googlesource.com/17805
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
I suspect this won't actually tell us much useful w.r.t. the Mac test
flakes, but we may as well print what we can get.
Change-Id: I4931f6000648c4bd955a132b54351ff83d6b6273
Reviewed-on: https://boringssl-review.googlesource.com/17804
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>
