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 02 23 #1452

Merged
merged 8 commits into from
Feb 28, 2024

Conversation

samuel40791765
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.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 87.50000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 76.90%. Comparing base (b8135dd) to head (4fcf0f4).

Files Patch % Lines
crypto/x509/rsa_pss.c 50.00% 2 Missing ⚠️
crypto/asn1/tasn_new.c 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1452      +/-   ##
==========================================
+ Coverage   76.87%   76.90%   +0.03%     
==========================================
  Files         425      425              
  Lines       71457    71462       +5     
==========================================
+ Hits        54930    54960      +30     
+ Misses      16527    16502      -25     

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

@samuel40791765 samuel40791765 marked this pull request as ready for review February 23, 2024 23:21
@samuel40791765 samuel40791765 requested a review from a team as a code owner February 23, 2024 23:21
torben-hansen
torben-hansen previously approved these changes Feb 26, 2024
static const ASN1_OBJECT *get_builtin_object(int nid) {
// |NID_undef| is stored separately, so all the indices are off by one. The
// caller of this function must have a valid built-in, non-undef NID.
BSSL_CHECK(nid > 0 && nid < NUM_NID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

funny that this probably must abort here... meh

dkostic
dkostic previously approved these changes Feb 26, 2024
@samuel40791765 samuel40791765 dismissed stale reviews from dkostic and torben-hansen via 541cff0 February 27, 2024 17:50
davidben and others added 8 commits February 27, 2024 09:51
This is never defined.

Change-Id: I1ecaa00f780d6b2f000dc67514c2f49eb4cf2a45
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63528
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 5e1496bf00a1bd740f5626aa4897b105a3561254)
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 <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 8a062a71124f84ed4c3ab304b75ee215b7439de3)
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 <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 26d84fdf024cf85e461aa10b59f9484699167533)
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 aws#412.

Bug: 412
Change-Id: Iba7f2869607e6361d6cb913416de21a19cdd6275
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63527
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit a014bdad2ad475ab3cfe5fb2bd20183b1a72ecd4)
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 <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
(cherry picked from commit 5d58c559ace6a24ea6613e412b26bd4c50668ab3)
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 <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 9e6144382ca4752591910b38b71a3301d97999df)
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 <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 39d7ee9c8262d9cd3338735bf3e95649857375e5)
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 <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit 3763efb56b5282cf92d71c259576352555c1a8f8)
@samuel40791765 samuel40791765 force-pushed the upstream-merge-2024-02-23 branch from 541cff0 to db36c3b Compare February 27, 2024 17:51
@samuel40791765 samuel40791765 merged commit 67cf4cc into aws:main Feb 28, 2024
38 of 39 checks passed
@samuel40791765 samuel40791765 deleted the upstream-merge-2024-02-23 branch February 28, 2024 19:13
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.

7 participants