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

Merge BoringSSL through 4d7b383e989eff619a60eb1da1b18f613af65daf #1650

Merged
merged 43 commits into from
Sep 24, 2023

Conversation

briansmith
Copy link
Owner

No description provided.

davidben and others added 30 commits June 23, 2022 23:28
Bug: 301
Change-Id: Idb558cd2a925e9c762369ec7cead08f7d1cec2eb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53093
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I521922842b3586ab7c6e242281188745a879f1df
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53094
Reviewed-by: Adam Langley <agl@google.com>
No sense in keeping two around. This does cause the functions to reject
some previously accepted invalid inputs. These were intentionally
accepted by
https://boringssl-review.googlesource.com/c/boringssl/+/13082 for
an old version of M2Crypto, but I belive we no longer need to be
compatible with that.

Update-Note: ASN1_TIME_print, ASN1_UTCTIME_print, and
ASN1_GENERALIZEDTIME_print will no longer accept various invalid inputs.

Change-Id: I4606d0b39585a19eb4b984ac809706e497a3f799
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53090
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Our free functions all tolerate NULL.

Change-Id: Ifcb3185c8d2f34afb83f7286c3463136edc926fa
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53125
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I8b1e4652d8d1445a6665af88d158e006ff3fdb20
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53185
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Since stdout and stderr aren't synchronised, grepping their combined
output can sometimes fail since the regexp is looking for the start of a
line. Better to discard the stdout, which always works.

Change-Id: I5d0331debfe4b3eddc628c1fecc9c6b83d462a7a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53106
Reviewed-by: David Benjamin <davidben@google.com>
Although not defined, older versions of certbot use numeric value 2,
which would be a hypothetical v3(2). See
certbot/certbot#9334. Accept these for
compatibility.

Bug: 467
Change-Id: I47cc64503569992595bdb42baa6333058d560242
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53229
Reviewed-by: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This reuses code we already have, and also avoids another union.

Bug: 301
Change-Id: I2f4ab564cf220818ca96d9c99a122c1e5cdc9e7a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53228
Reviewed-by: Adam Langley <agl@google.com>
This checks validity for UTF8String, BMPString, and UniversalString.

When we detach the core types from ASN1_ITEM, this will likely also be
reshuffled around, probably into type-specific functions. But for now
just get the checks in place.

Update-Note: Invalid strings in X.509 certificates and other ASN.1
structures will now be rejected. This change is less risky than it seems
because most strings in X.509 are in X509_NAME, which already rejected
invalid instances of these string types (but not other string types)
during canonicalization. See https://crbug.com/boringssl/412 for a
discussion of that mess.

Bug: 427
Change-Id: I0d7e24dfd841703d2a8581ec4e78ed5bc3862d75
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53225
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Switch to the CBS functions, which do all the checks together. This
resolves a TODO that ASN1_STRING_print_ex was inconsistently checking
for invalid codepoints. It also removes an optimization when
round-tripping UTF-8. This optimization was incorrect if the input was
invalid.

Finally, this removes UTF8_getc, which no longer has any callers.
(I've left UTF8_putc for now because CBB would force a malloc on every
character, even with CBB_init_fixed. We should either decide we don't
care, or make it possible to stack-allocate the cbb_buffer_st.)

Update-Note: This will make ASN1_STRING_print_ex newly fail, but such
inputs should be unreachable from the parser as of an earlier change.

Change-Id: I52d747c500c6f5f9ef659cdee3ef5d241f38ed21
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53226
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
ASN1_STRING_set saves having to manually manage the ASN1_STRING's
buffer. Also size that buffer correctly and align the free_s vs tmps
mismatch.

We can also assume OPENSSL_gmtime writes to the passed-in struct tm.
Every other caller already assumes this, and both POSIX (gmtime_r) and C
(gmtime_s) agree this is valid.

Finally, the cleanup logic is much simpler if we do all the time stuff
before handling the maybe-object-reuse bits.

Change-Id: I25befc8fa93a1a60246c6abcea6e5d58ee563b48
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53227
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
It seems no one has ever exercised this mode to notice it doesn't work.
Add a test for this, which catches the memory leak. See also
openssl/openssl#18677

Change-Id: I8890febc71decd553a040bdda8739f68ed616c39
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53285
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
https://github.com/google/googletest/blob/main/docs/advanced.md#distributing-test-functions-to-multiple-machines

Change-Id: I3998bb4735800a645286728852cb8494b28a6577
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51225
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Types which do not contain an X509_NAME can be const. X509_NAME still
requires some thought. (i2d_X509_NAME can mutate the object and isn't
even thread-safe when modified.)

Bug: 407
Change-Id: Iceafa2b4ea9c4194cfcc3044d90393b5d91f7c11
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53305
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
We have no X509_REQ_up_ref function, and neither does OpenSSL. That
means this reference count is unreachable, so remove it.

Change-Id: I6a0899b8fcb0716d47c89174b561e90d99a9b54b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53306
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
I found no uses of this. If we have to put it back, it's no big deal,
but one less group of functions to have to document. (SEQUENCE OF
AlgorithmIdentifier is mostly a weird PKCS#7 thing.)

Update-Note: X509_ALGORS is removed. If someone was relying on it, we
can re-export it.

Change-Id: I2b8e8d0d1d56d2debf99687023bc3621e92f6b08
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53307
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
CMake 3.8 was released April 10, 2017, which puts it past our five
year horizon. (CMake 3.9 is just a few days short of it, so using 3.8
for now.) In particular, depending on 3.7+ gets us some nice features
like VERSION_GREATER_EQUAL.

Change-Id: I90457ad41e7add3c6b2dd3664da964c4b6ede499
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53345
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This only does a few of them for now, in preparation for the following
CL, which tries to group things into sections.

Bug: 426
Change-Id: I4604d458ff2d8e81c8c8f0361a519e5291b8e119
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53308
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
In particular, since mutating X509s (and X509_CRLs and X509_REQs) does
confusing things, I've split the mutation functions into their own
section, which discusses constructing new objects.

Bug: 426
Change-Id: I1f7dbc5e47f78433e34c74f0cd966803a213e59a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53309
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OpenSSL calls the timestamp on the CRL lastUpdate, but it's actually
called thisUpdate in RFC 5280.

Bug: 426
Change-Id: I8a34f3de24e1914eda17d300321febe0205b2ec1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53310
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
The X509_NAME representation is a little odd because it flattens the
RDN sequence, but maintains RDN indices (X509_NAME_ENTRY_set) to recover
the information. add_entry and delete_entry both have logic to maintain
invariants. Test that they work.

Change-Id: I7d4a033db0a82a1f13888bbfca29076b589cc214
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53325
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Unfortunately, these functions are not const-correct, even the
accessors. Document what they should have been, especially since
mutating an X509_NAME_ENTRY directly won't even update the 'modified'
bit correctly.

Do a pass at adding consts to our code internally, but since the
functions return non-const pointers, this isn't checked anywhere. And
since serializing an X509_NAME is not always thread-safe, there's a
limit to how much we can correctly mark things as const.

Bug: 426
Change-Id: Ifa3d8bafb5396fbe7b91416f234de4585284c705
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53326
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OpenSSL has a large exported API surface for exporting the policy tree
out of an X509_STORE_CTX. As far as I can tell, no one uses any of these
APIs. Remove them.

Update-Note: It is no longer possibly to see the policy tree after an
X.509 verification. As far as we can tell, this feature is unused.

Change-Id: Ieab374774805e90106555ce4e4155f8451ceb5b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53327
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This was originally added for the openssl command-line utility and
otherwise is not very useful. I found no callers, so drop it.

Update-Note: An unused function was removed.
Change-Id: I12aa314fd3d8f1dad79eb5a07e0dea662dd9b4a8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53328
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: Suite B flags in the X.509 stack are no longer supported.
This isn't expected to affect anything but bindings wrapping unused
options.

Change-Id: Ia0770e545d34e041ab995e80ea11b4dd4a5e47ef
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53329
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
It doesn't hash the SPKI, just the BIT STRING in it.

Bug: 426
Change-Id: Ib5b8f14f530cc8045a60ac1e9780779d90f5a3bf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53330
Commit-Queue: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
There's a lot of these, so toss them into a corner somewhere.

I've added a note to note to discourage using them with memory BIOs.
Memory BIOs are, by far, the most common use I've seen for i2d_*_bio and
d2i_*_bio. It works, but it's unnecessary.

Bug: 426
Change-Id: I8645207a4ac9f1223d0739b5351c99a55400195f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53331
Commit-Queue: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
I can't find any users of these APIs. If we remove them, the only
publicly exposed operation on PKCS8_PRIV_KEY_INFO becomes
EVP_PKCS82PKEY. We can then parse it without a dependency on the legacy
ASN.1 stack.

While I'm here, remove the callback on the structure. OPENSSL_free
automatically calls OPENSSL_cleanse in BoringSSL, so the call is
redundant.

Update-Note: Removed some unused accessors.
Change-Id: I400748463abe3c28dfa42ae9de9be59cb76cd2b2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53332
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Bug: 407
Change-Id: I973e0cfe636fb0cdef211b078503cce5df5293b6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53333
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Although it contains an X509_NAME (by way of GENERAL_NAME), the
GENERAL_NAME field does not participate in serialization, so it is
actually const.

Bug: 407
Change-Id: I299815789744597e2b355cb0b996ba90c3b6a72f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53334
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
davidben and others added 11 commits July 15, 2022 20:08
Some don't have sections to go with yet, but will later.

Bug: 426
Change-Id: I903a8bf8c33cdc026a79601a8fd37469c839fa00
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53335
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Some of these were non-const because dup functions weren't
const-correct, but they are now. Once nuisance is the accessors. Ideally
they'd return non-const pointers, but that'll break OpenSSL consumers.

Bug: 407
Change-Id: I52b939a846b726d1d84dd2d5fdf71a7a7284d49e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53336
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
I haven't done the i2d/d2i functions yet, since we haven't talked about
how extensions can be known to the library. Also X509_REVOKED still
needs a home.

Bug: 407
Change-Id: I19fb600ccfda5528728849a42a957803b350b5c5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53337
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Our TLS 1.3 stack predates OpenSSL's. We chose TLS1_CK_* to align with
the existing names. OpenSSL made a new convention, TLS1_3_CK_*. Match
them.

This means that, in the likely event that TLS 1.4 uses the same
constants, they'll have weird names, just as several of our constants
still say SSL3_* but it doesn't particularly matter.

Change-Id: I97f29b224d0d282e946344e4b907f2df2be39ce1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53425
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Our TLS 1.3 stack predates OpenSSL's. We chose TLS1_TXT_* to align with
the existing names. OpenSSL made a new convention, TLS1_3_RFC_*. Match
them.

Similar to 53425

Change-Id: I8737d98c9c1d5c201c4726739ddcbe96123d9370
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53445
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I88bf98f0149e12598839a116f064d7dcfd5c860b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53385
Auto-Submit: Adam Langley <alangley@gmail.com>
Reviewed-by: David Benjamin <davidben@google.com>
Support is still needed for ACVP until 2023

Change-Id: Ia131a85bc06e7c61c823f1b3c021e2625a8769c4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53465
Reviewed-by: Corie Pressley <cpressley@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
CPython uses this function. Our implementation is slightly weird since
it leaks the clamping behavior, but probably not a big deal.

Update-Note: When this is merged into the internal repository, we can
simplify the CPython patches.

Change-Id: I291ddf852fb463bf02998fe04d0d0e8cb358dc55
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53485
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Nothing calls this function, it doesn't support most key types, and
accesses pkey.rsa without checking the type. Just remove it.

Change-Id: I073dfe74c545c7e08578b85105c88a19bbddf58a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53505
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
…rnal.h.

These are needed for the next merge from BoringSSL.
@briansmith briansmith self-assigned this Sep 23, 2023
@codecov
Copy link

codecov bot commented Sep 23, 2023

Codecov Report

Merging #1650 (65fb8b8) into main (6e9f1b7) will decrease coverage by 0.14%.
The diff coverage is 38.46%.

@@            Coverage Diff             @@
##             main    #1650      +/-   ##
==========================================
- Coverage   92.23%   92.09%   -0.14%     
==========================================
  Files         132      132              
  Lines       18849    18879      +30     
  Branches      196      196              
==========================================
+ Hits        17385    17387       +2     
- Misses       1428     1456      +28     
  Partials       36       36              
Files Changed Coverage Δ
crypto/fipsmodule/aes/aes_nohw.c 0.00% <0.00%> (ø)
crypto/internal.h 49.00% <28.88%> (-16.46%) ⬇️
crypto/poly1305/poly1305_vec.c 96.11% <100.00%> (-0.08%) ⬇️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@briansmith briansmith merged commit ad59665 into main Sep 24, 2023
265 of 267 checks passed
@briansmith briansmith deleted the b/merge-boringssl-10 branch September 24, 2023 03:28
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.

2 participants