reuse_message and V2ClientHellos each caused messages to be
double-reported.
Change-Id: I8722a3761ede272408ac9cf8e1b2ce383911cc6f
Reviewed-on: https://boringssl-review.googlesource.com/18764
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>
The following code:
int closed; /* valid iff peer != NULL */
size_t len; /* valid iff buf != NULL; 0 if peer == NULL */
size_t offset; /* valid iff buf != NULL; 0 if len == 0 */
should be rewritten as:
int closed; // valid iff peer != NULL
size_t len; // valid iff buf != NULL; 0 if peer == NULL
size_t offset; // valid iff buf != NULL; 0 if len == 0
But the existing code lost the alignment when shifting the third comment
over to follow the two-space rule. Also warn about > 80 character lines
so they may be manually fixed up.
Change-Id: Idd3b4267b972c9b8891ceefd50f6d2a0e67ed51c
Reviewed-on: https://boringssl-review.googlesource.com/18784
Reviewed-by: Adam Langley <agl@google.com>
There was a typo (then => the), but I think this is clearer, albeit
longer.
Change-Id: Ic95368a1bea1feba9d6a00029bbfb5b8ffd260ec
Reviewed-on: https://boringssl-review.googlesource.com/18747
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 memcpy of a pointer looks like a typo, though it isn't. Instead,
transcribe what the functions expect into a union and let C fill it in.
Change-Id: Iba4c824295e8908c5bda68ac35673040a8cff116
Reviewed-on: https://boringssl-review.googlesource.com/18744
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>
This would only come up if the peer didn't pack records together, but
it's free to handle. Notably OpenSSL has a bug where it does not pack
retransmits together.
Change-Id: I0927d768f6b50c62bacdd82bd1c95396ed503cf3
Reviewed-on: https://boringssl-review.googlesource.com/18724
Reviewed-by: David Benjamin <davidben@google.com>
There are still a ton of them, almost exclusively complaints that
function declaration and definitions have different parameter names. I
just fixed a few randomly.
Change-Id: I1072f3dba8f63372cda92425aa94f4aa9e3911fa
Reviewed-on: https://boringssl-review.googlesource.com/18706
Reviewed-by: Steven Valdez <svaldez@google.com>
The following code was misconverted:
BIO *peer; /* NULL if buf == NULL.
* If peer != NULL, then peer->ptr is also a bio_bio_st,
* and its "peer" member points back to us.
* peer != NULL iff init != 0 in the BIO. */
Per the criteria in the comment, this comment is eligible, which is what
we want. Only continuation lines must be prefixed by spaces. But the
loop treated the first line as immediately ineligible. Moreover, in that
case, it dropped the line on the floor rather than echoing it. Fix this
by dropping that case.
Change-Id: Ic523fe1e6bc8dde37a9897e2a93e815c11feb95a
Reviewed-on: https://boringssl-review.googlesource.com/18746
Reviewed-by: Adam Langley <agl@google.com>
Code like this:
if (// Check if the cipher is supported for the current version.
SSL_CIPHER_get_min_version(c) <= ssl3_protocol_version(ssl) &&
ssl3_protocol_version(ssl) <= SSL_CIPHER_get_max_version(c) &&
// Check the cipher is supported for the server configuration.
(c->algorithm_mkey & mask_k) &&
(c->algorithm_auth & mask_a) &&
// Check the cipher is in the |allow| list.
sk_SSL_CIPHER_find(allow, &cipher_index, c)) {
should not get an extra space.
Change-Id: I772cbcfabf2481dc8e3a8b257d85573b0b5ac1b7
Reviewed-on: https://boringssl-review.googlesource.com/18745
Reviewed-by: David Benjamin <davidben@google.com>
Bug: 132
Change-Id: I75d6ce5a2256a4b464ca6a9378ac6b63a9bd47e2
Reviewed-on: https://boringssl-review.googlesource.com/18644
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>
placement new requires operator new (size_t, void*) to be defined, which
requires pulling in the <new> header.
Change-Id: Ibaa8f3309b03129958f201d32de8afcfafed70f6
Reviewed-on: https://boringssl-review.googlesource.com/18664
Reviewed-by: David Benjamin <davidben@google.com>
I'm not sure why these aren't on by default, but Chromium does this too.
Colors are nice.
Change-Id: I7d7bf006014e9f40ec2f48290ad8fe7a70c1cfce
Reviewed-on: https://boringssl-review.googlesource.com/18704
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The first line of bssl::New is invalid in LLVM CFI as we are casting a
pointer to T before the object is constructed. Instead, we should leave
it as void* and only use it as a T* afterward being constructed.
Bug: chromium:750445
Change-Id: I0ae60c2a7e541b45bc0155dd8f359b662f561dcc
Reviewed-on: https://boringssl-review.googlesource.com/18684
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>
Change-Id: I84b9a7606aaf28e582c79ada47df95b46ff2c2c2
Reviewed-on: https://boringssl-review.googlesource.com/18624
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>
Similarly, add EVP_AEAD_CTX_tag_len which computes the exact tag length
for required by EVP_AEAD_CTX_seal_scatter.
Change-Id: I069b0ad16fab314fd42f6048a3c1dc45e8376f7f
Reviewed-on: https://boringssl-review.googlesource.com/18324
Reviewed-by: Adam Langley <agl@google.com>
I'm not that fast when debugging.
Change-Id: I37a120a77e9a35ac5255ad760513b983f83d9bd7
Reviewed-on: https://boringssl-review.googlesource.com/18605
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>
Pushing entries onto a stack when handling malloc failures is a
nuisance. sk_push only takes ownership on success. PushToStack smooths
that over with a UniquePtr.
Bug: 132
Change-Id: I4f0a9eee86dda7453f128c33d3a71b550beb25e9
Reviewed-on: https://boringssl-review.googlesource.com/18468
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Apparently C does not promise this, only that casting zero to a pointer
gives NULL. No compiler will be insane enough to violate this, but it's
an easy assumption to document.
Change-Id: Ie255d42af655a4be07bcaf48ca90584a85c6aefd
Reviewed-on: https://boringssl-review.googlesource.com/18584
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The s390x patches keep on coming.
Change-Id: I6d7f79e5ee7c8fcfe6b2e8e549b18ee686b4392b
Reviewed-on: https://boringssl-review.googlesource.com/18564
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 kind of a mess. Some projects will wrap our public headers in
extern "C", so we use extern "C++" around our C++ APIs. However this
needs to be done when including C++ standard library headers too since
they don't always, themselves, guard against being wrapped in extern
"C".
Change-Id: Ib7dd4a6f69ca81dd525ecaa1418b3b7ba85b6579
Reviewed-on: https://boringssl-review.googlesource.com/18504
Reviewed-by: Adam Langley <agl@google.com>
My original plan here was to make STACK_OF(T) expand to a template so
the inner type were extractable. Unfortunately, we cannot sanely make
STACK_OF(T) expand to a different type in C and C++ even across
compilation units because UBSan sometimes explodes. This is nuts, but so
it goes.
Instead, use StackTraits to extract the STACK_OF(T) parameters and
define an iterator type.
Bug: 189
Change-Id: I64f5173b34b723ec471f7a355ff46b04f161386a
Reviewed-on: https://boringssl-review.googlesource.com/18467
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The changes to the assembly files are synced from upstream's
64d92d74985ebb3d0be58a9718f9e080a14a8e7f. cpu-intel.c is translated to C
from that commit and d84df594404ebbd71d21fec5526178d935e4d88d.
Change-Id: I02c8f83aa4780df301c21f011ef2d8d8300e2f2a
Reviewed-on: https://boringssl-review.googlesource.com/18411
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Also clear AVX512 bits if %xmm and %ymm registers are not preserved. See
also upstream's 66bee01c822c5dd26679cad076c52b3d81199668.
Change-Id: I1bcaf4cf355e3ca0adb5d207ae6185f9b49c0245
Reviewed-on: https://boringssl-review.googlesource.com/18410
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>
Rather than manually register the stack deleters separately, instantiate
them automatically from DEFINE_STACK_OF and BORINGSSL_MAKE_DELETER. The
StackTraits bridge in DEFINE_STACK_OF will additionally be used for
other C++ STACK_OF conveniences.
Bug: 132
Change-Id: I95d6c15b2219b34c7a8ce06dd8012d073dc19c27
Reviewed-on: https://boringssl-review.googlesource.com/18465
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>
It returns false for incomplete types (or is undefined prior to C++14),
so other instantiations can get confused. Instead, require an explicit
kAllowUniquePtr toggle.
I tried using sizeof(T) to SFINAE-detect an incomplete type but ran into
MSVC issues, I think
https://connect.microsoft.com/VisualStudio/feedback/details/820390/vc-sizeof-doesnt-work-as-expected-in-sfinae-context
Though it seems this also may cause ODR violations if different
compilation units disagree on whether a type is complete. This is all a
mess, so just do the boring thing.
Bug: 132
Change-Id: I6f2d47499f16e75f62629c76f43a5329e91c6daf
Reviewed-on: https://boringssl-review.googlesource.com/18464
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
In particular, this removes -fno-rtti, which allows the OSS-Fuzz folks
to run with -fsanitize=vptr. See
https://github.com/google/oss-fuzz/issues/741.
(-fsanitize=vptr isn't especially useful right now as we're just
starting with C++ support, but perhaps it'll be more useful in the
future.)
Change-Id: Ie8944a3e637ebc8dc28c03d331923a7528d7d328
Reviewed-on: https://boringssl-review.googlesource.com/18484
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>
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>
