In the STL, <iterator> has a std::size for arrays. Some of these could
also just be ranged for loops. One static_assert could not use
std::size(out->whatever) because out was not a compile-time value, but
std::extent_v<decltype(out->whatever)> works instead.
Change-Id: I28007c79f5583e09167b81a34a447e205ee6dd9b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81658
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Historically, sha.h included both SHA-1 and SHA-2 functions. But SHA-1
functions mostly shouldn't be used now, and it's useful to be able to
audit at the level of header names in some contexts.
Therefore move SHA-2 things into a new sha2.h. In order not to break
everything, sha.h now includes sha2.h so no changes are needed in
existing callers.
Change-Id: I68d5e991f58a1c74ca377ba017caaff356acc870
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/80327
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Although the fuzzers are expected to be used with
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION, build systems sometimes build
them in a non-fuzzer configuration. Keep the targets working in those
configs.
Change-Id: I351c3f6366ca97cb23144164fd5e49d1fa2286a6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78007
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Instead of having a pair of bespoke build definitions use the standard
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION toggle. We actually originated
the idea of a fuzzing-specific build toggle, and then libFuzzer
standardized a toggle when we talked to them about what we were doing.
The problem is our fuzzer mode toggle substantially changed the TLS
stack behavior, such that downstream code would likely go haywire. So we
couldn't easily fold into the standard one, and all of BoringSSL's
downstream fuzzer builds were messy.
Instead, make a few changes:
1. Switch BORINGSSL_UNSAFE_DETERMINISTIC_MODE to
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION. That flag is not expected
to cause downstream issues as it just makes the PRNG deterministic.
2. Replace BORINGSSL_UNSAFE_FUZZER_MODE with a runtime toggle that is
only available when building with
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION.
3. Instead of the no_fuzzer_mode fuzzers being special corpora for the
client and server fuzzers, they're now just separate fuzzerrs and
follow the usual naming conventions between fuzzers and their
corpora.
Update-Note: Downstream fuzzer builds can now be simplified. If the
fuzzing infrastructure already builds with
FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION, the separate boringssl_fuzz
(or whatever) target can be removed.
Bug: 42290128
Change-Id: Ia1e479777f366908951e15067c96c9767c229f0a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77749
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
CMake expects you to provide your link lines in the right order for
platforms without rescanning linkers. When they're not in the right,
it'll preserve your order but duplicate transitive dependencies. That
is, if:
A -> C B
B -> C
The final link line will be:
A C B C
Wheras if A wrote B C then there would be no duplication. Newer macOS
toolchains (which do not need the duplication) seem to warn on duplicate
libraries, which is how I noticed this.
That said, this is not actually sufficient to avoid duplication and thus
the warning. Consider:
A -> B D
B -> C D
CMake always lists direct dependencies before transitive ones, so the
result will be:
A B D C D
decrepit_test triggers this because decrepit_test does not directly
depend on ssl but decrepit does. It's a bit awkward to have to list it
again, but adding it avoids this issue.
Newer CMakes (3.31) now know that:
1. macOS rescans dependencies so the dependencies don't have to be in
order.
2. macOS has this warning so it should dedup things.
However, even updating to 3.31 isn't sufficient because CMake keys all
behavior changes on cmake_minimum_required. I didn't set the target
policy version because I figure testing it at 3.16's behavior is
probably useful for the sake of keeping 3.16 hopefully working.
Change-Id: Ibb006eefbbb23f1899ef91277465d31fa8202d2e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77747
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
With is_standard_layout and is_trivially_copyable. is_pod is deprecated
in C++20 so we should change uses of it with appropriate checks.
Bug: 402877140
Change-Id: Ie1997fcac0b05a4bdfbb67c15b1d76bfa4e3a1bc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77527
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Applications sometimes need to create SSL_CLIENT_HELLO objects for
testing. We already have this function internally. Just expose it as
public API.
The only non-trivial change here is I've moved the error queue bits from
the caller of ssl_client_hello_init to inside SSL_parse_client_hello.
This matches our conventions a bit better (usually functions are
responsible for originating the error), and keeps ssl.h's public APIs
consistently using the error queue here.
Bug: 395069491
Change-Id: I16fb35772a61e98d2621f781e475aa5611b13582
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76128
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
We use the standard Apache 2.0 file header, described in "APPENDIX: How
to apply the Apache License to your work."
This was primarily automated by running:
git ls-tree -r --name-only HEAD | xargs go run ./util/relicense.go
See go/boringssl-relicensing-triage for the results of triaging the
output of the tool.
As part of this, switch from taking fiat-crypto under MIT license to
Apache 2.0. (It is licensed under MIT OR Apache-2.0 OR BSD-1-Clause.)
The copyright_summary tool can also be used to confirm we didn't
accidentally drop any copyright lines:
# Run before the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -out /tmp/old.json
# Run after the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -compare /tmp/old.json
Bug: 364634028
Change-Id: I17c50e761e9d077a1f92e25969e50ed35e320c59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75852
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Following the guidance in
https://opensource.google/documentation/reference/releasing/authors,
start maintaining an AUTHORS file.
Update all existing Google copyright lines to 'The BoringSSL Authors'
per the document. This CL also changes the styling to match the new
guidance: removed the '(c)' and the comma.
All other existing copyright lines are left unmodified. Going forward,
our preference will be that new contributions to BoringSSL use 'The
BoringSSL Authors', optionally adding to the AUTHORS file if the
contributor desires.
To avoid being presumptuous, this CL does *not* proactively list every
past contributor in the BoringSSL half of the AUTHORS file. Past
contributors are welcome to send us a patch to be added, or request that
we add you. (Listed or not, the commit log continues to be a more
accurate record, and any existing non-Google copyright lines were left
unmodified.)
The OpenSSL half of the AUTHORS file is seeded with the contents of the
current OpenSSL AUTHORS file, as of writing. The current contents in the
latest revision of the 1.1.1 branch
(b372b1f76450acdfed1e2301a39810146e28b02c) and master
(d992e8729ee38b082482dc010e090bb20d1c7bd5) are identical, just formatted
in text vs Markdown.
Note when reviewing: CONTRIBUTING.md and AUTHORS contain non-mechanical
changes.
Bug: 364634028
Change-Id: I319d0ee63ec021ad85e248e8e3304b9cf9566681
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74149
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The public headers are not yet merged. That will be doen in the
subsequent CL. This required teaching make_errors.go that x509v3 are
found elsewhere, also to skip irrelevant OPENSSL_DECLARE_ERROR_REASON
calls.
Change-Id: Ic40de51f9a5325acd60262c614924dc3407b800c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64137
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Seed the corpus from cert_corpus. As part of that, check in the result of minimizing all the corpora.
Note this is just making one of the fuzzers build, I'll adapt
the others and follow on by updating the IMPORT process to do it
in a follow on cl.
Bug: chromium:1322914
Change-Id: Iea1b89f8fee938fa99c0a4d8134bcd0e7023d149
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61765
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Right now we use NIDs to configure the group list, but group IDs (the
TLS codepoints) to return the negotiated group. The NIDs come from
OpenSSL, while the group ID was original our API. OpenSSL has since
added SSL_get_negotiated_group, but we don't implement it.
To add Kyber to QUIC, we'll need to add an API for configuring groups to
QUICHE. Carrying over our inconsistency into QUICHE's public API would
be unfortunate, so let's use this as the time to align things.
We could either align with OpenSSL and say NIDs are now the group
representation at the public API, or we could add a parallel group ID
API. (Or we could make a whole new SSL_NAMED_GROUP object to pattern
after SSL_CIPHER, which isn't wrong, but is even more new APIs.)
Aligning with OpenSSL would be fewer APIs, but NIDs aren't a great
representation. The numbers are ad-hoc and even diverge a bit between
OpenSSL and BoringSSL. The TLS codepoints are better to export out to
callers. Also QUICHE has exported the negotiated group using the
codepoints, so the natural solution would be to use codepoints on input
too.
Thus, this CL adds SSL_CTX_set1_group_ids and SSL_set1_group_ids. It
also rearranges the API docs slightly to put the group ID ones first,
and leaves a little note about the NID representation before introducing
those.
While I'm here, I've added SSL_get_negotiated_group. NGINX seems to use
it when available, so we may as well fill in that unnecessary
compatibility hole.
Bug: chromium:1442377
Change-Id: I47ca8ae52c274133f28da9893aed7fc70f942bf8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60208
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We're this awkward mix of "group" and "curve" right now. On the spec
side, this is because they used to be "curves", but then RFC 7919
renamed to "group" in an attempt to generalize FFDH and ECDH. The
negotiated FFDH stuff never really went anywhere (the way it used cipher
suite values in TLS 1.2 made it unusable), but the name change stuck.
In our implementation and API, we originally called it "curve". In
preparation for TLS 1.3, we renamed the internals to "group" to match
the spec in
https://boringssl-review.googlesource.com/c/boringssl/+/7955, but the
public API was still "curve".
Then we exported a few more things in
https://boringssl-review.googlesource.com/c/boringssl/+/8565, but I left
it at "curve" to keep the public API self-consistent.
Then we added OpenSSL's new "group" APIs in
https://boringssl-review.googlesource.com/c/boringssl/+/54306, but
didn't go as far to deprecate the old ones yet.
Now I'd like to add new APIs to clear up the weird mix of TLS codepoints
and NIDs that appear in our APIs. But our naming is a mess, so either
choice of "group" or "curve" for the new API looks weird.
In hindsight, we probably should have left it at "curve". Both terms are
equally useless for the future post-quantum KEMs, but at least "curve"
is more unique of a name than "group". But at this point, I think we're
too far through the "group" rename to really backtrack:
- Chromium says "group" in its internals
- QUICHE says "group" in its internals and public API
- Our internals say "group"
- OpenSSL has switched to "group" and deprecated "curve", so new APIs
will be based on "group"
So align with all this and say "group". This CL handles set1_curves and
set1_curves_list APIs, which already have "group" replacements from
OpenSSL. A follow-up CL will handle our APIs. This is a soft deprecation
because I don't think updating things is particularly worth the churn,
but get the old names out of the way, so new code can have a simpler API
to target.
Also rewrite the documentation for that section accordingly. I don't
think we need to talk about how it's always enabled now. That's a
reference to some very, very old OpenSSL behavior where ECDH negotiation
needed to be separately enabled.
Change-Id: I7a356793d36419fc668364c912ca7b4f5c6c23a2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60206
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Trying to fix all the places where these formats go quadratic isn't a
good use of time. We've already documented that they're not safe for use
with untrusted inputs. Even without such DoS issues, they cannot be
safely used anyway. (E.g. RUSTSEC-2023-0023.)
Just cap the fuzzer input. It'd be nice if we could avoid this more
systematically in the function, but they're not structured to make this
easy to do, and anyone concerned about DoS in this function has worse
problems.
Bug: chromium:1444420, oss-fuzz:56048, 611
Change-Id: I53eeb346f59278ec2db3aac4a92573b927ed8003
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59785
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
We used to have a tower of fallbacks to support older Androids that were
missing getauxval. The comments say getauxval is available in Android
API level 20 or higher, but this wasn't right. It's actually API level
18 or higher per the NDK headers and
https://developer.android.com/ndk/guides/cpu-features
Android API level 18 is Android 4.3, or Jelly Bean MR2. Recent versions
of the NDK (starting r24, March 2022) don't even support Jelly Bean,
i.e. the minimum API level is 19, and the usage statistics in the latest
Android Studio stop at KitKat. As far as I know, nothing needs us to
support API levels 17 and below anymore.
Update-Note: BoringSSL now requires API level 18 or later. Projects
needing to support API level of 17 or below will fail to build due to
the use of getauxval. If any such projects exist, please contact
BoringSSL maintainers.
Change-Id: Iedc4836ffd701428ab6d11253d4ebd5a9121e667
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57506
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This was added with the generated symbol-prefixing header. But it
seems to be sufficient for crypto to have a dependency on the
generated header, along with some of the stray bits of delocate.
It's a little unclear from CMake documentation how these are processed;
normally .o files can be built before libraries are built or linked,
only the link step depends on.
But, empirically, if A links B, and B has a dependency on C, then CMake
seems to run C before building any of A. I tested this by making a small
project where the generation step slept for three seconds and running
with enough parallelism that we'd have tripped.
Interestingly, in the Makefile output, the individual object file
targets didn't have that dependency, but the target itself did. But this
was true on both A and B, so I think that just might not work.
Also fix the dependency in the custom target. The old formulation broke
when using an absolute path to the symbols file.
Change-Id: I2053d44949f907d465da403a5ec69c191740268f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
It's unclear to me whether doing it target-by-target is an improvement
in crypto/fipsmodule, but this otherwise does seem a bit tidier. This
aligns with CMake's documentation and "modern CMake" which prefers this
pattern.
Change-Id: I36c81842bff8b36eeaaf5dd3e0695fb45f3376c9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56585
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
All evidence we have points to these devices no longer existing (or at
least no longer taking updates) for years.
I've kept CRYPTO_has_broken_NEON around for now as there are some older
copies of the Chromium measurement code around, but now the function
always returns zero.
Change-Id: Ib76b68e347749d03611d00caecb6b8b1fdbb37b1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56765
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This fuzzes the config file parser, and the converrsion to X.509
extensions. The initial corpus was computed by:
1. Import every file from OpenSSL 1.1.1 that ends in .cnf.
2. For each section in each such file, add a copy with that section
copied to the top (the "default") section.
3. Also add a file for each unit test.
4. Minimize the corpus.
While I'm here, sort the targets in fuzz/CMakeLists.txt.
Change-Id: I0cfc1ae8e2be3e67dae361605ad19833aec3fe4d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56167
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We use unsigned, but we actually assume it is 32-bit for the bit-packing
strategy. But also introduce a typedef to hint that callers shouldn't
treat it as an arbitrary 32-bit integer. A typedef would also allow us
to extend to uint64_t in the future, if we ever need to.
Update-Note: Some APIs switch from unsigned * to uint32_t * out
pointers. This is only source-compatible if unsigned and uint32_t are
the exact same type. The CQ suggests this is indeed true. If they are
not, replace unsigned with CBS_ASN1_TAG to fix the build.
Bug: 525
Change-Id: I45cbe127c1aa252f5f6a169dca2e44d1e6e1d669
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54986
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Ran the following command at OpenSSL commit
18622c7625436d7f99c0f51895c4d3cea233c62e:
./build-fuzz/fuzz/cert -merge=1 -max_len=10000 fuzz/cert_corpus/ ~/openssl/fuzz/corpora/x509
Change-Id: I22c4051351138736a0fa0202c0977ca9afc6924c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49047
Reviewed-by: Adam Langley <agl@google.com>
Previously we would extract the KEM ID from the ECHConfig and then parse
the private key using the corresponding KEM type. This CL makes it take
a pre-pared EVP_HPKE_KEY and checks it matches. This does require the
caller pass the key type through externally, which is probably prudent?
(On the other hand we are still inferring config from the rest of the
ECHConfig... maybe we can add an API to extract the EVP_HPKE_KEM from a
serialized ECHConfig if it becomes a problem. I could see runner or tool
wanting that out of convenience.)
The immediate motivation is to add APIs to programmatically construct
ECHConfigs. I'm thinking we can pass a const EVP_HPKE_KEY * to specify
the key, at which point it's weird for SSL_ECH_KEYS_add to look
different.
Bug: 275
Change-Id: I2d424323885103d3fe0a99a9012c160baa8653bd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48002
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This CL adds an initial implementation of the ECH server, with pieces of
the client in BoGo as necessary for testing. In particular, the server
supports ClientHelloInner compression with ech_outer_extensions. When
ECH decryption fails, it can send retry_configs back to the client.
This server passes the "ech-accept" and "ech-reject" test cases in
tls-interop-runner[0] when tested against both the cloudflare-go and nss
clients. For reproducibility, I started with the main branch at commit
707604c262d8bcf3e944ed1d5a675077304732ce and updated the endpoint's
script to pass the server's ECHConfig and private key to the boringssl
tool.
Follow-up CLs will update HPKE to the latest draft and catch us up to
draft-10.
[0]: https://github.com/xvzcf/tls-interop-runner
Bug: 275
Change-Id: I49be35af46d1fd5dd9c62252f07d0bae179381ab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45285
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
