Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream merge 2024-11-11 #1985

Merged
merged 4 commits into from
Nov 25, 2024
Merged

Conversation

andrewhop
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@andrewhop andrewhop requested a review from a team as a code owner November 12, 2024 00:31
@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.90%. Comparing base (85f58da) to head (fae4aee).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1985   +/-   ##
=======================================
  Coverage   78.89%   78.90%           
=======================================
  Files         595      595           
  Lines      102451   102454    +3     
  Branches    14525    14528    +3     
=======================================
+ Hits        80832    80840    +8     
+ Misses      20969    20963    -6     
- Partials      650      651    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@andrewhop andrewhop requested review from justsmth and removed request for nebeid November 14, 2024 23:03
rolandshoemaker and others added 4 commits November 25, 2024 12:50
Rather than using the pre-generated certificates, generate them on the
fly. This allows TLS stacks for which certificate validation and
verification are coupled to work as expected. Certificates and keys are
written to temporary files which are then passed to the shim, and
cleaned up on exit. This requires reworking how testCase passes
certs/keys by adding a new field, sendCertificate, rather than manually
setting the -cert-file and -key-file flags. Incidentally the
rsaChainCertificate is removed, since it was essentially unused, and all
tests that used it also work with rsaCertificate. Finally, include a
single SAN ("test") in all certificates, which fixes some TLS stacks
which require this to operate (such as rustls, which currently
regenerates all the certificates currently in the tree to add a SAN).

Additionally, add a new flag, -trust-cert, which tells the the shim
which certificates it should trust. Shims for TLS stacks which
can completely decouple validation and verification of X509 certificates
(like BoringSSL) can ignore this flag, but for stacks where this
functionality is somewhat more intertwined (like Go), this allows the
shim to properly process the sent certificates.

Change-Id: Ic5c63e18fb2b852cc693aacb3b06cfe7993bc90c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62565
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
(cherry picked from commit df3b58ea74c50ff785ab902be3b007ff008d3e3c)
This removes the need to ship the PEM files with the built runner.
Instead we can use go:embed to pick up the key files. We do, annoyingly,
need to write the Channel ID file to a temporary, but it's not a huge
deal. When/if we rework all this to JSON, we can avoid this.

Change-Id: Ie0d187a5396546dc157906430639c26b3cc59ca2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66627
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit 1e8461cc151960ad941ee7dd0e0bb13337e3c556)
This is passing in a different TLS version, but the TLS version is both
nonsense and doesn't figure into the delegated credential anyway. All
this test is doing is generating a different keypair and mixing them up.
Probably we should move it to ssl_test, as it's not really testing
anything about the protocol, but I've just left it alone and fixed the
test.

Also fix another issue in the test: the getSigner / signMessage chord
should just be a plain signMessage call. There were a few other issues
of that shape, but they'll be fixed in a follow-up change because they
reveal a deeper problem with
https://boringssl-review.googlesource.com/c/34884

Change-Id: I090b41a081f694b4ff8d97f3895645d6a620904d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66549
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 9f376b0694dfb8528fa2200369b48632563e972f)
RFC 9345 has this bizarre special case forbiding the rsaEncryption OID
for delegated credentials. This doesn't make much sense as DCs already
constrain to a single signature algorithm. In fact, they didn't need to
use SPKIs at all and could have just encoded the type-specific values.

Nonetheless, this is where the spec went up. We have long rejected the
RSASSA-PSS OID as being unusably complex, so this effectively means we
will never permit RSA delegated credentials.

This was another oversight in
https://boringssl-review.googlesource.com/c/34884. Fix it separately
before everything is reworked to SSL_CREDENTIAL.

Bug: 249
Change-Id: I7eae1e8da9da8052b8d985e78388ef8f2b235942
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66567
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit c9a9d8d5a90b55bea3ce019465821478e7036077)
@andrewhop andrewhop merged commit 412018d into aws:main Nov 25, 2024
116 of 119 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants