first is 1 if and only if min_next_nonce is non-zero, so we don't need
to waste the 8 bytes keep track of it.
(Note 564 in evp_aead_ctx_st_state was actually 568 because of
alignment. Going down to 560 reduces it by 8 bytes.)
Change-Id: I7156f317e4ccd227a39732360e421ff7e55cc611
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78487
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
When we have both crypto/foo/internal.h and
crypto/fipsmodule/foo/internal.h, it's important that we put FIPSMODULE
into the header guard.
At that point, we probably should just put CRYPTO in there too, and
indeed we have been putting it in a few places. Go ahead and rewrite
them all with some clever shell scripting.
Change-Id: If61586f1d49ed477023530c36e9ed74373d4d1e6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77727
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@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>
The referenced bug notes some ODR warnings in an unspecified
configuration. (Which I can't get AppleClang or GCC 14 to reproduce, at
least.) This is likely due to us switching to C++. The ODR rules in C++
are a little different, but also perhaps the detection is only kicking
in for C++.
Either way, structs in .cc files are generally not intended to leave
that compilation unit. (Unless it's an opaque struct listed in base.h.)
There's no such thing as `static struct` so this change wraps many
structs in .cc files in `namespace {`. There are also lots of structs in
test file. Those are less concerning, but test files should mostly
entirely be in a namespace so this change does that where possible for
test files containing structs.
Bug: 384186552
Change-Id: I6ccf715fbcdc3ea6260b5d5d05f305182b1a9450
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74407
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
https://boringssl-review.googlesource.com/c/boringssl/+/74268 broke the
SSSE3 GHASH fallback because it no longer maintained alignment of
Htable. We had been relying on the memcpy to copy Htable into something
aligned.
Maintaining the alignment requirement without the memcpy is kind of a
nuisance because it now leaks into EVP_AEAD_CTX. Since we don't have a
good way to make caller-allocatable structs aligned, it would mean
allocating 15 extra bytes and then finding the right position.
Benchmarks shows that the alignment makes no difference on a Intel(R)
Xeon(R) Gold 6154 CPU @ 3.00GHz. Of course, this is artificial because
that CPU would never run this code anyway.
I recall adding the alignment requirement because it gave a bit of a
perf boost on the old Mac Mini 2010 I was testing against, which
actually is a CPU that would run it. I was able to dig it up, but
apparently I no longer have a keyboard that's compatible with it. (That
machine is also long EOL and cannot even run Chrome's minimum macOS
version. Although its CPU may be representative for older Windows.)
Regardless, I don't think it makes sense to expend this complexity for
this. (See internal Chrome UMA Net.QuicSession.PreferAesGcm on Windows
for the percentage of Windows that would be running this code. Though
they should also be using ChaCha20-Poly1305 anyway.)
Bug: 42290477
Change-Id: I4ef8c636bfc18200869f011ea50cc5d4988244ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74327
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
AES_KEY was separate from GCM128_KEY because crypto/modes was originally
written as if there were non-AES 128-bit block ciphers to worry about.
This has long since stopped being the case for BoringSSL. This avoids
some duplicate key setup logic in EVP_CIPHER and EVP_AEAD.
GCM128_KEY was embedded into GCM128_CONTEXT because OpenSSL assembly
once relied on the exact order of a bunch of the fields, some of which
we per-operation and some of which were per-key. See
https://boringssl-review.googlesource.com/c/boringssl/+/13122
This assumption has since been removed. See
https://boringssl-review.googlesource.com/c/boringssl/+/59526
Now that that is done, we can pull it out and instead pass it in as a
separate pointer, like how AES_KEY used to. (I made the key the first
argument because key than op seems more natural to me than op than key.
Also I didn't noticed I'd flipped them until I was done with the CL.) By
pulling it out, we avoid a pointless 500-ish byte memcpy before every
AES-GCM operation, which actually speeds up shorter inputs
non-trivially. See below. A nice bonus to cleaner code.
This doesn't do all the things in crbug.com/382503563, but gets us
partway towards it. It also gets the GCM128_* functions slightly closer
to being a self-contained abstraction.
As part of this, move aes_ctr_set_key into crypto/fipsmodule/aes instead
of crypto/fipsmodule/cipher. It was previously in cipher because it
depended on modes and modes already depends on aes, but now the GCM
dependency is moved out, I think aes makes more sense for it.
On an Intel(R) Xeon(R) Gold 6154 CPU @ 3.00GHz
Before:
Did 22847000 AES-128-GCM (16 bytes) seal operations in 2000070us (182.8 MB/sec)
Did 13704000 AES-128-GCM (256 bytes) seal operations in 2000025us (1754.1 MB/sec)
Did 5856000 AES-128-GCM (1350 bytes) seal operations in 2000107us (3952.6 MB/sec)
Did 1323000 AES-128-GCM (8192 bytes) seal operations in 2001392us (5415.2 MB/sec)
Did 683000 AES-128-GCM (16384 bytes) seal operations in 2000266us (5594.4 MB/sec)
Did 20866750 AES-256-GCM (16 bytes) seal operations in 2000005us (166.9 MB/sec)
Did 12114000 AES-256-GCM (256 bytes) seal operations in 2000109us (1550.5 MB/sec)
Did 4572000 AES-256-GCM (1350 bytes) seal operations in 2000142us (3085.9 MB/sec)
Did 972000 AES-256-GCM (8192 bytes) seal operations in 2000988us (3979.3 MB/sec)
Did 497000 AES-256-GCM (16384 bytes) seal operations in 2000832us (4069.7 MB/sec)
After:
Did 25786000 AES-128-GCM (16 bytes) seal operations in 2000050us (206.3 MB/sec) [+12.9%]
Did 14489000 AES-128-GCM (256 bytes) seal operations in 2000004us (1854.6 MB/sec) [+5.7%]
Did 5927000 AES-128-GCM (1350 bytes) seal operations in 2000248us (4000.2 MB/sec) [+1.2%]
Did 1316000 AES-128-GCM (8192 bytes) seal operations in 2000236us (5389.7 MB/sec) [-0.5%]
Did 679000 AES-128-GCM (16384 bytes) seal operations in 2001792us (5557.4 MB/sec) [-0.7%]
Did 23180500 AES-256-GCM (16 bytes) seal operations in 2000016us (185.4 MB/sec) [+11.1%]
Did 12703000 AES-256-GCM (256 bytes) seal operations in 2000070us (1625.9 MB/sec) [+4.9%]
Did 4668000 AES-256-GCM (1350 bytes) seal operations in 2000238us (3150.5 MB/sec) [+2.1%]
Did 976000 AES-256-GCM (8192 bytes) seal operations in 2000115us (3997.5 MB/sec) [+0.5%]
Did 500000 AES-256-GCM (16384 bytes) seal operations in 2001380us (4093.2 MB/sec) [+0.6%]
The difference is even more pronounced on GCC:
Before:
Did 19500000 AES-128-GCM (16 bytes) seal operations in 2000077us (156.0 MB/sec)
Did 12833000 AES-128-GCM (256 bytes) seal operations in 2000040us (1642.6 MB/sec)
Did 5544000 AES-128-GCM (1350 bytes) seal operations in 2000325us (3741.6 MB/sec)
Did 1305000 AES-128-GCM (8192 bytes) seal operations in 2000029us (5345.2 MB/sec)
Did 677000 AES-128-GCM (16384 bytes) seal operations in 2002554us (5538.9 MB/sec)
Did 18222000 AES-256-GCM (16 bytes) seal operations in 2000026us (145.8 MB/sec)
Did 11351750 AES-256-GCM (256 bytes) seal operations in 2000036us (1453.0 MB/sec)
Did 4431000 AES-256-GCM (1350 bytes) seal operations in 2000278us (2990.5 MB/sec)
Did 965000 AES-256-GCM (8192 bytes) seal operations in 2000617us (3951.4 MB/sec)
Did 497000 AES-256-GCM (16384 bytes) seal operations in 2003070us (4065.2 MB/sec)
After:
Did 25878000 AES-128-GCM (16 bytes) seal operations in 2000001us (207.0 MB/sec) [+32.7%]
Did 14510250 AES-128-GCM (256 bytes) seal operations in 2000034us (1857.3 MB/sec) [+13.1%]
Did 5936000 AES-128-GCM (1350 bytes) seal operations in 2000273us (4006.3 MB/sec) [+7.1%]
Did 1314000 AES-128-GCM (8192 bytes) seal operations in 2000033us (5382.1 MB/sec) [+0.7%]
Did 677000 AES-128-GCM (16384 bytes) seal operations in 2002827us (5538.2 MB/sec) [-0.0%]
Did 23281000 AES-256-GCM (16 bytes) seal operations in 2000048us (186.2 MB/sec) [+27.8%]
Did 12750000 AES-256-GCM (256 bytes) seal operations in 2000008us (1632.0 MB/sec) [+12.3%]
Did 4685000 AES-256-GCM (1350 bytes) seal operations in 2000205us (3162.1 MB/sec) [+5.7%]
Did 977000 AES-256-GCM (8192 bytes) seal operations in 2001842us (3998.1 MB/sec) [+1.2%]
Did 501000 AES-256-GCM (16384 bytes) seal operations in 2003419us (4097.2 MB/sec) [+0.8%]
Bug: 382503563, 42290602
Change-Id: I4690b79212242084cbcde49aa59979344012e5f6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74268
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
There were two places where the ctr32_encrypt_blocks function could be
missing:
1. In the AES-CTR EVP_CIPHER, with aes_nohw, we forgot to fill it in but
we actually provide it. We don't care enough about AES-CTR to
benchmark it, but I expect this to be a significant perf win because
aes_nohw is bitsliced.
2. The 32-bit x86 vpaes implementation did not have a
ctr32_encrypt_blocks. Just implement it in C.
This allows us to delete a ton of code for a broadly unreachable
codepath.
32-bit x86 is not a particularly important target in 2024, but just to
confirm, here is the impact on AES-GCM on an Intel(R) Xeon(R) Gold 6154
CPU @ 3.00GHz, running in 32-bit mode and with
OPENSSL_ia32cap='~0x200000000000000'
Before:
Did 5587000 AES-128-GCM (16 bytes) seal operations in 1000098us (5586452.5 ops/sec): 89.4 MB/s
Did 1063000 AES-128-GCM (256 bytes) seal operations in 1000036us (1062961.7 ops/sec): 272.1 MB/s
Did 225000 AES-128-GCM (1350 bytes) seal operations in 1001725us (224612.5 ops/sec): 303.2 MB/s
Did 39000 AES-128-GCM (8192 bytes) seal operations in 1014011us (38461.1 ops/sec): 315.1 MB/s
Did 19127 AES-128-GCM (16384 bytes) seal operations in 1009808us (18941.2 ops/sec): 310.3 MB/s
Did 4742000 AES-256-GCM (16 bytes) seal operations in 1000051us (4741758.2 ops/sec): 75.9 MB/s
Did 820000 AES-256-GCM (256 bytes) seal operations in 1000199us (819836.9 ops/sec): 209.9 MB/s
Did 171000 AES-256-GCM (1350 bytes) seal operations in 1000656us (170887.9 ops/sec): 230.7 MB/s
Did 30000 AES-256-GCM (8192 bytes) seal operations in 1034187us (29008.3 ops/sec): 237.6 MB/s
Did 15000 AES-256-GCM (16384 bytes) seal operations in 1031233us (14545.7 ops/sec): 238.3 MB/s
After:
Did 5314000 AES-128-GCM (16 bytes) seal operations in 1000040us (5313787.4 ops/sec): 85.0 MB/s
Did 1035000 AES-128-GCM (256 bytes) seal operations in 1000919us (1034049.7 ops/sec): 264.7 MB/s
Did 220000 AES-128-GCM (1350 bytes) seal operations in 1001498us (219670.9 ops/sec): 296.6 MB/s
Did 37000 AES-128-GCM (8192 bytes) seal operations in 1009332us (36657.9 ops/sec): 300.3 MB/s
Did 19000 AES-128-GCM (16384 bytes) seal operations in 1018435us (18656.1 ops/sec): 305.7 MB/s
Did 4512250 AES-256-GCM (16 bytes) seal operations in 1000047us (4512037.9 ops/sec): 72.2 MB/s
Did 803000 AES-256-GCM (256 bytes) seal operations in 1000917us (802264.3 ops/sec): 205.4 MB/s
Did 168000 AES-256-GCM (1350 bytes) seal operations in 1002181us (167634.4 ops/sec): 226.3 MB/s
Did 29000 AES-256-GCM (8192 bytes) seal operations in 1033945us (28047.9 ops/sec): 229.8 MB/s
Did 15000 AES-256-GCM (16384 bytes) seal operations in 1055312us (14213.8 ops/sec): 232.9 MB/s
It's a bit slower, but comparable. Any affected device should really be
served ChaCha20-Poly1305 anyway. (Of course, this is entirely
unscientific because this code would never run on anything remotely like
this CPU.)
Fixed: 383994657
Change-Id: Ifca6a6195b20497dc7053d736c61b9000d3074f9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74267
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@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>
This change switches nearly all of BoringSSL to use C++. The public
functions still use the C ABI, and so code written in C can still use
BoringSSL. Also the use of the C++ standard library is minimal and no
run-time requirement for it is intended.
Change-Id: I902d2f51a3c8d6bd0dc4aabe1b192a15d7b788e8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72747
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
I briefly thought we would need to look at this for DTLS, but DTLS
doesn't have the TLS 1.0 implicit IV bug. Add some basic tests to ensure
it doesn't crash as we rework the record layer.
Also add a missing error code in the implementation.
Bug: 42290594
Change-Id: I16e4dfb85aa1b84c38fd2433e01499347b8bf8d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71487
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Having the files named .c but included causes a ton of problems with
build systems. Many of our build systems care about three categories of
files:
- public headers, available to downstream targets
- source files, each of which is compiled as a compilation unit
- internal headers, not available to downstream targets
There is usually a check, in Bazel called layering_check that enforces
source files only include headers that are declared somewhere
appropriate. The bcm.c fragments, under this classification, are
internal headers.
However, in both GN and Bazel, internal headers and sources
both go in the source list. They are distinguished only by file
extension. When FIPS fragments have a .c file extension, they are
misinterpreted as source files, and many things break.
Rename them. Either .h and .inc would be sufficient. Because we had to
disable Bazel's parse_headers feature, there is no difference (AFAICT)
in their handling. Also, these files actually pass the parse_headers
feature, even though they don't have an include guard. Still, the tech
of the style guide suggests that .inc is probably the better file
extension.
https://google.github.io/styleguide/cppguide.html#Self_contained_Headers
I used .c.inc rather than plain .inc so that we can easily
rename them back to .c when we solve https://crbug.com/362530616.
Note that, as .inc is not as common of a file extension, people working
on BoringSSL may need to reconfigure their editors to map .inc to C/C++.
Update-Note: Some downstream builds have been working around this by
building the fragments individually and excluding bcm.c. This change
will break those workarounds but also remove the need for it. It should
now be consistently possible to build BoringSSL without modifying the
file list.
Bug: 362664827
Change-Id: I933115c37843317a066e24a1092728c9afce35f5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70689
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
BCM uses only passive entropy, in that from BCM's
point of view, entropy is requested and obtained
from an external source, and fed to BCM's CTR_DRBG
as required. With this change entropy is always gathered
in an OS specific manner outside of BCM by libcrypto,
while the CTR_DRBG remains in BCM using that entropy.
BCM functions (but not yet the tests!) now no longer
use the public RAND_bytes function, but instead use
BCM_rand_bytes which uses the BCM module CTR_DRBG.
BCM_rand_bytes is in turn used by libcrypto to implement
the public RAND_bytes function. All public RAND_
functions are now implemented in rand_extra.
As part of this two new headers are introduced to
start defining the interface boundary between libcrypto
and BCM.
crypto/bcm_support.h <- Functions implemented by libcrypto
and used by bcm
fipsmodule/bcm_interface.h <- Functions implemented by
bcm and used by libcrypto.
Bug: 723
Change-Id: I6b618dfe4df257f67971e88cbd79126c837e21d6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68147
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
OpenSSL added a similar helper function. It's very, very common for us
to malloc something an then zero it. This saves some effort. Also
replace some more malloc + memcpy pairs with memdup.
Change-Id: I1e765c8774a0d15742827c39a1f16df9748ef247
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63345
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Previously, EVP_CIPHER_CTX_iv_length always returned the cipher's fixed IV length. Now, after modification with EVP_CTRL_AEAD_SET_IVLEN, it returns the correct value.
Fixed: 626
Change-Id: Id98c929439850b3e83a80111f35aabebc6e5d47a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62907
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Remove all the other ERR_R_MALLOC_FAILURES from the
codebase.
Also changes cbb to push to the error stack, to correctly
report cbb failures instead of now only reporting
malloc failures. Previously it turned all cbb failures
into a malloc failure
Bug: 564
Change-Id: Ic13208bf9d9aaa470e83b2f15782fc94946bbc7b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57046
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We no longer have a need to support ppc64le, nor do we have any testing
story for the assembly we previously had. Remove all ppc64le-specific
assembly.
This CL stops short of removing it from base.h. That'll be done in a
follow-up CL, just to separate which removals are for the assembly and
which removals remove all support.
Update-Note: After this change, ppc64le builds drop assembly
optimizations and will fallback to a generic C-based AES implementation.
Change-Id: Ic8075638085761d66cebc276eb16c4770ce03920
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56388
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Poison the EVP_CIPHER_CTX structure on failures, and indicate
that it is an error to re-use an EVP_CIPHER_CTX context in another
call after a failure.
Bug: 494
Change-Id: Ibcdf28b83a2e690f7aab789d908c076d844231c6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54185
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The C11 change has survived for three months now. Let's start freely
using static_assert. In C files, we need to include <assert.h> because
it is a macro. In C++ files, it is a keyword and we can just use it. (In
MSVC C, it is actually also a keyword as in C++, but close enough.)
I moved one assert from ssl3.h to ssl_lib.cc. We haven't yet required
C11 in our public headers, just our internal files.
Change-Id: Ic59978be43b699f2c997858179a9691606784ea5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53665
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
I don't think these are all UB by C's rules, but it's easier not to
think about the pointers. Still more to go, but these were some easy
ones.
Bug: 301
Change-Id: Icdcb7fb40f85983cbf566786c5f7dbfd7bb06571
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52905
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
If we're to have any hope of fixing EVP_CIPHER_CTX's calling convention, we
need to be able to change the shape of its method table.
Looking back, it looks like we exported this in
https://boringssl-review.googlesource.com/4330, for OpenSSH. I don't
remember exactly what OpenSSH was doing, but I see in this commit, they
removed a bunch of custom EVP_CIPHERs which would definitely have
required an exported EVP_CIPHER struct:
https://github.com/openssh/openssh-portable/commit/cdccebdf85204bf7542b7fcc1aa2ea3f36661833
That's been gone for a while now, so hopefully we can hide it again. (If
a project needs a cipher not implemented by OpenSSL, it's not strictly
necessarily to make a custom EVP_CIPHER. It might be convenient to reuse
the abstraction, but you can always just call your own APIs directly.)
Update-Note: EVP_CIPHER is now opaque. Use accessors instead.
Bug: 494
Change-Id: I9344690c3cfe7d19d6ca12fb66484ced57dbe869
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52725
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
These symbols were not marked OPENSSL_EXPORT, so they weren't really
usable externally anyway. They're also very sensitive to various build
configuration toggles, which don't always get reflected into projects
that include our headers. Move them to crypto/internal.h.
Change-Id: I79a1fcf0b24e398d75a9cc6473bae28ec85cb835
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50846
Reviewed-by: Adam Langley <agl@google.com>
The non-_ex EVP_CIPHER_CTX Final functions are a bit interesting. Unlike
EVP_DigestFinal(_ex), where the non-_ex version calls EVP_MD_CTX_cleanup
for you, the EVP_CIPHER_CTX ones do not automatically cleanup.
EVP_CipherFinal and EVP_CipherFinal_ex are identical in all releases
where they exist.
This appears to date to OpenSSL 0.9.7:
Prior to OpenSSL 0.9.7, EVP_MD_CTX and EVP_CIPHER_CTX did not use void*
data fields. Instead, they just had a union of context structures for
every algorithm OpenSSL implemented.
EVP_MD_CTX was truly cleanup-less. There were no EVP_MD_CTX_init or
EVP_MD_CTX_cleanup functions at all. EVP_DigestInit filled things in
without reference to the previous state. EVP_DigestFinal didn't cleanup
because there was nothing to cleanup.
EVP_CIPHER_CTX was also a union, but for some reason did include
EVP_CIPHER_CTX_init and EVP_CIPHER_CTX_cleanup. EVP_CIPHER_CTX_init
seemed to be optional: EVP_CipherInit with non-NULL EVP_CIPHER similarly
didn't reference the previous state. EVP_CipherFinal did not call
EVP_CIPHER_CTX_cleanup, but EVP_CIPHER_CTX_cleanup didn't do anything.
It called an optional cleanup hook on the EVP_CIPHER, but as far as I
can tell, no EVP_CIPHER implemented it.
Then OpenSSL 0.9.7 introduced ENGINE. The union didn't work anymore, so
EVP_MD_CTX and EVP_CIPHER_CTX contained void* with allocated
type-specific data. The introduced EVP_MD_CTX_init and
EVP_MD_CTX_cleanup. For (imperfect!) backwards compatibility,
EVP_DigestInit and EVP_DigestFinal transparently called init/cleanup for
you. EVP_DigestInit_ex and EVP_DigestFinal_ex became the more flexible
versions that left init/cleanup to the caller.
EVP_CIPHER_CTX got the same treatment with
EVP_CipherInit/EVP_CipherInit_ex, but *not*
EVP_CipherFinal/EVP_CipherFinal_ex. The latter did the same thing. The
history seems to be that 581f1c84940d77451c2592e9fa470893f6c3c3eb
introduced the Final/Final_ex split, with the former doing an
auto-cleanup, then 544a2aea4ba1fad76f0802fb70d92a5a8e6ad85a undid it.
Looks like the motivation is that EVP_CIPHER_CTX objects are often
reused to do multiple operations with a single key. But they missed that
the split functions are now unnecessary.
Amusingly, OpenSSL's documentation incorrectly said that EVP_CipherFinal
cleaned up after the call until it was fixed in
538860a3ce0b9fd142a7f1a62e597cccb74475d3. The fix says that some
releases cleaned up, but there were, as far as I can tell, no actual
releases with that behavior.
I've put the new Final functions in the deprecated section, purely
because there is no sense in recommending two different versions of the
same function to users, and Final_ex seems to be more popular. But there
isn't actually anything wrong with plain Final.
Change-Id: Ic2bfda48fdcf30f292141add8c5f745348036852
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50485
Reviewed-by: Adam Langley <agl@google.com>
This may as well be computed from block_size. This reduces the
per-EVP_CIPHER_CTX memory usage slightly.
Update-Note: It doesn't look like anyone is reading into this field. If
they are, we can ideally fix it, or revert this if absolutely necessary.
Change-Id: Ieef9177bed1671efca23d4f94d3d528f82568fc6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45884
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CVE-2021-23840
(Imported from upstream's 6a51b9e1d0cf0bf8515f7201b68fb0a3482b3dc1.)
This differs slightly from upstream's version:
- EVP_R_OUTPUT_WOULD_OVERFLOW didn't seem necessary when ERR_R_OVERFLOW
already exists. (Also since we use CIPHER_R_*, it wouldn't have helped
with compatibility anyway. Though there's probably something to be
said for us folding CIPHER_R_* back into EVP_R_*.)
- For simplicity, just check in_len + bl at the top, rather than trying
to predict the exact number of bytes written.
Update-Note: Passing extremely large input lengths into EVP_CipherUpdate
will now fail. Use EVP_AEAD instead, which is size_t-based and has more
explicit output bounds.
Change-Id: I31835c89dcdecb6b112828f57deb798dc7187db5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45685
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
David Benjamin