We should never mix them up with a context that expects
id-rsaEncryption. Right now it fails because we can't parse the
certificate. Later it will fail at a slightly different point.
Bug: 384818542
Change-Id: I64dc99a0099f6423ffa2686bede369b14b7544b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/80268
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is a test certificate library that is slightly more convenient to
build certificate chains. More importantly, it reimplements the
crypto/x509 serializer with x/crypto/cryptobyte.
This is a fair amount of work, but means we can add new key types that
Go does not support, notably RSA-PSS keys. This is to ensure that later
work to add RSA-PSS keys to libcrypto (but *not* libssl) will not
regress libssl. (All this work to test that we *don't* support something
in libssl.)
Bug: 384818542
Change-Id: I54cd264cf9e774fc38d8f8780becb61f886466a3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/80267
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This field is unused and already wasn't filled in with
garbageCertificate. Removing it also makes it more obvious that runner
does not actually care if it can parse its own certificate.
Change-Id: I788a6f6fe8784579d03c1c4023728b5ca77c3f88
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79911
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
When runner fails a test at the first connection, but the shim tries to
make a second connection before it is killed, the dispatcher doesn't
recognize the shim ID and we get a confusing message:
> Error dispatching connection: shim ID 55 not found
Fix this by remembering closed shim IDs and silently rejecting them.
Change-Id: Ic8afdd853da2ab3c9ef6d7102a5a0a7d52f905df
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79808
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>
P-224 is too small to meet our security requirements. It seems that
nothing is using this anymore, so remove it to avoid folks accidentally
turning it on when they don't mean to.
Update-Note: Attempting to configure P-224 in TLS will now fail. This
does not impact P-224 as a general cryptographic primitive. Note that
this was off by default, so unless your project was explicitly enabling
this, this will not impact you.
Change-Id: I7b931ef0f37e3fac87848a37c5173892442e4f4f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79427
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CL originally by Bob Beck.
We did not filter this list to things that would be usable in the
handshake. This allows the client to not bother retrying with a
credential that wouldn't be usable anyway.
Fixed: 402692373
Change-Id: I78850ada5014bfd18235cfe5463fa2973da91a30
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79188
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@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>
Although we only need a subset of draft-ietf-tls-tlsflags, go ahead and
implement helper functions good enough for response flags to get some
experience with the extension.
Change-Id: Iba1581686c9d1883439cfd6445e98801f8fad098
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77128
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Now runner.go contains only the test runner, while the various test
suites are moved into their own files, named foo_tests.go. (foo_test.go
would be treated as a Go test.)
I broadly just split by the addFooTests functions, but in a few cases I
grouped them together.
Now we no longer have a single 24,000 line file with all the tests. That
was getting unwieldy.
Change-Id: I76f372f60f5f0de5f1ba0913317918a4053372a3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77107
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
First, simplify the API a bit:
- Just take a boolean param rather than having both set and clear
functions.
- Unless we need it, no need to bother with a getter. We generally
assume that the caller knows what they configured.
Next, expand on the docs and move it with other credential APIs, not
SSL_PRIVATE_KEY_METHOD.
Finally, fix a bug and test this in runner: the TLS 1.2 handshake forgot
to check the issuer, which meant that it assumed all credentials were
viable. Fix this and add tests to cover it all. In doing so, this pulls
in the MustMatchIssuer runner plumbing out of
https://boringssl-review.googlesource.com/c/boringssl/+/73087 to land a
little sooner.
Also test that issuer matching works with delegated credentials. May as
well.
Change-Id: I22aee148dd81fb9804d80b4243b68a5ecdead480
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76708
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We had some extra logic for detecting empty certificate_authorities in a
funny way, but since this is a syntax error, runner can just check this
unconditionally and stick with a more straightforward data model.
Also individual CAs cannot be empty, so fix runner to enforce this.
Change-Id: I5e697687d6dcc44e36b4f98a6284889129a077a4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76707
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: SSL_VERIFY_PEER_IF_NO_OBC is removed. This was used as the
transition plan between the long-deprecated TLS Channel ID, and its
even-longer-deprecated precessor, Origin-Bound Certificates. Callers
should have no more reason to use this feature. (See also cl/728350196.)
Change-Id: I7a02e92592c4f71bed343935fdb094564701bd37
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76687
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
One of our environments is using a slightly older development snapshot
leading to Go 1.24, which seems to be slightly incompatible with the
final crypto/mlkem API. Until that gets updated, revert back to the
external module.
Change-Id: I5715a6800219dc0a42bca1022fdc992a8bcbdfa3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76327
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
This lets us fold X25519 into the ECDH bits, and drop
x/crypto/curve25519, but then means we need to carry a copy of the
crypto/elliptic version for P-224 because crypto/ecdh doesn't support
it.
Change-Id: Ie053679f26462c68c1d553de97e06afdb77c7eed
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76208
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Currently, trying to "go install" something from BoringSSL's Go module
fails because the proxy-reachable name is
boringssl.googlesource.com/boringssl.git, but the go.mod name is
boringssl.googlesource.com/boringssl.
$ go install -v boringssl.googlesource.com/boringssl.git/util/fipstools/acvp/acvptool@master
go: downloading boringssl.googlesource.com/boringssl.git v0.0.0-20250122182937-e056f59c7dfd
go: boringssl.googlesource.com/boringssl.git/util/fipstools/acvp/acvptool@master: version constraints conflict:
boringssl.googlesource.com/boringssl.git@v0.0.0-20250122182937-e056f59c7dfd: parsing go.mod:
module declares its path as: boringssl.googlesource.com/boringssl
but was required as: boringssl.googlesource.com/boringssl.git
Using boringssl.googlesource.com/boringssl fails because without the
.git hint, the go tool will fetch
https://boringssl.googlesource.com/boringssl/util/fipstools/acvp/acvptool?go-get=1
which is not implemented by gitiles.
Adding .git to the module name makes the first command work.
Change-Id: I6a6a4656a34fac424114a5d65d23df677ca7de47
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76107
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Notably Go and rules_cc, but also pick up the rest while I'm here.
Updating libc++ once again required reworking the config. They seem to
change it every couple of weeks.
Bug: 396087264
Change-Id: Ied0f6fa11cd8c34fe9f0f87e63fd6283a699a22c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76167
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Bob pointed out that the previous CL didn't quiiiite move the errors
around right. I looked at the ssl_parse_client_hello_with_trailing_data
calls but not the SSL_parse_client_hello calls. As a result, we doubled
up some errors.
I'd also missed that we already have SSL_R_CLIENTHELLO_PARSE_FAILED.
That said, which error to use is a little interesting. Some codepaths
used to use SSL_R_DECODE_ERROR and some used
SSL_R_CLIENTHELLO_PARSE_FAILED. Further complicating things is that some
ClientHello error paths are unreachable because only the first time a
ClientHello is parsed matters. But we have the second ClientHello in HRR
and inner ClientHellos to content with. (A TLS connection can have up to
four ClientHellos now!)
I've erred towards picking the more specific one, given this whole mess.
Update-Note: The error when the server cannot parse the ClientHello is
now a bit more specific. This might be visible to server-specific
logging, but will not change what is sent over the wire.
Change-Id: I64a4305968616a9f414d3c95fb4ffbd1cfdc4ecc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76147
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
These were temporary flags in case of compatibility issues with some
older changes, set to be removed after June 2024. It is now well past
June 2024 and no one ever had to use these APIs. Remove them.
Update-Note: Removed some unused APIs.
Change-Id: I5fe34e0ebcb30f81281e413017d5a6a968a96a97
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76127
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@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>
I was going to add a corresponding test for PAKEs and noticed we
neglected this for PSKs. This also fixes a bug in runner where client
auth + resumption handshakes as a server didn't work right. (I guess
none of our tests exercise this case.)
Change-Id: I9e82dcbca54aedba4059e45c3e40a39b390de34e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75667
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
I am not sure why we ran through this increasingly large block of code,
with side effects, twice. All this really needed was to send a second
HRR and make sure the client rejected.
Change-Id: I1122ef2c5f8f85e2f356a6112ae2042653469417
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75631
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@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>
I would be surprised if this were ever used by anyone, but we should
either make it work, or add code to turn the feature off when
configured. All the things that prevented it from working we just bugs,
so it was easier to just fix this and enable all the tests.
The main nuisance is how our in-memory representation reflects the old
DTLS 1.2 header, conflicting ECH's many uses of the transcript. (We
really should switch the in-memory representation.) Then some bits of
ClientHello processing needed to not lose track of the cookie.
(This test already covers HelloVerifyRequest interactions because we
run through HelloVerifyRequest in DTLS 1.2.)
Bug: 42290594
Change-Id: Ic09b78ddc0c5524ffc6c5b965eafab881eff0a0f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73729
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
This was originally looking for the client to specifically support
the final TLS 1.3 version 0x0304. This has a side effect of not picking
up DTLS 1.3, which has a different codepoint.
We did it this way because, early in TLS 1.3's development, we had draft
versions of TLS 1.3 flying around, and only the final TLS 1.3 can safely
ship enforcing this check.
Those draft versions are now gone, and this check is now getting in the
way of DTLS 1.3. Switch it to checking hs->max_version.
Bug: 42290594
Change-Id: Ic2d143af965b4b8bafef524f3f0e85cc3efa42fe
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73728
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
If client Finished gets through, but the server's responding ACK is
lost, the client will retransmit Finished (epoch 2). The server must
then process that retransmission and send another ACK.
However, the client may have since sent application data (epoch 3),
which the server may have since processed. Despite receiving data at
epoch 3, the server must keep epoch 2 open for some period of time to
accomodate this.
Unfortunately, there is no explicit signal in the protocol to do this.
The best we have is some guidance to be willing to do this for at least
2x MSL. See this discussion in the TLS mailing list:
https://mailarchive.ietf.org/arch/msg/tls/rof4SqkDrwU8o7WqSa9AO3YEUJI/
Implement this by keeping old epochs around for 2x MSL. This has the
side effect of also keeping old epochs around for a spell on KeyUpdate,
making us more resilient to packet reordering around KeyUpdate. For
simplicity, I've just get the same timeout for both, 2x MSL, or 4
minutes.
This opens a few cans of worms, because now record processing logic must
accomodate old epochs:
- The check against fragments from old epochs gets more complex.
- The check for maximum message size must move later; after the
handshake, the server has a tight maximum message size, but it must be
willing to receive (and skip over) retransmits from before the
handshake when the limit was higher.
- Application data is fine. We already, at a lower layer, forbid any
application data from coming in epochs 0 and 2.
- The ACK logic already did not make any particular assumptions here.
Now we'll process ACKs from older epochs for slightly longer, but we
already check that old epochs cannot ACK new epochs' messsages.
- ChangeCipherSpec is silly and would probably become moot once we start
rejecting CCS in DTLS 1.3, but I just made it drop old epochs CCS on
the floor to match the old behavior for now. (May as well avoid
relying too much on DTLS 1.2 keeping one epoch around at once.)
This also requires tweaking the test slightly. flushHandshake needs to
run after the application data keys are installed, or the callback
cannot send data. That, in turn, means OutEpoch() is no longer the right
epoch for sending an ACK so some of the tests need to be updated to
still pick up epoch 2. (What's going on is that sending ACKs happens
before you construct the flight, but we want to test reordering, so we
simulate it afterwards.)
Bug: 42290594
Change-Id: I199b304fa9c3d8d252d258ab8ff231e3b5688e2e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74027
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
For now, just have runner tolerate the PSK extension changing across
HVR. It's possible we'll want to change this, but I think we can resolve
that separately.
The tests required an additional fix to deal with a syntactic
impossibility in DTLS. (Also QUIC, if TLS 1.2 in QUIC existed.)
Bug: 42290594
Change-Id: Ia06975104933e049bcd2c62aa8a29fd74b20f474
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73827
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
In DTLS 1.2, message sequence number overflow was impossible. The
counter reset on every handshake, and every handshake had a bounded
number of messages. (The counter reset was actually problematic. The end
of one handshake and the start of the next shared epochs, so sequence
numbers become ambiguous. 1.3 fixes this by removing the reset but, in
hindsight, resetting on each epoch would probably have been better.)
In DTLS 1.3, there is no bound and overflow is possible. Check for
overflow. Somewhat annoyingly, because we tend to store the next
sequence number, we actually need 17 bits per sequence number to
represent this, if we want to accept up to the maximum possible message.
Test this on the read side with KeyUpdate. (To test this on the write
side, we must be able to send KeyUpdate.)
Bug: 42290594
Change-Id: I691855dc72427afb9e82d8de0fb2eeea6818f2ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73507
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
David Benjamin