Builds that compile the FIPS stuff separately don't get this header from
other files.
Change-Id: I8a1b30ae360b08d4f4b9f804cd234998889477bc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51405
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
AS10.20 requires that the self-test for the integrity algorithm pass
before the integrity check itself. IG 10.3.A requires an HMAC self-test
now. Therefore run these tests before the integrity check.
Since we also need the ability to run all self-tests, both SHA
self-tests and the HMAC test are run again when running self-tests.
I'm assuming that they're so fast that it doesn't matter.
Change-Id: I6b23b6fd3cb6107edd7420bc8680780719bd41d2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51328
Reviewed-by: David Benjamin <davidben@google.com>
The word “calculated” is two letters longer than “expected” and it's
nice to line up the ouptuts.
Change-Id: Idac70e62d98fbe26c430f03f4643ba295e40853d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51327
Reviewed-by: David Benjamin <davidben@google.com>
The provision of FIPS that allowed the tests to be skipped based on a
flag-file has been removed in 140-3. Therefore we expect to run the fast
KATs on start-up, but to defer to slower ones until the functionality in
question is first used. So this change splits off the fast KATs and
removes support for skipping KATs based on a flag-file.
Change-Id: Ib24cb1739cfef93e4a1349d786a0257ee1083cfb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51326
Reviewed-by: David Benjamin <davidben@google.com>
This matches our other free functions.
Fixed: 473
Change-Id: Ie147995c2f5b429f78e95cfc9a08ed54181af94e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51005
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
140-3 says
> the zeroisation of protected and unprotected SSPs
> shall be performed in the following scenarios:
> ...
> For temporary value(s) generated during the integrity test of the
> module’s software or firmware upon completion of the integrity test.
(IG 9.7.B)
Change-Id: I911f294860bf33b13b2c997fc633c9bda777fc48
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50945
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
GCC's __ARMEL__ and __ARMEB__ defines denote little- and big-endian arm,
respectively. They are not defined on aarch64, which instead use
__AARCH64EL__ and __AARCH64EB__.
However, OpenSSL's assembly originally used the 32-bit defines on both
platforms and even define __ARMEL__ and __ARMEB__ in arm_arch.h. This is
less portable and can even interfere with other headers, which use
__ARMEL__ to detect little-endian arm. (Our own base.h believes
__ARMEL__ implies 32-bit arm. We just happen to check __AARCH64EL__
first. base.h is probably also always included before arm_arch.h.)
Over time, the aarch64 assembly has switched to the correct defines,
such as in 32bbb62ea634239e7cb91d6450ba23517082bab6. This commit
finishes the job.
(There is an even more official endianness detector, __ARM_BIG_ENDIAN in
the Arm C Language Extensions. But I've stuck with the GCC ones here as
that would be a larger change.)
See also https://github.com/openssl/openssl/pull/17373
Change-Id: Ic04ff85782e6599cdeaeb33d12c2fa8edc882224
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50848
Reviewed-by: Adam Langley <agl@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>
This imports the changes to sha512-armv8.pl from
upstream's af0fcf7b4668218b24d9250b95e0b96939ccb4d1.
Tweaks needed:
- Add an explicit .text because we put .LK$BITS in .rodata for XOM
- .LK$bits and code are in separate sections, so use adrp/add instead of
plain adr
- Where glibc needs feature flags to *enable* pthread_rwlock, Apple
interprets _XOPEN_SOURCE as a request to *disable* Apple extensions.
Tighten the condition on the _XOPEN_SOURCE check.
Added support for macOS and Linux, tested manually on an ARM Mac and a
VM, respectively. Fuchsia and Windows do not currently have APIs to
expose this bit, so I've left in TODOs. Benchmarks from an Apple M1 Max:
Before:
Did 4647000 SHA-512 (16 bytes) operations in 1000103us (74.3 MB/sec)
Did 1614000 SHA-512 (256 bytes) operations in 1000379us (413.0 MB/sec)
Did 439000 SHA-512 (1350 bytes) operations in 1001694us (591.6 MB/sec)
Did 76000 SHA-512 (8192 bytes) operations in 1011821us (615.3 MB/sec)
Did 39000 SHA-512 (16384 bytes) operations in 1024311us (623.8 MB/sec)
After:
Did 10369000 SHA-512 (16 bytes) operations in 1000088us (165.9 MB/sec) [+123.1%]
Did 3650000 SHA-512 (256 bytes) operations in 1000079us (934.3 MB/sec) [+126.2%]
Did 1029000 SHA-512 (1350 bytes) operations in 1000521us (1388.4 MB/sec) [+134.7%]
Did 175000 SHA-512 (8192 bytes) operations in 1001874us (1430.9 MB/sec) [+132.5%]
Did 89000 SHA-512 (16384 bytes) operations in 1010314us (1443.3 MB/sec) [+131.4%]
(This doesn't seem to change the overall SHA-256 vs SHA-512 performance
question on ARM, when hashing perf matters. SHA-256 on the same chip
gets up to 2454.6 MB/s.)
In terms of build coverage, for now, we'll have build coverage
everywhere and test coverage on Chromium, which runs this code on macOS
CI. We should request a macOS ARM64 bot for our standalone CI. Longer
term, we need a QEMU-based builder to test various features. QEMU seems
to have pretty good coverage of all this, which will at least give us
Linux.
I haven't added an OPENSSL_STATIC_ARMCAP_SHA512 for now. Instead, we
just look at the standard __ARM_FEATURE_SHA512 define. Strangely, the
corresponding -march tag is not sha512. Neither GCC and nor Clang have
-march=armv8-a+sha512. Instead, -march=armv8-a+sha3 implies both
__ARM_FEATURE_SHA3 and __ARM_FEATURE_SHA512! Yet everything else seems
to describe the SHA512 extension as separate from SHA3.
https://developer.arm.com/architectures/system-architectures/software-standards/acle
Update-Note: Consumers with a different build setup may need to
limit -D_XOPEN_SOURCE=700 to Linux or non-Apple platforms. Otherwise,
<sys/types.h> won't define some typedef needed by <sys/sysctl.h>. If you
see a build error about u_char, etc., being undefined in some system
header, that is probably the cause.
Change-Id: Ia213d3796b84c71b7966bb68e0aec92e5d7d26f0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50807
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This imports 753316232243ccbf86b96c1c51ffcb41651d9ad5,
46f4e1bec51dc96fa275c168752aa34359d9ee51, and
32bbb62ea634239e7cb91d6450ba23517082bab6.
The last commit fixes a detection of big-endian aarch64 in the kernel,
which we do not support at all, but is imported to reduce the upstream
diff. Though it points out a messy part of arm_arch.h: __ARMEL__ and
__ARMEB__ are specific to 32-bit ARM. __AARCH64EB__ and __AARCH64EL__
are the 64-bit ones. But OpenSSL's arm_arch.h defines __ARME[LB]__ for
aarch64 and uses it in perlasm. We should fix the files upstream to
look at the aarch64 ones. (Indeed our own base.h assumes __ARMEL__
implies 32-bit ARM.)
Change-Id: I6c2241e103a97e8c3599cdfa43dcc6f30d4a2581
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50806
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We currently have two aarch64 SHA-256 implementations: one using
general-purpose registers and one using the SHA-256 extensions.
Upstream's 866e505e0d663158b0fe63a7fb7455eebacc6470 added a NEON
version.
This CL syncs the transforms at the bottom of the file, to avoid
potential mistranslations in future imports. It doesn't change the
output for our current assembly.
Skips the NEON implementation itself for now. It only helps
processors without SHA-256 instructions. While Android does not
actually mandate the cryptography extensions on ARMv8, most devices
have it.
Additionally, this file does CPU dispatch in assembly, without taking
advantage of static information. We'd end up shipping both fallback
SHA-256 implementations. This is particularly silly because NEON is
mandatory in ARMv8-A anyway. (Does anyone build us on -R or -M? Probably
not?)
(If we later have a reason to import it, the binary size cost isn't that
significant. Moreover, the NEON fallback is actually slightly smaller
than the non-NEON fallback, so if we move CPU dispatch to C, importing
may even be worthwhile.)
Change-Id: I3c8ca6e77e4e6d1299f975c407cbcf4c9c240523
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50805
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
GCC has a warning that complains about even more type mismatches in
printf. Some of these are a bit messy and will be fixed in separate CLs.
This covers the easy ones.
The .*s stuff is unfortunate, but printf has no size_t-clean string
printer. ALPN protocol lengths are bound by uint8_t, so it doesn't
really matter.
The IPv6 printing one is obnoxious and arguably a false positive. It's
really a C language flaw: all types smaller than int get converted to
int when you do arithmetic. So something like this first doesn't
overflow the shift because it computes over int, but then the result
overall is stored as an int.
uint8_t a, b;
(a << 8) | b
On the one hand, this fixes a "missing" cast to uint16_t before the
shift. At the same time, the incorrect final type means passing it to
%x, which expects unsigned int. The compiler has forgotten this value
actually fits in uint16_t and flags a warning. Mitigate this by storing
in a uint16_t first.
The story doesn't quite end here. Arguments passed to variadic functions
go through integer promotion[0], so the argument is still passed to
snprintf as an int! But then va_arg allows for a signedness mismatch[1],
provided the value is representable in both types. The combination means
that %x, though actually paired with unsigned, also accept uint8_t and
uint16_t, because those are guaranteed to promote to an int that meets
[1]. GCC recognizes [1] applies here.
(There's also PRI16x, but that's a bit tedious to use and, in glibc, is
defined as plain "x" anyway.)
[0] https://en.cppreference.com/w/c/language/conversion#Default_argument_promotions
[1] https://en.cppreference.com/w/c/variadic/va_arg
Bug: 450
Change-Id: Ic1d41356755a18ab922956dd2e07b560470341f4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50765
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
OpenSSL 1.1.0 made this structure opaque. I don't think we particularly
need to make it opaque, but external code uses it. Also add
RSA_test_flags.
Change-Id: I136d38e72ec4664c78f4d1720ec691f5760090c1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50605
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>
Although the compiler will hopefully optimize it out, this is
technically a VLA. The new Android NDK now warns about this.
Change-Id: Ib9f38dc73c40e90ab61105f29a635c453f1477a1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50185
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We have a ton of per-file rotation functions, often with generic names
that do not tell you whether they are uint32_t vs uint64_t, or rotl vs
rotr.
Additionally, (x >> r) | (x << (32 - r)) is UB at r = 0.
(x >> r) | (x << ((-r) & 31)) works for 0 <= r < 32, which is what
cast.c does. GCC and Clang recognize this pattern as a rotate, but MSVC
doesn't. MSVC does, however, provide functions for this.
We usually rotate by a non-zero constant, which makes this moot, but
rotation comes up often enough that it's worth extracting out. Some
particular changes to call out:
- I've switched sha256.c from rotl to rotr. There was a comment
explaining why it differed from the specification. Now that we have
both functions, it's simpler to just match the specification.
- I've dropped all the inline assembly from sha512.c. Compilers should
be able to recognize rotations in 2021.
Change-Id: Ia1030e8bfe94dad92514ed1c28777447c48b82f9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49765
Reviewed-by: Adam Langley <agl@google.com>
There is an obvious bug there: upon entry to 'vpaes_cbc_encrypt'
LR may get signed. However, on the 'cbc_abort' path the LR is
not going to be unsigned before 'ret' is executed.
Found by manual code inspection.
Co-authored-by: Russ Butler <russ.butler@arm.com>
Change-Id: I646cdfaee28db59aafbbd412d4bb6ba022eff15b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49605
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
GCC 11.2.1 reportedly warns that CTR_DRBG_init may be passed an
uninitialized personalization buffer. This appears to be a false
positive, because personalization_len will be zero. But it's easy enough
to zero-initialize it, so silence the warning.
Bug: 432
Change-Id: I20f6b74e09f19962e8cae37d45090ff3d1c0215d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49245
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The bulk of RSA_check_key is spent in bn_div_consttime, which is a naive
but constant-time long-division algorithm for the few places that divide
by a secret even divisor: RSA keygen and RSA import. RSA import is
somewhat performance-sensitive, so pick some low-hanging fruit:
The main observation is that, in all but one call site, the bit width of
the divisor is public. That means, for an N-bit divisor, we can skip the
first N-1 iterations of long division because an N-1-bit remainder
cannot exceed the N-bit divisor.
One minor nuisance is bn_lcm_consttime, used in RSA keygen has a case
that does *not* have a public bit width. Apply the optimization there
would leak information. I've implemented this as an optional public
lower bound on num_bits(divisor), which all but that call fills in.
Before:
Did 5060 RSA 2048 private key parse operations in 1058526us (4780.2 ops/sec)
Did 1551 RSA 4096 private key parse operations in 1082343us (1433.0 ops/sec)
After:
Did 11532 RSA 2048 private key parse operations in 1084145us (10637.0 ops/sec) [+122.5%]
Did 3542 RSA 4096 private key parse operations in 1036374us (3417.7 ops/sec) [+138.5%]
Bug: b/192484677
Change-Id: I893ebb8886aeb8200a1a365673b56c49774221a2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49106
Reviewed-by: Adam Langley <agl@google.com>
This syncs this file up to e7ff223a20697e5a401d2d9bb7a75e699ed46633 from
upstream's OpenSSL_1_1_1-stable branch. The main change of note is the
4x loop from upstream's 7ff2fa4b9281232f0ca1db03d42a954c462ef77d,
9ee020f8dc7813db82a119058d8f57e70e7e8904,
aa7bf316980259a11dcbaf6128ed86d33dc24b97, and
603ebe03529101424670051aa0c616dc6e037b28.
Benchmarks on a Pixel 4a.
Before:
Did 14069000 AES-128-GCM (16 bytes) seal operations in 2000042us (112.5 MB/sec)
Did 6768000 AES-128-GCM (256 bytes) seal operations in 2000182us (866.2 MB/sec)
Did 1902000 AES-128-GCM (1350 bytes) seal operations in 2000479us (1283.5 MB/sec)
Did 359000 AES-128-GCM (8192 bytes) seal operations in 2003942us (1467.6 MB/sec)
Did 182000 AES-128-GCM (16384 bytes) seal operations in 2002245us (1489.3 MB/sec)
Did 13388000 AES-256-GCM (16 bytes) seal operations in 2000144us (107.1 MB/sec)
Did 6069000 AES-256-GCM (256 bytes) seal operations in 2000276us (776.7 MB/sec)
Did 1638000 AES-256-GCM (1350 bytes) seal operations in 2001076us (1105.1 MB/sec)
Did 305000 AES-256-GCM (8192 bytes) seal operations in 2000040us (1249.3 MB/sec)
Did 155000 AES-256-GCM (16384 bytes) seal operations in 2009398us (1263.8 MB/sec)
After:
Did 13837000 AES-128-GCM (16 bytes) seal operations in 2000131us (110.7 MB/sec) [-1.7%]
Did 7506000 AES-128-GCM (256 bytes) seal operations in 2000197us (960.7 MB/sec) [+10.9%]
Did 2289000 AES-128-GCM (1350 bytes) seal operations in 2000734us (1544.5 MB/sec) [+20.3%]
Did 443000 AES-128-GCM (8192 bytes) seal operations in 2000321us (1814.2 MB/sec) [+23.6%]
Did 225000 AES-128-GCM (16384 bytes) seal operations in 2002308us (1841.1 MB/sec) [+23.6%]
Did 13280000 AES-256-GCM (16 bytes) seal operations in 2000011us (106.2 MB/sec) [-0.8%]
Did 6630000 AES-256-GCM (256 bytes) seal operations in 2000229us (848.5 MB/sec) [+9.2%]
Did 1916000 AES-256-GCM (1350 bytes) seal operations in 2000373us (1293.1 MB/sec) [+17.0%]
Did 365000 AES-256-GCM (8192 bytes) seal operations in 2001519us (1493.9 MB/sec) [+19.6%]
Did 185000 AES-256-GCM (16384 bytes) seal operations in 2006588us (1510.5 MB/sec) [+19.5%]
(See cl/387919990 for some notes I made in reviewing, though likely
future me will find them incomprehensible anyway.)
Change-Id: Id386e80143611487e07b2fbfda15d0abc54ea145
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48726
Reviewed-by: Adam Langley <agl@google.com>
CMake's language is rather fragile and unsound. For the most part, it is
a shell script with more parentheses. That is, it simply expands command
arguments into a list of strings and then evaluates it, complete with
shell-style differences between "${FOO}" and ${FOO}.
The if() command is special and internally also expands variables. That
is why things like if(FOO STREQUAL "BAR") work. CMake interprets "FOO"
as a variable if it can find a variable, or a string otherwise. In
addition to getting very confused on typos, it means that
if("${FOO}" STREQUAL "BAR") will double-expand, and it will do strange
things if BAR is a variable.
CMP0054 patches this (which we set by minimum version) so that if() only
expands if the token was unquoted. This fixes
if("${FOO}" STREQUAL "BAR"). However, if(${FOO} STREQUAL "BAR")
continues to double-expand FOO.
We had a mix of all three of FOO, ${FOO}, and "${FOO}". It's not clear
which is the canonical spelling at this point, but CMake own files
(mostly) use FOO, as do most of our lines, so I've standardized on that.
It's a little unsatisfying if we typo a variable, but I suppose ${FOO}
also silently ignores unset variables.
Bug: 423
Change-Id: Ib6baa27f4065eed159e8fb28820b71a0c99e0db0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48705
Reviewed-by: Adam Langley <agl@google.com>
Trusty wants to seed from a different RNG than the one that supplies
per-draw entropy. This is a no-op change unless you're substituting in
your own implementations of these functions.
To see that it's a no-op in urandom.c, note that it only changes the
|seed| argument to |fill_with_entropy|. That causes the value of
|extra_getrandom_flags_for_seed_bss_get| to be ORed into the flags,
but that value will always be zero unless it's an Android FIPS build.
Change-Id: Ic8d954df3074559cbf1bfee1ae91a4a2b7e14d9d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48485
Reviewed-by: David Benjamin <davidben@google.com>
See also f8fc0e35e0b1813af15887d42e17b7d5537bb86c from upstream, though
our BN_divs have diverged slightly.
Change-Id: I49fa4f0a5c730d34e6f41f724f1afe3685470712
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48426
Reviewed-by: Adam Langley <agl@google.com>
EVP_MD_nid, in OpenSSL, is the same as EVP_MD_type. EVP_MD_type seems to
be the preferred spelling, so put EVP_MD_nid in the deprecated bucket.
Also add an EVP_MD_do_all alias to EVP_MD_do_all_sorted.
Change-Id: I4e7b800902459ac5cb9ef0df65d73da94afdf927
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48365
Reviewed-by: Adam Langley <agl@google.com>
This comment dates to SSLeay. It appears to be describing the
incremental trial division strategy where they would pick a starting
candidate, compute moduli by small primes, and then update by
incrementing the candidate and saved moduli instead of dividing from
scratch. We use a simpler rejection sampling strategy.
Change-Id: If2203d616f2b1f632bcd7033ceb60a83d1b75674
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48047
Reviewed-by: Adam Langley <agl@google.com>
We'll need to maintain two transcripts on the ECH client and then, once
we know which of ClientHelloOuter or ClientHelloInner is used, overwrite
the default transcript with the alternate one.
Rather than indirect through a pointer, move support is easy enough.
Then this can just be hs->transcript = std::move(hs->inner_transcript).
Bug: 275
Change-Id: Id4b0a0a48b956cd65ce8fc3dacfd16eebe2eb778
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47993
Reviewed-by: Adam Langley <agl@google.com>
Because file names are not enclosed in quotation marks in the open call.
https://bugs.chromium.org/p/boringssl/issues/detail?id=415
```
cmake --build "C:\Projects\ Extern\Visual C++ 2015\x64 Debug\Build\BoringSSL\."
[9/439] Generating rdrand-x86_64.asm
FAILED: crypto/fipsmodule/rdrand-x86_64.asm
cmd.exe /C "cd /D "C:\Projects\ Extern\Visual C++ 2015\x64 Debug\Build\BoringSSL\crypto\fipsmodule" && "C:\Program Files\CMake\bin\cmake.exe" -E make_directory . && C:\Perl64\bin\perl.exe "C:/Projects/ Extern/Source/BoringSSL/crypto/fipsmodule/rand/asm/rdrand-x86_64.pl" nasm rdrand-x86_64.asm"
Can't open perl script "C:/Projects/": No such file or directory
error closing STDOUT at C:/Projects/ Extern/Source/BoringSSL/crypto/fipsmodule/rand/asm/rdrand-x86_64.pl line 87.
ninja: build stopped: subcommand failed.
```
Bug: 415
Change-Id: I83c4a460689b9adeb439425ad390322ae8b2002a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47884
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Similar to
https://boringssl-review.googlesource.com/c/boringssl/+/46405,
SHA256_Final and SHA224_Final hit array size warnings in the new GCC.
The array sizes are, strictly speaking, purely decoration, but this is a
good warning so we should be clean with it on.
That same change is difficult to apply to md32_common.h because
md32_common.h generates the functions for us. md32_common.h is already
strange in that it is multiply-included and changes behavior based on
macros defined by the caller.
Instead, replace it with inline functions, which are a bit more
conventional and typesafe. This allows each hash function to define the
function prototype. Use this to add an unsized helper for SHA-256.
Bug: 402
Change-Id: I61bc30fb58c54dd40a55c9b1ebf3fb9adde5e038
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47807
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Peter Foley <pefoley@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The macro isn't doing any work here.
Change-Id: Id97dfa4b027407c5e4b3e7eb1586c3c2a2d977d8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47806
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We usually call the parameter 'digest', but people sometimes think they
can skip the hashing for short inputs are short. I also suspect the term
'digest' is less common. Add warnings about this.
There were also some cases where we called it 'in' and even 'msg'. This
CL fixes those to say 'digest'. Finally, RSA_{sign,verify}_raw are
documented to be building blocks of signature schemes, rather than
signature schemes themselves.
It's unfortunate that EVP_PKEY_sign means "sign a digest", while
EVP_DigestSign means "sign, likely internally digesting it as the first
step", but we're a bit stuck there.
Change-Id: I4c38afff9b6196e2789cf27653fe5e5e8c68c1bf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47504
Reviewed-by: Adam Langley <agl@google.com>
https://boringssl-review.googlesource.com/c/boringssl/+/42504 aligned
RSA private key checks, but I missed the public key ones. We have two
different sets of RSA public key checks right now. One in the parser
just checks for e = 1 and even e. The other, when using the key, checks
for overly large e and n.
Align the two. Now parsing RSA public keys calls RSA_check_key and the
extra checks on e are added to RSA_check_key. Note RSA private key
parsing already called RSA_check_key. The consequences are:
First, RSA public keys with large n, large e, or n < e will be rejected
at parse time. Previously, they would be parsed but all operations on
them would fail. This aligns with our existing behavior for parsing
private keys.
Second, operations on RSA public keys with even e will fail. They
already failed to parse, but it was possible to manually construct such
a key. Previously, operations wouldn't explicitly fail, but they
wouldn't do anything useful because even exponents are not invertible.
(Encrypting would produce something undecryptable and the private key
would have a hard time reliably producing signatures we'd accept.) There
is no change to RSA private keys with even e. Those would already fail
the (e, d) consistency check and the fault check.
Third, operations on RSA public keys with e = 1 will fail. They already
failed to parse, but it was possible to manually construct such a key
and "verify" signatures or "encrypt" messages. However, with e = 1,
those operations are no-ops.
Finally, RSA private keys with e = d = 1 will be rejected at parse and
use. This is the only case that affects private keys because e = d = 1
are inverses, just pointless. Uses paired with RSA public key parsing
(e.g. our TLS library checks consistency with a certificate public key)
are not affected. Those already rejected such keys because we rejected
them in the public key parser. This CL aligns the private half.
This doesn't close https://crbug.com/boringssl/316, but we won't be able
to resolve that without a consistent story for what keys are valid.
Update-Note: See above.
Bug: 316
Change-Id: Ic27df18c4f48e5e3e57a17d6fe39399e2f8d5c68
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47524
Reviewed-by: Adam Langley <agl@google.com>
We have loads of variations of these. Align them in one set. This avoids
the HOST_* macros defined by md32_common.h, so it'll be a little easier
to make it a more conventional header.
Change-Id: Id47fe7b51a8f961bd87839f8146d8a5aa8027aa6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46425
Reviewed-by: Adam Langley <agl@google.com>
It's a little confusing to have load_word_le but actually use size_t
instead of crypto_word_t.
NOTE: on some platforms, notably NaCl, crypto_word_t is larger than
size_t. (Do we still need to support this?) We don't have a good testing
story here, so I tested it by hacking up a 32-bit x86 build to think it
was OPENSSL_64_BIT.
Change-Id: Ia0ce469e86803f22655fe2d9659a6a5db766429f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46424
Reviewed-by: Adam Langley <agl@google.com>
MSAN doesn't like the counters starting at whatever value malloc
found to be free.
Change-Id: I0968e61e0025db35b82291fde5d1e193aef77c1e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46444
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Although it is strictly fine to call SHA512_Final in SHA384_Final
(array sizes in C parameters are purely decorational, according to the
language), GCC 11 reportedly checks now and gets upset about the size
mismatch. Use an unsized helper function so all our code matches the
specified bounds.
Unfortunately, the bounds in all the functions are a bit misleading
because SHA512_Final really outputs based on sha->md_len (which Init
function you called) rather than which Final function. I've fixed this
places within a library where we mismatched and added asserts to the
smaller functions. SHA512_Final is assert-less because I've seen lots of
code use SHA384_Init / SHA512_Update / SHA512_Final.
This doesn't fix the SHA256 variant since that is generated by a pile of
macros in a multiply-included file. This is probably a good opportunity
to make that code less macro-heavy.
Update-Note: There is a small chance the asserts will trip something,
but hopefully not since I've left SHA512_Final alone.
Bug: 402
Change-Id: I4c9d579a63ee0a0dea103c19ef219c13bb9aa62c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46405
Reviewed-by: Adam Langley <agl@google.com>
See also 8129ac6ac4c0ca3a488c225cde580ede7dabe874 and
81198bf323ea9deda907714170d329ca7d2ff01f from upstream.
In trying to figure out why ASan (which normally catches overlapping
memcpys) didn't flag this, I noticed that we actually don't have tests
for empty inputs. I've added them to cipher_tests.txt where missing and
fixed a bad assert in ofb.c.
ASan still doesn't flag this because LLVM even requires memcpy handle
dst == src. Still, fixing it is less effort than getting a clear answer
from GCC and MSVC. Though this puts us in the frustrating position of
trying to follow a C rule that our main toolchain and sanitizer disavow.
https://bugs.llvm.org/show_bug.cgi?id=11763https://reviews.llvm.org/D86993
Change-Id: I53c64a84834ddf5cddca0b3d53a29998f666ea2f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46285
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Adam Langley
David Benjamin
Tamas Petz
Shelley Vohr
Florin Crișan
Adam Langley
Bradley Hess