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 a85ef9aa6c6538e5ae014a408e9d69870bc4f404 #2397

Merged
merged 78 commits into from
Feb 18, 2025

Conversation

briansmith
Copy link
Owner

No description provided.

davidben and others added 30 commits January 2, 2025 09:15
We've since added a few more files that don't have a counterpart in
OpenSSL, or are named differently from their OpenSSL counterparts.

Change-Id: I7057d8b258cb9656924054022654359d11a164f8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74667
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
OpenSSL ran a "copyright consolidation" process, driven by this script.
We will need to modify it slightly because it sometimes sets the year
based on OpenSSL's git.

Initially, this checks in an unmodified copy of OpenSSL's script.

Bug: 364634028
Change-Id: I5acd518a900d9c5bb4ac637c5a3986ddf0b22bd0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74707
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
OpenSSL's "copyright consolidation" script standardizes their various
old copyright headers on a new one. In doing so, it recomputed the
copyright year as follows:

- The end year was always 2016, when they ran the script.
- If the file began with an EAY copyright line, that starting year was
  used.
- Otherwise, the start year was ignored and recomputed from version
  control.

This final step will not run in BoringSSL, because we started a new
history. Instead, modify the script to simply take the result of the
process from the corresponding file in OpenSSL.

Bug: 364634028
Change-Id: I6083a398c7d742210d1b67110dda755ba0509f6c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74708
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
We'll simply be picking up the copyright lines from upstream OpenSSL.
But since OpenSSL's script ended up changing the years around, this
script will let us check which start years changed. (End years in
OpenSSL got all rewritten to 2016 in the "copyright consolidation"
process, so there's nothing terribly interesting to check.)

Bug: 364634028
Change-Id: Id7263b05a98898fe6a6a121af9655ac0857c3ba2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74709
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This applies the OpenSSL "copyright consolidation" process from the
following upstream changes:

* openssl/openssl@e0a6519
* openssl/openssl@3fb2cf1
* openssl/openssl@ac3d0e1
* openssl/openssl@c2f312f
* openssl/openssl@596d6b7
* openssl/openssl@e18cf66
* openssl/openssl@846e33c
* openssl/openssl@440e5d8
* openssl/openssl@21dcbeb
* openssl/openssl@6286757
* openssl/openssl@4f22f40
* openssl/openssl@d2e9e32
* openssl/openssl@2039c42
* openssl/openssl@b132225
* openssl/openssl@aa6bb13
* openssl/openssl@b6cff31
* openssl/openssl@9e20068
* openssl/openssl@6aa36e8
* openssl/openssl@44c8a5e

This was mostly automated, but partially manual. The automated portion
can be reproduced by checking OpenSSL to commit
44c8a5e2b9af8909844cc002c53049311634b314, and running the following:

  git grep -l -E 'Copyright remains Eric Young|Copyright.*The OpenSSL Project\.|Written by.*for the OpenSSL Project' crypto/ decrepit/ include/ ssl/ | grep -v objects.go > files.txt
  cat files.txt | xargs -n1 perl -i ./util/copyright.pl

From there, some years were fixed up manually according to
go/openssl-copyright-consolidation-comparison (internal-only).

Three files required additional manual fixing:

- crypto/ecdh_extra/ecdh_extra.cc
- crypto/fipsmodule/ecdh/ecdh.cc.inc
- include/openssl/ecdh.h

These files have an OpenSSL header, but *after* a different header, so
the script does not correctly detect the now redundant OpenSSL header.
They were manually modified to remove it. This matches what seems to
have been done to crypto/ec/ecdh_ossl.c in OpenSSL's
4f22f40507fea3f272637eb8e00cadf1f34b10d9.

Bug: 364634028
Change-Id: I79a559a409ebe2476f2cb8a48a488ac5dd77c90a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74710
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
This allows the script to recreate the new header.

Bug: 364634028
Change-Id: Ie399e95f284b0170e8073e60f71806bf16cf48e2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74711
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This imports the following changes from OpenSSL:

* openssl/openssl@aa8f3d7
* openssl/openssl@5aba2b6
* openssl/openssl@c80149d

See the following links for some related discussion in OpenSSL's
repository:

* openssl/openssl#3663
* openssl/openssl#3684
* openssl/openssl#3585 (comment)
* openssl/openssl#3685

The copyright_summary script may be used to compare this CL. Note there
is one change to ecdsa_test.cc to align with OpenSSL. See
go/openssl-copyright-consolidation-comparison (internal-only).

Bug: 364634028
Change-Id: I987c4e145d2ccd0c32bbf9e7bb2cc69e89019d35
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74712
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This imports
openssl/openssl@624265c
from upstream OpenSSL. The only part that applies to BoringSSL is
x_spki.c.

Bug: 364634028
Change-Id: I709ed9765ee78ed35983384b0a472071a3cee4ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74713
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Now that the change has been applied, we no longer need these.

Bug: 364634028
Change-Id: I2979b62489d640807c6b2568227c015a05af4d4b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74767
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
We have some logic to implicitly define OPENSSL_STATIC_ARMCAP on some
platforms. This was being done a hair too late for the NEED_CPUID logic
in crypto/internal.h to pick up.

I'm not sure why this is only tripping the Android build now. It seems
to have been broken for a long while. I put it in the public headers
because <openssl/crypto.h> is also sensitive to OPENSSL_STATIC_ARMCAP,
so it seems prudent for it to be set all in one place.

Change-Id: I53691b018282a71f5d0cb0f6a6c457e1ee4d1df9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74787
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
These aren't used externally. While I'm here, const-correct
PEM_do_header. Really we could have just made these file-local except
that PEM_X509_INFO_read_bio does something weird.

Bug: 42290574
Change-Id: I455b9c31da0efb854925bbe38797d3c0e221fcdf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74807
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This imports upstream's
openssl/openssl@6714cb1

Bug: 364634028
Change-Id: I270390f9a0ab8acb5ec508e2c992ccf6b1091a07
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74768
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
There's a lot more we could improve in this function, but fix this
particular egregious issue first.

Change-Id: Idc4b7f9aa62972293ead4f8534b8461942318e21
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74808
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
This commit adjusts the ACVP.md documentation for the KDF-counter
command to match the implementation.

Prior to this the kdf struct in subprocess that dispatches KDF-counter
command invocations had a few divergences from the docs:

* If the test case has the "Deferred" property set to true, then the key
  argument provided to the wrapper is empty.
* The wrapper is expected to output three values: the input key (since
  for deferred tests it was generated module-side), the fixed counter
  data, and the derived key.

For deferred tests the returned key is written to the response `KeyIn`.
For non-deferred tests the returned key is verified to match the one
that was sent to the submodule as a command arg.

Change-Id: If266383e279d2222f55975aa3376e8fb134899d7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74727
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Add a VAES-optimized AES-GCM implementation that is optimized for AMD
Zen 3 processors, using AVX2 instead of AVX512 / AVX10.  With AVX2 only
16 vector registers are available and some instructions are missing,
which is inconvenient and makes the code not easily sharable with the
AVX512 / AVX10 version.  However, using VAES still gives a significant
performance improvement, about 80-85% on long messages as shown by the
following tables which show the change in AES-256-GCM throughput in MB/s
on a Zen 3 "Milan" processor for various message lengths in bytes.

Encryption:

            | 16384 |  4096 |  4095 |  1420 |   512 |   500 |
    --------+-------+-------+-------+-------+-------+-------+
    Before  |  3955 |  3749 |  3597 |  3054 |  2411 |  2038 |
    After   |  7128 |  6631 |  5975 |  4788 |  3807 |  2676 |

            |   300 |   200 |    64 |    63 |    16 |
    --------+-------+-------+-------+-------+-------+
    Before  |  1757 |  1405 |   856 |   602 |   356 |
    After   |  1885 |  1430 |   940 |   593 |   381 |

Decryption:

            | 16384 |  4096 |  4095 |  1420 |   512 |   500 |
    --------+-------+-------+-------+-------+-------+-------+
    Before  |  3962 |  3774 |  3593 |  2978 |  2510 |  1998 |
    After   |  7378 |  6836 |  6282 |  4826 |  3868 |  2753 |

            |   300 |   200 |    64 |    63 |    16 |
    --------+-------+-------+-------+-------+-------+
    Before  |  1742 |  1428 |   856 |   535 |   383 |
    After   |  1940 |  1534 |   940 |   573 |   383 |

Change-Id: I583dd6b48b81ab3c6df51bfe8729366cad500537
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74368
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Since there is now a VAES+AVX2 implementation of AES-GCM, and the future
of AVX10/256 is uncertain, disable the AES-GCM functions that use
AVX10/256 (equivalently AVX512 with a maximum vector length of 256
bits).  This leaves VAES+AVX2 as the sole 256-bit support for now.

For now this just affects Intel Ice Lake and Tiger Lake (which actually
support AVX512, but where downclocking issues make 256-bit arguably
preferable to 512-bit), where a slight performance loss is seen on long
messages.  The following tables compare AES-256-GCM throughput in MB/s
on Ice Lake server for various message lengths:

Encryption:

            | 16384 |  4096 |  4095 |  1420 |   512 |   500 |
    --------+-------+-------+-------+-------+-------+-------+
    Before  |  7533 |  6990 |  6220 |  5096 |  4200 |  2702 |
    After   |  7403 |  6879 |  6236 |  4980 |  4040 |  2868 |

            |   300 |   200 |    64 |    63 |    16 |
    --------+-------+-------+-------+-------+-------+
    Before  |  2086 |  1555 |  1031 |   657 |   433 |
    After   |  2069 |  1635 |  1045 |   667 |   430 |

Decryption:

            | 16384 |  4096 |  4095 |  1420 |   512 |   500 |
    --------+-------+-------+-------+-------+-------+-------+
    Before  |  7703 |  7140 |  6524 |  5283 |  4244 |  2990 |
    After   |  7572 |  7056 |  6494 |  5155 |  4224 |  3073 |

            |   300 |   200 |    64 |    63 |    16 |
    --------+-------+-------+-------+-------+-------+
    Before  |  2276 |  1733 |  1070 |   680 |   447 |
    After   |  2249 |  1743 |  1100 |   692 |   447 |

This change should be reconsidered if AVX10/256 sees widespread support,
as we shouldn't carry forward a restriction to AVX2 unnecessarily.

This change also replaces gcm_init_vpclmulqdq_avx10 with
gcm_init_vpclmulqdq_avx10_512, now instantiated using 512-bit vectors.
Otherwise it would be the only avx10 function left using 256-bit.

Change-Id: I7fd21568482118a2ce7a382e9042b187cd2739f7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74369
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This imports
openssl/openssl@0904e79
from upstream OpenSSL. The tree intentionally does not compile at this
point.

Bug: 364634028
Change-Id: I39001741cf0db059e76ad4940004a1d57bf8af12
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74827
Reviewed-by: Adam Langley <[email protected]>
…upstream OpenSSL

This imports
openssl/openssl@320a812
from upstream OpenSSL.

This causes the following free functions to no longer check for NULL:

* BIO_CONNECT_free
* BUF_MEM_free
* BN_CTX_free
* BN_RECP_CTX_free
* BN_MONT_CTX_free
* BN_BLINDING_free
* X509_STORE_free
* SSL_SESSION_free

(It also causes tls_free to no longer check for NULL, but that check was
unnecessary.)

Bug: 364634028
Change-Id: Ia625039a0a22b0bf368c39d6b8090ca15955f8e4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74828
Reviewed-by: Adam Langley <[email protected]>
…some functions.

This change reimplements some OpenSSL changes based only on the
description of the work in base.h.

Change-Id: I1a8b3d2774216c43ab446aa56b31cbb40d58b29d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74847
Reviewed-by: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Years computed from version control.

Bug: 364634028
Change-Id: I949149b156ea24966813f304699869a5ad304c98
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74888
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Bug: 364634028
Change-Id: Id26a236e3cc74944111f1ce74e32dbb481c4b309
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74829
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
The crate won't build without it anyway, so unconditionally
try and enable it.

Change-Id: Ief8a7dbf8d0af3040b1832424007150987ce654d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74947
Auto-Submit: Pete Bentley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
I think this was not supposed to be checked in. The generate scripts
tend to drop things extra files into an out directory, but for better or
worse, we don't actually end up checking them in. (They're not in the
corresponding files in Chromium.)

Probably these were lying around in the worktree at the time.

Change-Id: Ibc423ce316ecdf2b4b62a4513d228618ff88bc67
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74830
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This reverts commit d678fbf.

The original change broke Chromium:

```
error[E0725]: the feature `new_uninit` is not in the list of allowed features
  --> ../../third_party/boringssl/src/rust/bssl-crypto/src/lib.rs:28:12
   |
28 | #![feature(new_uninit)]
   |            ^^^^^^^^^^
```

Change-Id: I6732a720a5705cfe7b0219b5d9d02a911af0f89a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74987
Auto-Submit: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
In ASN1_TIME_to_generalizedtime we were already parsing the
input time string to check for validity at the start of the
function. Instead of throwing away the tm we construct to do this,
keep the tm, and just convert the time normally to what we want
after the check instead of messing with the original string.

Change-Id: I32b351abd135b1f9c30b228c09718c5be994187a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74848
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
davidben and others added 24 commits January 16, 2025 13:21
The foo / foo_extra split will make increasingly less sense as we stop
putting public APIs in crypto/fipsmodule. Just call it crypto/foo and
crypto/fipsmodule/foo.

Bug: 42290602
Change-Id: I5143d3edfb768ed7a1aa288ff606f6f13faa9278
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75151
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
…ributor that we cannot find" from upstream OpenSSL
…and `sprintf`, and handle NULL in some functions.
@briansmith briansmith self-assigned this Feb 17, 2025
Copy link

codecov bot commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.61%. Comparing base (4b6b02c) to head (7daa6fb).
Report is 79 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2397   +/-   ##
=======================================
  Coverage   96.61%   96.61%           
=======================================
  Files         176      176           
  Lines       21640    21640           
  Branches      529      529           
=======================================
  Hits        20908    20908           
  Misses        618      618           
  Partials      114      114           

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

@briansmith briansmith merged commit c7eaca4 into main Feb 18, 2025
174 checks passed
@briansmith briansmith deleted the b/bm-7-5 branch February 18, 2025 00:06
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