Anonymous unions are now standard in C, as of C11, but not in C++.
Enabling sufficiently strict warnings in GCC and Clang flag this.
I considered whether we should just remove this and go back to the
OpenSSL formulation, but we actually rely on this being an array when
calling sha1_block_data_order. Upstream types these as taking pointers
to the context, which would work, but taking a pointer to the state is a
bit more accurate. (The assembly function should not touch the buffering
inside the context.)
This anonymous union does mean wpa_supplicant's behavior is slightly
questionable from a strict aliasing perspective, but ah well.
wpa_supplicant uses this to implement an old FIPS 186-2 PRF, which was
based on SHA-1's underlying permutation. Ideally we would either
implement this PRF for them, or have them use their own SHA-1
implementation. They've actually done the latter for OpenSSL 3.0, but
it's a little silly to duplicate the code.
strongswan and go/another-fips-186-2-prf seems to do this too. I'm not
positive what strongswan is doing. I've filed crbug.com/boringssl/667
for follow-up work.
Bug: 667
Fixed: 566
Change-Id: Ife32cc8c278e0dbbd95401ccdd3bd62945e10cf2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63967
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OPENSSL_memcpy already internally checks for empty lengths.
Change-Id: I0015758fd5410e036b532ae727341ae0c0edbdbf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63826
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The AES-GCM EVP_CIPHER has a bunch of machinery for internally managing
a fixed/counter IV split. OpenSSH uses these APIs, so test it.
(OpenSSH barely uses this. It only ever passes -1 as a length to
EVP_CTRL_AEAD_SET_IV_FIXED, and then uses EVP_CTRL_GCM_IV_GEN to
internally increment a counter. But may as well just test the whole
thing, at least until we decide to remove it.)
Change-Id: I688616c586208d27a431836c81d3412799d830bd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63825
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
The actual maximum allowed for this function is INT_MAX - 1. However,
elsewhere we bound parsers to INT_MAX / 2, to guard against overflows
elsewhere in crypto/asn1.
As parsing code is rewritten with CBS, it will naturally handle size_t.
Rather than manually pepper bounds checks in all the functions, we can
catch it at ASN1_STRING_set time. (Whether we'll still need this hack by
the end, I'm not sure, but let's keep it for now.)
Bug: 547
Change-Id: Ia24c9d77d198ba1b5200825a347ca91850f470a2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63526
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
$CMAKE_ASM_FLAGS does not do variable expansion in CMake, only
${CMAKE_ASM_FLAGS}. As a result, we got the string "$CMAKE_ASM_FLAGS"
into the Makefile. That seems to then get interpreted as
${C}MAKE_ASM_FLAGS. I'm not sure why things worked in Ninja, but I
suspect there is some underlying escaping bug in CMake somewhere.
Change-Id: Ibc365282a449ea48f8bcfc194faf62c93e29b6d6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63805
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The implementation is based on the current round 3 specification with
the modifications to FORS indices generation suggest on the mailing
list. The implementation passes test vectors and uses the default SHA256 implementation of BoringSSL.
Change-Id: Iab2dbaf5f692d490577dc940d9f3e298a72e9193
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60965
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Mostly to get things to stop complaining about the old copy of x/net,
though we don't use the HTTP/2 package.
Change-Id: Iab93b5d8639c38370457d0a626a362f47c2e01e2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63766
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
iOS cannot actually fork. Since they have pthread_atfork, we set that up
anyway just in case, but since fork will just fail, we should skip the
tests.
Change-Id: I05f34e13fae84f65f950cd0d71e238ed0366b98e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63765
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This change adds ALPS codepoint supports on split handshake.
serialize_features sends list of codepoints, handshaker will serialize
the codepoint when calling SSL_serialize_handback, and send the chosen
codepoint to the server, server can apply the codepoint when
SSL_apply_handback.
Change-Id: Id7bc985c4b9847b7c337595f1bc23b2af93d96e7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63265
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
waitpid is defined in <sys/wait.h>. Reportedly FreeBSD cares about this.
Fixed: 664
Change-Id: I05be6aa50f97dec3ef7098480b8207cffc73e7a2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63705
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
We get this question a fair amount so it's probably worth putting a
notice in the file.
Change-Id: If14ae76878e5b4085d1fbe2f946d0b1b8060e606
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63647
Auto-Submit: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The latest Clang will generate `.hword`.
Change-Id: Ibdb0356049725c758c20f23fc3c9b60a75c28751
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63646
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
So the chrome build can be slightly less oopy
Bug: chromium:1322914
Change-Id: I0967ba67f8e43e8b996bc246c516b9ffe50bc708
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63665
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This brings in pem_unittest.cc which belongs in here now with
the pem stuff.
Change-Id: I04bb16692d31e2aa9de2c146785684c05543b1a7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63645
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
This provides an alternative way for fork detection that can
be used if we are certain the platform does not have other ways
of cloning an address space in use that are not noticed by
pthread_atfork.
This supports using pthread_atfork for fork detection on
macOS, iOS, OpenBSD and FreeBSD. Linux continues to use MADVISE
for the generation number and fork detection.
Change-Id: I79fd7769477dc90bfe37229d2ff2e8c16898dff7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59906
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
If |param_type| is different from |V_ASN1_UNDEF|, there will usually
be a call to |ASN1_TYPE_new| which allocates and can thus fail. The
result of a failure is that |pval| will leak, which is the case in
both callers in the RSA-PSS code.
This changeset leaves out the call in |X509_ALGOR_set_md|, which
is a void function. This could be fixed in three ways: change its
signature to allow error checking, call |X509_ALGOR_set0| up front
to preallocate, or inline the function in its only internal caller
and remove it from the public API.
Change-Id: I25ed3593947f9ee58208b980a95730d37789c9e1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63585
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We split nid.h out of obj.h and evp_errors.h out of evp.h, but folks who
include the original headers can reasonably assume that the child header
is included.
Change-Id: I81c40fd88df58500a0f10bfa864b8bc98451dbc0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63485
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Callers to these functions are usually using them to grab
subject name components and universally use the result as a C
string, whereas the OpenSSL versions return raw ASN1_STRING
bytes and ignore the encoding, which "usually works" in a
"hold my beer here are some bytes" sort of way until the
object is not encoded as you hoped.
Make this safer for the callers by making the returned
result be at least "text" and a C string. This converts
the ASN1_STRING bytes to UTF-8, and will introduce new
failure cases for this function if either memory allocation
fails for the UTF-8 conversion, or if the resulting UTF-8
contains a 0 codepoint and would produce an artificially
truncated C string. Additionally if the provided buffer
is not NULL but is too small to hold the output, we fail
rather than returning a truncated output.
Fixed: 436
Change-Id: I487c10a5ff5188e94df520ef4c8982e593c680d7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58445
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
This was never updated and not in use. Remove this to simplify the code
and avoid an issue on ARM64 Windows.
~~~
D:\a\firebase-cpp-sdk\firebase-cpp-sdk\BinaryCache\firebase\external\src\boringssl\crypto\mem.c(152): error C2220: the following warning is treated as an error
D:\a\firebase-cpp-sdk\firebase-cpp-sdk\BinaryCache\firebase\external\src\boringssl\crypto\mem.c(152): warning C4746: volatile access of 'kBoringSSLBinaryTag' is subject to /volatile:<iso|ms> setting; consider using __iso_volatile_load/store intrinsic functions
~~~
Change-Id: Iedb0999e38c769add5bb1d5623e038b17f8f245f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63325
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This is currently done by duplicating the list of constants. This was done for two reasons: 1) bindgen doesn't seem to do anything with bare-defines, 2) the list of defines appears to change incredibly rarely.
The `links` key is required in `Cargo.toml` to work around https://github.com/rust-lang/cargo/issues/3544
Change-Id: I11dca6e7eb62ab1b04053df654a4061cb5e25723
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63305
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Add a negative ENUMERATED. This is redundant with
ASN1Test.NegativeEnumeratedMultistring, but once we fix the X509_NAME
value representation, d2i_ASN1_PRINTABLE will be gone. In doing so, I
noticed that we weren't really testing re-encoding, so fix that.
Also add some negative tests, both capturing actual invalid values, and
values which should be valid but aren't due to issue #412.
Bug: 412
Change-Id: Iba7f2869607e6361d6cb913416de21a19cdd6275
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63527
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I764551f842361b455f122287bcf7c4aef4b5cb82
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63546
Auto-Submit: Cindy Lin <cinlin@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
tasn_*.c have two dependencies on the OID table: initializing
ASN1_OBJECTs to the undef object, and the ADB (ANY DEFINED BY)
machinery. Fix the first by pulling the entry out of the table. The
latter will be fixed by rewriting the certificate policy parser.
Bug: 551
Change-Id: I7c423ff9ce78b850555203a31c2d220d92d04f35
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63530
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
NID_undef actually has names, but OBJ_sn2nid and OBJ_ln2nid's calling
convention cannot distinguish finding NID_undef from finding nothing.
Thus we may as well save 4 bytes by omitting this.
Change-Id: I6102e67141a2f5524aacf0ea84e6a2b2d2add534
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63529
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
This warning is ultimately a false positive, but does reflect an
annoyingly sharp edge in the CBB API.
Due to C's lack of destructors and |CBB|'s auto-flushing API, a failing
|CBB|-taking function may leave a dangling pointer to a child |CBB|. As
a result, the convention is callers may not write to |CBB|s that have
failed. But, as a safety measure, we lock the |CBB| into an error
state. Once the error bit is set, |cbb->child| will not be read.
See also https://boringssl-review.googlesource.com/8840
However, there were a few codepaths in cbb.c that did not set the error
bit. Additionally, GCC does not know this invariant, so it flags a
dangling pointer warning. Fix the missing path, and explicitly null the
child pointers whenever we set the error flag.
Weirdly, the explicit null doesn't seem to be necessary, but if I inline
things manually and then delete some seemingly unrelated branches, it
becomes necessary. I assume GCC's analysis is just very fragile or
buggy, so let's just explicitly null the pointer.
This still isn't quite ideal. A |CBB| function *outside* this file may
originate an error while the |CBB| points to a local child. In that
case we don't set the error bit and are reliant on the error convention.
Perhaps we allow |CBB_cleanup| on child |CBB|s and make every child's
|CBB_cleanup| set the error bit if unflushed. That will be convenient
for C++ callers, but very tedious for C callers. So C callers perhaps
should get a |CBB_on_error| function that can be, less tediously, stuck
in a |goto err| block. I've left this as a TODO for now. (Note the
|CBB_cleanup| strategy will capture the error bit, which is the
important one, but it cannot capture the explicit nulling. So we are
also relying on GCC not seeing through translation units right now.)
Fixed: 621
Change-Id: I9dd1c48e642fc2834940d178678f17b14009c412
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63206
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Since ACLE is now widely implemented, just use the standard one. I've
left a TODO with __ARM_MAX_ARCH__. Probably should just remove that one?
Also deduplicate some code between arm_arch.h and asm_base.h. I think I
meant to move it to asm_base.h and didn't finish the job?
Change-Id: I85bb3160ec64acdabd11d741f1958ff56199c4c7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63525
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In order to run KATs, we need a copy of libcrypto which won't
abort on integrity check failure, or we'd never get to the KAT.
Previously this was expected to be magically present in the
current directory, but nowawdays we have a build rule for it
so pick it up from the build artifacts.
Change-Id: Ia5359b5369475d94bbde90e81353420a0f1efea2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63445
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Pete Bentley <prb@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is not in upstream OpenSSL but saves a bunch of manual overflow
checks. Note it does also introduce some zeroing of buffers, but I think
this should be fine here.
Change-Id: I0c3e65ce2d21ee9d206ccbe3075ce5291c3acb30
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63365
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OpenSSL added a similar helper function. It's very, very common for us
to malloc something an then zero it. This saves some effort. Also
replace some more malloc + memcpy pairs with memdup.
Change-Id: I1e765c8774a0d15742827c39a1f16df9748ef247
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63345
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
With -march=haswell -DOPENSSL_SMALL=1 on cascadelake:
Did 9999 ECDH P-256 operations in 1062469us (9411.1 ops/sec) [+63.5%]
Did 25000 ECDSA P-256 signing operations in 1028302us (24311.9 ops/sec) [+48.9%]
Did 11004 ECDSA P-256 verify operations in 1072646us (10258.7 ops/sec) [+58.8%]
Same configuration measured no performance difference on haswell.
The added assembly code occupies 1352 bytes.
Change-Id: I42635b7a9bf24d942817976a5d4ce269f642251c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63185
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
- Create an internal `BlockCipher` trait similar to the existing
`StreamCipher` trait for AES-CBC.
- Create wrappers in the internal `Cipher` struct for one-shot
allocating encryption and decryption operations.
Change-Id: I17f667b3b92f907bc14c3454ee49b88cb91c49f3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63125
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
It's probably worth explaining in a comment that this is about
implementation-defined behavior, and why we consider it okay to make
assumptions like uint8_t == unsigned char.
Change-Id: Ia35248aef7895b0998831b6bac06993e845e6297
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63285
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This removes one more patch, and adapts import to deal with gmock from chrome
which is now included in boring.
Bug: chromium:1322914
Change-Id: I2a5957f741252941fea76205a21e98fd655f8cae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63225
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Instead the spelling is message(FATAL_ERROR "blah"). Although
error("blah") also works because it just complains that error doesn't
exist.
Change-Id: I80384e0198a9013f93f9403d0a4c256749905045
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63106
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
BoringSSL can currently be built in C11 or pre-C11 mode in MSVC. They're
broadly the same, but do use completely different implementations of
alignas and alignof. Now that every build configuration I'm aware of has
been moved to the C11 mode, we don't even test the pre-C11 mode anymore.
Start requiring it.
Update-Note: If building with MSVC, BoringSSL now requires building with
/std:c11 or later. (On non-MSVC compilers, we have required C11 for a
while now.)
Fixed: 624
Change-Id: Ie9f66eee0bebac8143c23a7229c6854afaefea6e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63065
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Old version Chrome with the existing ALPS codepoint can potentially cause network error due to an arithmetic overflow bug in Chrome ALPS decoder (We already fixed the issues starting from M100 in Chrome).
This CL add a new codepoint for ALPS extension in a way that can be enabled on individual connections., To support multiple versions of Chrome, we need to support both codepoints in BoringSSL.
For details: https://docs.google.com/document/d/16pysbV_ym_qAau_DBYnrw2A4h5ve2212wfcoYASt52U
Change-Id: Iea7822e757d23009648febc8eaff1c91b0f06e18
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61125
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Bug: chromium:1322914
Change-Id: Ic5a1349013bcfb279e5fee9f9838c63558d663b7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63025
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
David Benjamin
Stefan Koelbl
Andres Erbsen
Victor Tan
Adam Langley
Bob Beck
Guillaume Endignoux
Theo Buehler
Saleem Abdulrasool
Alex Gaynor
Cindy Lin
Pete Bentley
Maurice Lam
Alice Wang
Victor Tan