We clear all heap memory on free now, thus the difference between these
functions is quite small. There are some differences though:
Firstly, BN_clear_free will attempt to zero out static limb data. But
static data is probably read-only and thus trying to zero it will crash.
Secondly it will try to zero out the BIGNUM structure itself. But either
it's on the heap, and will be zeroed anyway, or else it's on the stack,
and we don't try and clear the stack in general because the compiler is
duplicating bits of it at will anyway.
Change-Id: I8a07385a102cfd308b555432942225c25eb7c12d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45084
Reviewed-by: David Benjamin <davidben@google.com>
Add Aarch64 support to delocate. Since it's a modern ISA, it's actually
not too bad once I understood the behaviour of the assembler.
Change-Id: I105fede43b5196b7ff7bdbf1ee71c6cfa2fc1aab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44848
Reviewed-by: David Benjamin <davidben@google.com>
OpenSSL has a fixed-width version of DH_compute_key nowadays. Searching
around callers of DH_compute_key, many of them go back and re-pad the
secret anyway. Uses of DH should migrate to modern primitives but, in
the meantime, DH_compute_key_padded seems worthwhile for OpenSSL
compatibility and giving fixed-width users a function to avoid the
timing leak.
Bump BORINGSSL_API_VERSION since one of the uses is in wpa_supplicant
and they like to compile against a wide range of Android revisions.
Update-Note: No compatibility impact, but callers that use
DH_compute_key and then fix up the removed leading zeros can switch to
this function. Then they should migrate to something else.
Change-Id: Icf8b2ace3972fa174a0f08ece39710f7599f96f2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45004
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
When offering 0-RTT, the client should check that all carried-over
values are consistent with its preferences. This ensures that parameter
negotiation happens independently of 0-RTT. The ALPS version of this
check was a tad too aggressive: a session without ALPS should be treated
as always compatible.
I'll follow this with a fix to the draft spec to clarify this.
Change-Id: Ia3c2a60449c555d1d91c4e528215f8e551a90a9f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45104
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Since we don't have XTS in the FIPS module, this change uses
testmodulewrapper for testing.
Change-Id: I82117472ea4288d017983fe9cc11d4ba808a972a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45064
Reviewed-by: David Benjamin <davidben@google.com>
IETF QUIC draft 33 is replacing the TLS extension
codepoint for QUIC transport parameters from 0xffa5
to 57. To support multiple versions of Chrome, we
need to support both codepoints in BoringSSL. This
CL adds support for the new codepoint in a way that
can be enabled on individual connections.
Note that when BoringSSL is not in QUIC mode, it
will error if it sees the new codepoint as a server
but it will ignore the legacy codepoint as that could
be a different private usage of that codepoint.
Change-Id: I314f8f0b169cedd96eeccc42b44153e97044388c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44704
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Aarch64 perlasm outputs need to be run via the C preprocessor before
being parsed by delocate.
Change-Id: Ie420388e0e1707cb064d696ee8f87728cab9a36e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44847
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
They align exactly with TLS record types, so just use the existing
constants.
Change-Id: I693e7c740458cf73061e6b573eeb466d0fce93cc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44990
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This changes the format of the mock QUIC transport to include an
explicit encryption level, matching real QUIC a bit better. In
particular, we need that extra data to properly skip rejected early data
on the shim side. (On the runner, we manage it by synchronizing with the
TLS stack. Still, the levels make it a bit more accurate.)
Testing sending and receiving of actual early data is not very relevant
in QUIC since application I/O is external, but this allows us to more
easily run the same tests in TLS and QUIC.
Along the way, improve error-reporting in mock_quick_transport.cc so
it's easier to diagnose record-level mismatches.
Change-Id: I96175a4023134b03d61dac089f8e7ff4eb627933
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44988
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This originally didn't work because we installed an async BIO, while
QUIC uses the BIO to mock out a QUIC transport. Our QUIC IO callbacks
don't have a meaningful notion of sync vs async, so no-op this portion
of the -async flag.
The immediate motivation is I'd like to make addExtensionTests run over
all protocols, and having the async tests fail is inconvenient. However,
async tests in QUIC is still meaningful anyway to support various
callbacks, so I've removed the workaround in the state machine coverage
tests. (Though most of those async tests are redundant as they're
concerned with IO, not callbacks.) Along the way, the various handshake
record controls are irrelevant to QUIC, so this actually results in a
net decrease in redundant tests.
Change-Id: I67c1ee48cb2d85b47ae3328fecfac86a24aa2ed1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44987
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The state machine around EndOfEarlyData is a bit messy, which caused a
problem introducing the new message in QUIC. We keep waffling on whether
that state junction should no-op the EndOfEarlyData state or skip it.
Since skipping it caused us to miss this spot, let's try no-op-ing it.
As a test, so this CL is easier to cherry-pick, I've just duplicated the
basic server test. Better, however, would be to run all the extensions
tests under QUIC. (In particular, this is missing 0-RTT coverage.) But
this is a large diff and requires improving the mock QUIC transport,
etc., in runner. That work is done in follow-up CLs, which replace this
duplicated test.
Change-Id: I25b6feabdc6e5393ba7f486651986a90e3721667
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44985
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
There was a QUIC-specific ALPS bug, so I'm thinking we should loop
addExtensionTests at all protocols. To do so, we need to fix this bug in
the test expectation.
Change-Id: Ic05a4cb2ea32e7145441a0273cd65966c41534ec
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44986
Reviewed-by: Adam Langley <agl@google.com>
This wasn't the cause of the bug, but I noticed we never tested it, so
fill that in.
Change-Id: Ib38bc08309e69f43c1995ba2a387643c0a7bae99
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44984
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This CL implements the backend server behavior described in Section 7.2
of draft-ietf-tls-esni-09.
Bug: 275
Change-Id: I2e162673ce564db0cb75fc9b71ef11ed15037f4b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43924
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Aarch64 assembly is quite different from x86-64 or POWER. But the system
of directives is the same so there's quite a lot of utility from being
able to use the same delocate framework.
Unfortunately, with peg, there's no obvious way to be able to parse
instructions differently without breaking the parsing into two stages.
Thus the parser is extended here to support all three ISAs. This seems
to work ok without breaking either of the other two.
Change-Id: Iced0f651e556e6ffae3eb35f2edfc0bf84167967
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44846
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Windows on Arm (WoA) builds are currently using the C implementations
of the various functions within BoringSSL. This patch enables feature
detection for the Neon and hardware crypto optimizations, and updates
the perl script to generate AArch64 .S files for WoA.
Note these files use GNU assembler syntax (specifically tested with
Clang assembler), not armasm.
Change-Id: Id8841f4db0498ec16215095a4e6bd60d427cd54b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43304
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
gai_strerror is one of the Windows functions which behaves differently
whether UNICODE is defined. See
https://docs.microsoft.com/en-us/windows/win32/intl/conventions-for-function-prototypes
Call gai_strerrorA so that we behave consistently in both modes. This
fixes the build failure in
https://chromium-review.googlesource.com/c/chromium/src/+/2613519.
It also fixes a type error in the connect BIO (built but not used in
Chromium), which wasn't noticed because ERR_add_error_data is a variadic
function and untyped. (The type error won't go out of bounds because
we're interpreting a NUL-terminated WCHAR* as a NUL-terminated char*.
The string will be misinterpreted, but it still will be terminated
either at the NUL WCHAR or, more likely, the upper zero byte of the
first Latin-1 character in the string.)
The ERR_add_error_data call raises the question of which of our char*
strings are UTF-8 and which are the POSIX locale / Windows code page
(when those are not also UTF-8). This CL doesn't address this and only
fixes the character width error. Realistically, calling code tosses
char* to printf so often that non-UTF-8 locales are probably a lost
cause. (Although right now we do not transform any OS error strings, so
tossing them to printf works fine. The outputs of functions like
ASN1_STRING_to_UTF8, not so much.)
Change-Id: Ie789730658829bde90022605ade2c86b8a65c3de
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44964
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Updating HPKE to draft-irtf-cfrg-hpke-07 added a number of tests to
crypto/hpke/hpke_test_vectors.txt. The time to run HPKE crypto tests
increased from 0.09 to 0.56 seconds (averaged over 6 runs). The runtime
for the whole crypto_test suite increased from 86.44 to 88.19 (measured
once).
Profiling revealed an excessive amount of CPU time (~50% for HPKE tests)
spent on std::map lookups in ReadNext(). I found a slow loop that
computes the suffix for a repeated attribute by hammering a std::map
with incremental guesses.
This CL adds a std::map<std::string, size_t> for counting repeated
attributes, eliminating the need for the loop. This reduces the runtime
for HPKE tests from 0.56 to 0.12 seconds (averaged over 6 runs). For the
whole crypto_test suite, runtime is reduced from 88.19 to 86.71
seconds (measured once).
Change-Id: Ie87f4a7f3edc95d434226d2959dcf09974f0656f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44905
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I814f55742910c519e9b64aca1b15a4d754adc541
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44944
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Bug: 275
Change-Id: Ifef2b94f701ab75755893c2806335b626b655446
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44904
Commit-Queue: Dan McArdle <dmcardle@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
It has been subsumed by the tests added in the previous change.
Change-Id: Ie53e8bd1116d2a70b9b88b2b59163e0f9a3140e8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44747
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This change eliminates an Aarch64 assembly pattern that only appears
in this case. It's easier to change this code than to pile more things
on top of the delocate parser.
Change-Id: I6bbbe9df744ec2ad4178d74456d8f4fecc3a2dae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44845
Reviewed-by: David Benjamin <davidben@google.com>
aarch64 assembly files use "//" as the comment indicator because '#'
indicates a constant value.
Change-Id: I53b18cbb3498522b0924716238abf55e6627d216
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44844
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Fuchsia is looking to remove the ZX_WAIT_ASYNC_ONCE constant, and our
copy of GoogleTest still has it. As usually with GoogleTest updates, our
old local patch is no longer necessary, but now we need a new one.
Change-Id: I8d226f01cf0951fd278605688684bf1ce3e17898
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44884
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This change causes the run_tests target, in FIPS builds, to run ACVP
for each supported algorithm. For most of them the output can be compared
against a known result. For some of them the output is too variable and
they are only run to ensure that they don't have local errors.
The ACVP test vectors have been trimmed significantly because they're
often huge. Firstly an included tool drops all but one test from each
group. Some vector sets have been manually trimmed to remove tests that
cause variable output.
Change-Id: Iff73851e3d47813041cc7ea6d881282750274940
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44746
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
One of the comments in e56dfcf9f4 was worded awkwardly. Thanks to Zi Lin
for fixing this.
Change-Id: I7ee647716e0ee30145bdce5be35128058130e1ec
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44764
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Newer versions of LLVM may use profile information to put code in
sections named .text.unlikely.*. Make sure those sections end up in
our .text section.
Change-Id: Ia0224fd8e683f5e77c60dd3ad34d59b33f9b41ab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44804
Commit-Queue: Peter Collingbourne <pcc@google.com>
Reviewed-by: Adam Langley <agl@google.com>
A couple of processing errors crept in over time. Caught by the tests
in the next change.
Change-Id: I0caa478d3321cb8a1da1e61ddde16ba8db91eb35
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44745
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The config file is only needed if interacting with an ACVP server.
Invocations that just want to process input from a file don't need it.
By moving this chunk of code down, the config isn't loaded until
after handling JSON inputs and just can be ignore if not needed.
Change-Id: Ibce334f63ddf8df34cf2917b923db20b3aaa735f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44744
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
BER permits lengths to be non-minimal. Previously this was not supported
at all. This change brings greater support, allowing non-minimal lengths
so long as they fit in a uint32_t.
Change-Id: I002ed2375c78fdb326e725eb1c23eca71ef9ba4a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44684
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This is so we can build the tool in Chromium and run benchmarks using
Chromium's build config.
Change-Id: I16b4d99a923cd61f338ba488cb0abdfce3c0a3d2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44724
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This reflects an internal LSC cleanup.
Change-Id: Ic6d363ab14e0b021a579cdcf0a7a68a9021e2e18
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44664
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This moved, en masse, into the FIPS module in e7f08827d2. But we want to
minimise the amount that's in the FIPS module and it doesn't appear that
we need this at the current time.
Change-Id: Ib2c243aad461b716314eeeb6a460955818a7aa22
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44605
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
IETF QUIC draft 33 is replacing the TLS extension
codepoint for QUIC transport parameters from 0xffa5
to 57. To support multiple versions of Chrome, we
need to support both codepoints in BoringSSL. This
CL adds support for the new codepoint in a way that
can be enabled on individual connections.
Change-Id: I3bf06ea0710702c0dc45bb3ff2e3d772e9f87f9b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44585
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The headers aren't quite interchangeable. stdlib.h defines ::abort()
while cstdlib defines std::abort(). The Google style guide doesn't give
much guidance but says to match the existing style, so I've switched it
to stdlib.h.
See https://github.com/apple/swift-nio-ssl/issues/259
Change-Id: I19feb5213e123a88b381d6d8f8fe9d8e87c81e67
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44625
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
I accidentally lost these lines juggling patches around.
Change-Id: I35551eeb0f1bb26dee74344048198a318c55209b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44624
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Current:
Did 2916000 ChaCha20-Poly1305 (16 bytes) seal operations in 1015000us (2872906.4 ops/sec): 46.0 MB/s
Did 1604750 ChaCha20-Poly1305 (256 bytes) seal operations in 1016000us (1579478.3 ops/sec): 404.3 MB/s
Did 516750 ChaCha20-Poly1305 (1350 bytes) seal operations in 1015000us (509113.3 ops/sec): 687.3 MB/s
Did 99750 ChaCha20-Poly1305 (8192 bytes) seal operations in 1016000us (98179.1 ops/sec): 804.3 MB/s
Did 50500 ChaCha20-Poly1305 (16384 bytes) seal operations in 1016000us (49704.7 ops/sec): 814.4 MB/s
With fix:
Did 6366750 ChaCha20-Poly1305 (16 bytes) seal operations in 1016000us (6266486.2 ops/sec): 100.3 MB/s
Did 3938000 ChaCha20-Poly1305 (256 bytes) seal operations in 1016000us (3875984.3 ops/sec): 992.3 MB/s
Did 1207750 ChaCha20-Poly1305 (1350 bytes) seal operations in 1015000us (1189901.5 ops/sec): 1606.4 MB/s
Did 258500 ChaCha20-Poly1305 (8192 bytes) seal operations in 1016000us (254429.1 ops/sec): 2084.3 MB/s
Did 131500 ChaCha20-Poly1305 (16384 bytes) seal operations in 1016000us (129429.1 ops/sec): 2120.6 MB/s
Change-Id: Iec6417b9855b9d3d1d5154c93a370f80f219c65f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44347
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
If using precompiled headers then this is needed otherwise bn/internal.h
doesn't have a definition for BN_ULONG etc.
Change-Id: I41b331465abae7108f255722a156d2ffb3016ba3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44604
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This involves adding a new function |DH_compute_key_hashed| that
combines the FFDH with the output hashing inside the FIPS module. This
new function uses the padded FFDH output, as newly specified in SP
800-56Ar3.
Change-Id: Iafcb7e276f16d39bf7d25d3b2f163b5cd6f67883
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44504
Reviewed-by: David Benjamin <davidben@google.com>
This change also drops ex_data from DH objects. The global would need
special handling in the FIPS module, which isn't hard, but just dropping
it saves some of the code-size costs of this change and I cannot find
any signs of use of this functionality.
Change-Id: I984bd70698c2ec329f340d294b3b9ec169cd0c4e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44524
Reviewed-by: David Benjamin <davidben@google.com>
Imported from upstream's 617b49db14fa4c1211bfc5d0e88294d0f159c9a9.
Change-Id: I64349b7cbbda8fbacf1e20ca609081ed42f10550
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44565
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
As of
https://chromium-review.googlesource.com/c/chromium/tools/build/+/2586225,
we no longer test on Yasm. Yasm hasn't seen a release for over six years
now and is missing support for newer x86 instructions.
This removes the remnants of support for Yasm on the CI. It also removes
the Yasm support we patched into x86nasm.pl, which removes a now
unnecessary divergence from upstream.
Update-Note: If a x86 Windows asm build breaks, switch from Yasm to
NASM. We're also no longer testing NASM on x86_64 Windows, but there
wasn't any patch to revert.
Change-Id: I016bad8757fcc13240db9f56dd622be518e649d7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44564
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Adam Langley
David Benjamin
Yuchen Dai
David Schinazi
Dan McArdle
Anthony Roberts
Adam Langley
Peter Collingbourne
Vlad Krasnov