On non-ELF platforms, WEAK_SYMBOL_FUNC expands to a static variable. On
ASan, we don't use sdallocx. Clang then warns about an unused static
variable. Silence the warning.

Change-Id: I3d53519b669d435f3801f45e4b72c6ca4cd27a3b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53565
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

This is split out from
https://boringssl-review.googlesource.com/c/boringssl/+/47544 just to
get the bugfixes and tests out of the way of the refactor.

If we trip the SSL_R_BAD_LENGTH check in tls_write_app_data, wnum is set
to zero. But wnum should only be cleared on a successful write. It
tracks the number of input bytes that have been written to the transport
but not yet reported to the caller. Instead, move it to the success
return in that function. All the other error paths already set it to
something else.

Change-Id: Ib22f9cf04454ecdb0062077f183be5070ab7d791
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53545
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

Writing application data goes through three steps:

1. Encrypt the data into the write buffer.
2. Flush the write buffer to the network.
3. Report to SSL_write's caller that the write succeeded.

In principle, steps 2 and 3 are done together, but it is possible that
BoringSSL needs to write something, but we are not in the middle of
servicing an SSL_write call. Then we must perform (2) but cannot perform
(3).

TLS 1.3 0-RTT on a client introduces a case like this. Suppose we write
some 0-RTT data, but it is blocked on the network. Meanwhile, the
application tries to read from the socket (protocols like HTTP/2 read
and write concurrently). We discover ServerHello..Finished and must then
respond with EndOfEarlyData..Finished. But to write, we must flush the
current write buffer.

To fix this, https://boringssl-review.googlesource.com/14164 split (2)
and (3) more explicitly. The write buffer may be flushed to the network
at any point, but the wpend_* book-keeping is separate. It represents
whether (3) is done. As part of that, we introduced a wpend_pending
boolean to track whether there was pending data.

This introduces an interesting corner case. We now keep NewSessionTicket
messages buffered until the next SSL_write. (KeyUpdate ACKs are
implemented similarly.) Suppose the caller calls SSL_write(nullptr, 0)
to flush the NewSessionTicket and this hits EWOULDBLOCK. We'll track a
zero-length pending write in wpend_*! A future attempt to write non-zero
data would then violate the moving buffer check. This is strange because
we don't build records for zero-length application writes in the first
place.

Instead, wpend_pending should have been wpend_tot > 0. Remove that and
rearrange the code to check that properly. Also remove wpend_ret as it
has the same data as wpend_tot.

Change-Id: I58c23842cd55e8a8dfbb1854b61278b108b5c7ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53546
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

27ffcc6 switched the integrity check to using SHA-256, but the
Aarch64 FIPS build was still passing -sha256 to inject_hash.go.

Change-Id: I641de17d62205c7f127cd2a910d4e98778d492e7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53605
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

p256-armv8-asm.pl defined ecp_nistz256_[to|from]_mont as global
functions, but p256-nistz.h defined them as static inlines.
Additionally, ecp_nistz256_to_mont was never used.

This change drops the assembly versions and drops ecp_nistz256_to_mont
completely.

Change-Id: Ie2cc5bf4adc423f72f61cf227be0e93c9a6e2031
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53606
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

b951243 readded 3DES support in acvptool, but not in
modulewrapper because we don't want it for BoringSSL itself. But without
modulewrapper support, the tests don't work. Support could be backported
into testmodulewrapper but it doesn't seem worthwhile for a few more
months support.

Change-Id: I4e7ace66f9ac1915996db7dfdeeb7e9d4969915f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53607
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

Change-Id: Ie071dcd94d2ae8aa8ee148682f9b0054ed9e3501
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52445
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

Follow-up work will add support for TLS 1.2 ticket decryption.

Bug: 504
Change-Id: Ieaee37d94562040f1d51227216359bd63db15198
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53525
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

This flag is currently set if DRBG entropy is obtained from RDRAND. It
indicates that we should add kernel entropy when seeding the DRBG. But
this might be true for methods other than RDRAND in the future so this
change renames it accordingly.

Change-Id: I91826178a806e3c6dadebbb844358a7a12e0b09b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52525
Reviewed-by: David Benjamin <[email protected]>

When seeding a DRBG for the first time we currently make two reads: one
to start the CRNGT and a second to read the actual seed. These reads can
be merged to save I/O.

Change-Id: I2a83edf7f3c8b9d6cebcde02195845be9fde19b2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52526
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

Change-Id: I69aba15ccf57d04c66a98755b98221b8688d291a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52527
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

This reverts commit 4259ae8.

Some Android builders perhaps lack getrandom support.

Change-Id: Ic7537c07dacb31a54adb453ddd5f82a789089eaf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53625
Auto-Submit: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

We still have our <= 0 return values because anything with BIOs tries to
preserve BIO_write's error returns. (Maybe we can stop doing this?
BIO_read's error return is a little subtle with EOF vs error, but
BIO_write's is uninteresting.) But the rest of the logic is size_t-clean
and hopefully a little clearer. We still have to support SSL_write's
rather goofy calling convention, however.

I haven't pushed Spans down into the low-level record construction logic
yet. We should probably do that, but there are enough offsets tossed
around there that they warrant their own CL.

Bug: 507
Change-Id: Ia0c702d1a2d3713e71b0bbfa8d65649d3b20da9b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47544
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

This works correctly, but part of implementing SSL_write_ex will, if not
done correctly, regress this. Specifically, if the read_shutdown check
in SSL_get_error were not conditioned on ret == 0, the last
SSL_get_error in the test would mistakenly classify the write error as
SSL_ERROR_ZERO_RETURN.

Add a regression test in advance.

Bug: 507
Change-Id: I8ddb4606e291977506ee81f4ed11427e5b1636d8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53626
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

We only care about dates within years 0000 to 9999 for
RFC5280. timegm() is only semi-standard. Some things require the
setting awkward defines to get libc to give it to you. Other things
let you have it but make it stop working at year 3000. Still other
things have 32 bit time_t.....

Let's just make our own that actually works. all the time, does
everything with an int64_t, and fails if you want to send something
out that would overflow a 32 bit time_t.

In the process of doing this, we get rid of the old Julian date stuff
from OpenSSL, which while functional was a bit awkward dealing only
with days, and using the Julian calendar as the reference point instead of potentially something more useful. Julian seconds since Jan 1 1970
00:00:00 UCT are much more useful to us than Julian days since a
Julian epoch.

The OS implementations of timegm() and gmtime() also can be pretty
complex, due to the nature of needing multiple timezone, daylight
saving, day of week, and other stuff we simply do not need for
doing things with certificate times. A small microbenchmark of
10000000 of each operation comparing this implementation to
the system version on my M1 mac gives:

bbe-macbookpro:tmp bbe$ time ./openssl_gmtime

real    0m0.152s
user    0m0.127s
sys     0m0.018s
bbe-macbookpro:tmp bbe$ time ./gmtime

real    0m0.422s
user    0m0.403s
sys     0m0.014s
bbe-macbookpro:tmp bbe$ time ./openssl_timegm

real    0m0.041s
user    0m0.015s
sys     0m0.019s
bbe-macbookpro:tmp bbe$ time ./timegm

real    0m30.432s
user    0m30.383s
sys     0m0.040s

Similarly On a glinux machine:

bbe@bbe-glinux1:~$ time ./openssl_gmtime

real    0m0.157s
user    0m0.152s
sys     0m0.008s
bbe@bbe-glinux1:~$ time ./gmtime

real    0m0.336s
user    0m0.336s
sys     0m0.002s
bbe@bbe-glinux1:~$ time ./openssl_timegm

real    0m0.018s
user    0m0.019s
sys     0m0.002s
bbe@bbe-glinux1:~$ time ./timegm

real    0m0.680s
user    0m0.671s
sys     0m0.011s
bbe@bbe-glinux1:~$


Bug: 501

Change-Id: If445272d365f2c9673b5f3264d082af1a342e0a1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53245
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

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 <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

We need to know which ARM instructions take a condition code because
otherwise the conditions look like symbols. This change includes all
instructions beginning with 'c' from [1] that include a `cond` argument.
Also sort them for easier comparison.

[1]
https://developer.arm.com/documentation/dui0802/a/A64-General-Instructions/CBNZ

Change-Id: Iea07aa4afe171d684135ff6655c52374d86529ce
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53745
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

Tests can now be run either in a local build or on an attached
device.  The script tries to infer the correct mode of operation
but it can also be specified on the command line.

Test: Ran break-tests.sh in both modes
Change-Id: I515ac0cede23e2cb775b99e0af8108a3ce0bde37
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53585
Reviewed-by: Adam Langley <[email protected]>

This syscall is required by generatekey in keystore.

Signed-off-by: Liu Cunyuan <[email protected]>
Signed-off-by: Mao Han <[email protected]>
Change-Id: I4dd0534daa6cfa52429e5bf398679fccb7d67e7f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53765
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

https://boringssl-review.googlesource.com/c/boringssl/+/53007
inadvertently changed the semantics of SSL_load_client_CA_file slightly.
The original implementation, by delaying allocating ret, would fail
rather than return an empty list.

Fix this and add a test. We don't have much support for testing
filesystem-related things yet, so I've just used /dev/null and gated it
to Linux + macOS for now. If we need it later, we can add temporary file
support to the test_support library.

Change-Id: If77dd493a433819a65378d76cf400cce48c0abaa
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53785
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>

This comment refers to something that was removed in
https://boringssl-review.googlesource.com/c/boringssl/+/43889

Change-Id: Icf10ed5eb2ce552f2c1dbcdb89853cddb1183ad1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53786
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

Change-Id: I1cf99586d72ee9c01e99ca6baa6479e5dd2aef5d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53787
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>

Rather than documenting the private sk_new_null, etc., APIs and then
expecting callers to infer the real API, describe a real sample API
under #if 0.

Also rename the function pointers to sk_FOO_whatever, which both matches
OpenSSL and reduces the namespaces we squat. The generic callback types
I've renamed to OPENSSL_sk_whatever, to similarly match OpenSSL. We
should also rename plain sk_whatever, but that'll require fixing some
downstream code.

Bug: 499
Change-Id: I49d250958d40858cb49eeee2aad38a17a63add87
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53009
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

This runs through much the same code as the TLS 1.3 bits, though we use
a different hint field to avoid mixups between the fields. (Otherwise
the receiver may misinterpret a decryptPSK hint as the result of
decrypting the session_ticket extension, or vice versa. This could
happen if a ClientHello contains both a PSK and a session ticket.)

Bug: 504
Change-Id: I968bafe12120938e6e46e52536efd552b12c66a0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53805
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

More OpenSSL compatibility functions.

Change-Id: I8e9429fcbc3e285f4c4ad9bdf4c1d9d3c73c3064
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53925
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

This was added in OpenSSL 1.1.0. cryptography.io binds it. They don't
actually use it, but this is a useful feature to have anyway. Projects
like Envoy currently implement such a mode with
X509_STORE_set_verify_cb, which is a very problematic API to support.
Add this so we can move them to something more sustainable.

Change-Id: Iaff2d08daa743e0b5f4be261cb785fdcd26a8281
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53965
Commit-Queue: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

OpenSSL renamed X509_STORE_CTX_trusted_stack to
X509_STORE_CTX_set0_trusted_stack. This name is a partially an
improvement as this is a setter, and partially a setback. The "set0"
name is a bit misleading.

set0 is narrowly correct, in that this function does not adjust
refcounts. But usually set0 functions don't adjust refcounts because
they take ownership of the input. This function does not. It simply
borrows the pointer and assumes it will remain valid for the duration of
X509_STORE_CTX.

OpenSSL also renamed X509_STORE_CTX_set_chain to
X509_STORE_CTX_set0_untrusted. I've declined to add that one for now, in
hopes that we can remove both functions. From what I can tell, there's
no point in ever using either function. It's redundant with the last
parameter to X509_STORE_CTX_init.

Change-Id: I0ef37ba56a2feece6f927f033bdcb4671225dc6f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53966
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

A later CL will tighten up SSL_ERROR_ZERO_RETURN handling. In
preparation for this, test that SSL_CTX_set_quiet_shutdown can trigger
SSL_ERROR_ZERO_RETURN.

Bug: 507
Change-Id: Ib50a02c514673ad4b73540934480d54b372d9505
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53945
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>

CPython uses this function.

Change-Id: I03ead7f54ad19e2a0b2ea3b142298cc1e55c3c90
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53967
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

Matching Chromium, Go, and TLS 1.3, only allow SHA-256, SHA-384, and
SHA-512 RSA-PSS signatures, where MGF-1 and message hash match and salt
length is hash length. Sadly, we are stuck tolerating an explicit
trailerField for now. See the certificates in cl/362617931.

This also fixes an overflow bug in handling the salt length. On
platforms with 64-bit long and 32-bit int, we would misinterpret, e.g,
2^62 + 32 as 32. Also clean up the error-handling of maskHash. It was
previously handled in a very confusing way; syntax errors in maskHash
would succeed and only be noticed later, in rsa_mgf1_decode.

I haven't done it in this change, but as a followup, we can, like
Chromium, reduce X.509 signature algorithms down to a single enum.

Update-Note: Unusual RSA-PSS combinations in X.509 are no longer
accepted. This same change (actually a slightly stricter version) has
already landed in Chrome.

Bug: 489
Change-Id: I85ca3a4e14f76358cac13e66163887f6dade1ace
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53865
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

Change-Id: I3f7026b982f8503fd814be6feb99725f8e60b274
Signed-off-by: Rebecca Chang Swee Fun <[email protected]>
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54005
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

Most SSL_ERROR_* values are tracked directly with rwstate. SSL_get_error
is just reading the extra return value out from the previous call.
However, SSL_ERROR_ZERO_RETURN infers close_notify from the SSL's
shutdown state and a zero return value (EOF).

This works, but if we implement SSL_read_ex and SSL_write_ex, a zero
return value is no longer as carefully correlated with EOF. Moreover,
it's already possible to get a non-EOF zero return post-close_notify if
BIO_write returns an (arguably incorrect) return value. Instead, track
SSL_ERROR_ZERO_RETURN in rwstate explicitly.

Since rwstate is exposed as SSL_want and SSL_ERROR_ZERO_RETURN was
previously never returned there, I've made it map SSL_ERROR_ZERO_RETURN
back to SSL_ERROR_NONE. I've also added a test for BIO_write returning
zero, though the real purpose is for a subsequent SSL_write_ex
implementation to retain all the other tests we've added in here.

Update-Note: This is intended to be safe, but if anything breaks around
EOFs, this change is a likely culprit.

Bug: 507
Change-Id: Ide0807665f2e02ee695c4976dc5e99fb10502cf0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53946
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

OHTTP concatenates enc to the ciphertext without any length prefix
(unlike ECH), so an implementation would want to know the length of enc
for the chosen KEM. Add an accessor to help with that.

While I'm here, fix a couple places where we assumed a specific KEM in
the HPKE implementation (although we still only support the one KEM so
this is all moot).

There's probably something to be said for lifting the length checks out
of the KEM-specific code and into the wrappers, as we're assuming
fixed-width fields anyway. But I've left it alone for now.

Change-Id: I634a053faa5e3b35d846b690140333bdc741f92a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54065
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

O_CLOEXEC avoids a race condition and is less code. It was supported in
Linux starting 2.6.23. https://bugs.python.org/issue26343#msg260151 says
it's been available since macOS 10.7. Let's try using it instead of
fcntl and see if anything breaks. It's even part of POSIX these days.

Update-Note: BoringSSL's /dev/urandom code now assumes the platform
supports O_CLOEXEC.

Change-Id: I95313892b36539591685d4c83a387f77129ad3d1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54125
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

CMake has FindThreads to determine how to link the threading library. Use that over just setting pthread in the link libraries and add support for Windows threads.

Change-Id: I5988c54db6a9db688b7d990fc3808d3ae5f2b66f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54146
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

Some HPKE consumers call into the KDF directly. We don't have an EVP_KDF
abstraction and it's not clear to me how settled "KDF" is as an
interface. (HPKE specifically assumes an extract/expand pair.)

For now, just add EVP_HPKE_KDF_hkdf_md which is defined to only work for
HKDF KDFs. As we don't implement ID -> KDF lookup ourselves and expect
callers to decide which algorithms they want to export, any future
non-HKDF-based KDF won't affect existing callers anyway. If that
happens, we can make this return an EVP_KDF or just add
EVP_HPKE_KDF_{extract,expand} depending on universal this turns out to
be.

Change-Id: I93b9c8a5340472974a6f1bfc45154371d8971600
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54085
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

Among many many problems with renegotiation is it makes every API
ambiguous. Do we return the pending handshake's properties, or the most
recently completed handshake? Neither answer is unambiguously correct:

On the one hand, OpenSSL's API makes renegotiation transparent, so the
pending handshake breaks invariants. E.g., currently,
SSL_get_current_cipher and other functions can return NULL mid
renegotiation. See https://crbug.com/1010748.

On the other hand, OpenSSL's API is callback-heavy. During a handshake
callback, the application most likely wants to check the pending
parameters. Most notably, cert verify callbacks calling
SSL_get_peer_certificate.

Historically, only the pending state was available to return anyway.
We've since changed this
(https://boringssl-review.googlesource.com/8612), but we kept the public
APIs as-is. I was particularly worried about cert verify callbacks.

As of https://boringssl-review.googlesource.com/c/boringssl/+/14028/ and
https://boringssl-review.googlesource.com/c/boringssl/+/19665/, cert
verify is moot. We implement the 3-SHAKE mitigation in library, so the
peer cert cannot change, and we don't reverify the certificate at all.

With that, I think we should switch to returning the established
parameters. Chromium is the main consumer that enables renegotiation,
and it would be better off with this behavior. (Maybe we should try to
forbid other properties, like the cipher suite, from changing on
renegotiation. Unchangeable properties make this issue moot.)

This CL would break if the handshake internally used SSL_get_session,
but this is no longer true as of
https://boringssl-review.googlesource.com/c/boringssl/+/41865.

Update-Note: Some APIs will now behave differently mid-renegotation. I
think this is the safer option, but it is possible code was relying on
the other one.

Fixed: chromium:1010748
Change-Id: I42157ccd9704cde3eebf947136d47cda6754c36e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54165
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

On aarch64 and x86_64 ABIs, the unused bits of 32-bit parameters have
unspecified value. That means if, say, the aarch64
aes_hw_set_encrypt_key accessed the 'bits' parameter as X1 rather than
W1, it could get a different value from what C passed in. To test this,
our ABI testing framework fills the upper half of the register with
garbage.  However, set_encrypt_key just cleanly returns error on
unrecognized bit length. So, to check that this all worked correctly, we
need to assert that the return value was correct.

Looking at the assembly, they all handle it correctly, but now we'll
also test it.

(Note these functions break the usual convention and use zero as the
success value.)

Change-Id: Icaf65ea54564ebfe3696b42287488fe3f72ef138
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54205
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Splitting this out from most of the -Wshorten-64-to-32 fixes since it
non-trivially rewrites the function. While I'm here, move variable
declarations slightly closer to their use and document how the salt
check differs from the spec.

Bug: 516
Change-Id: I2e53afecb8ba720fd8c02da504b56c829c20c93b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54206
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

This adds a boringssl interface to get up to 256 bytes of system
entropy from system entropy sources without going through
RAND_bytes. It should only be used for seeding custom prng's
or where malloc() should not be called from boringssl.

Just as with RAND_bytes(), this can abort the program on failure.

Bug: chromium:1295105

Change-Id: Ia55509702970608fe09cfee9809d02f107c15c8c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54045
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

For various reasons, our FIPS mode build will sometimes seed from RDRAND
instead of the OS. (And, when
https://boringssl-review.googlesource.com/c/boringssl/+/52527 relands,
there'll be another non-OS source.)

To help with this,
https://boringssl-review.googlesource.com/c/boringssl/+/37664 made the
FIPS mode rand_get_seed opportunistically incorporate OS entropy when
available. Originally, it just XORed into the original entropy.

Then https://boringssl-review.googlesource.com/c/boringssl/+/44305
rearranged this so that rand_get_seed had an out_used_cpu (since renamed
to out_want_additional input) output, with the caller mixing the entropy
in instead, into the personalization input to CTR_DRBG_init.

In doing so, that change lost the OS entropy in the CTR_DRBG_reseed
calls. Add it back in, using the additional_data parameter. As part of
this, move the CRYPTO_sysrand_if_available call back to rand_get_seed,
this time as a second output which the caller is responsible for passing
into CTR_DRBG_{init,reseed} alongside the main output.

Change-Id: Ie3335c74e940c760031a28de932d6fedfe355ea0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54126
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>

It is now 2022. See if we can assume getrandom in this configuration.

Update-Note: The /dev/urandom fallback is no longer available in FIPS
builds. This fallback relied on RNGGETENTCNT and was quite flaky.

Change-Id: Icf6d29f6d5952fb6c5656c9039a4cfaf1de2d724
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54127
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

GCC 12's -Wstringop-overflow flags issues in SHA224_Final, etc., because
it calls into generic code that might output a SHA-224 length or a
SHA-256 length, and the function prototype declares the array is only
sized for SHA-224.

This is a bit messy because OpenSSL's API for the truncated SHA-2 hashes
allows you to mix and match them. The output size is set by SHA224_Init
and then, originally, SHA256_Final and SHA224_Final were the same thing.
See how OpenSSL's own SHA224 function calls SHA224_Init + SHA256_Final:
https://github.com/openssl/openssl/blob/OpenSSL_1_1_1q/crypto/sha/sha256.c#L49-L61

To get the function prototype bounds to work out, we tightened this
slightly in
https://boringssl-review.googlesource.com/c/boringssl/+/47807 and added
an assert to SHA224_Final that ctx->md_len was the right size.
SHA256_Final does not have that assert yet. The assert says that mixing
SHA256_Init and SHA224_Final is a caller error.

This isn't good enough for GCC 12, which checks bounds assuming there is
no external invariant on ctx->md_len. This CL changes the behavior of
the shorter Final functions: they will now always output the length
implied by the function name. ctx->md_len only figures into an assert()
call. As we don't have the assert in the untruncated functions yet, I've
preserved their behavior, but the test run with cl/471617180 should tell
us whether apply this to all functions is feasible.

Update-Note: Truncated SHA-2 Final functions change behavior slightly,
but anyone affected by this behavior change would already have tripped
an assert() in debug builds.

Change-Id: I80fdcbe6ad76bc8713c0f2de329b958a2b35e8ae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54246
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

GCC 12 triggers a -Warray-bounds false positive in crypto/x509v3's IPv6
parser. Although v6stat.total cannot exceed 16 because of the callback,
GCC doesn't know this and seems to get confused. Checking >= 16 seems to
silence it.

While I'm here, move the comments so they don't obscure the
if/else-if chains and avoid a theoretical overflow in 'zero_cnt' by
checking for the maximum value inside the callback.

Change-Id: If1610a36693915aa92085d8cb3a4709ae82992ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54245
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

Node just calls every function they can find. I've added the other ones
from RFC 3526 (although some of these are *quite* large) but, for now,
skipped the 768-bit and 1024-bit ones. Those are too small. See
nodejs/node#44539.

I've also reordered so DH_get_rfc7919_2048 is first. In so far as we
want to recommend DH at all, that's probably the one to list first.

Change-Id: If101b32114cc631f80ac6696733c440e222d769a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54305
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

Node calls these. OpenSSL renamed their APIs to align with the IETF
renaming NamedCurve to NamedGroup. (Ironically, with post-quantum
ciphers, that name turns out also to be wrong and it probably should
have been a reference to KEMs.)

To avoid churn for now, I haven't marked the old ones as deprecated, or
renamed any of the internal types yet. We can see about doing that
later.

Change-Id: I5765cea8398f3836611977805bf8ae7d6efc0a70
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54306
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Node uses this.

Change-Id: I13e1734a8f60d4ad0c6a7bcab830c3a0406542b1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54307
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Without this, Node expects SSL_trace and friends to be available.

Change-Id: Iaccb9fba819846a418e8f3cd4598dcbc1d62744d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54308
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

OpenSSL added a separate "secure heap" to allocate some data in a
different heap. We don't implement this, so just act as if initializing
it always fails. Node now expects these functions to be available.

Change-Id: I4c57c807c51681b16ec3a60e9674583b193358c4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54309
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

We don't support DSA EVP_PKEY_CTXs (trying to create one will just
fail), but to aid building projects that try to create them, add the
functions and make them always fail.

In particular, Node calls these two. It calls
EVP_PKEY_CTX_set_dsa_paramgen_q_bits via EVP_PKEY_CTX_ctrl, but I'll
send them a patch to use the wrapper function.

Change-Id: Ic134c50b6ea0b59dc8f15be77243b9ae9dfa6a61
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54310
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

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 <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

When installing a library individual destinations should be specified. This is required on Windows which has a .dll that goes in the runtime destination while the .lib ends up in the library destination.

Change-Id: I93cf51089f71c4375324270c6b1c4eadbc637477
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54147
Reviewed-by: Daniel Thornburgh <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

Change-Id: I5e1d37106d7df8e8aaede295e8eb74c971553fd5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54365
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

This adds runtime checks that types which are aliases of ASN1_STRING
are in fact the expected ASN.1 type. Not comprehensive -- I got the
obvious ones from x509.h. These checks are not generally covered by
unit tests, except for one which was easy to test as a sanity-check.

Bug: 445
Change-Id: I8cd689b6b1e6121fce62c7f0ab25fee7e2a0b2ff
Update-Note: Various X.509 functions will now fail given the wrong ASN1_STRING subtype.
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54286
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

None of the built-in X509_LOOKUP functions support
X509_LOOKUP_by_fingerprint, X509_LOOKUP_by_issuer_serial, or
X509_LOOKUP_by_alias. We also made X509_LOOKUP_METHOD opaque and haven't
added the corresponding X509_LOOKUP_meth_set_* functions[*], so it is
currently impossible to usefully use these.

I found no callers which use or implement these, which makes sense. The
reason to implement X509_LOOKUP is to plug it into the X509_STORE, which
only cares about lookup by subject.

So just remove them. We can put it back later if it comes up.

[*] Actually it looks like we haven't added any way to make a custom
X509_LOOKUP_METHOD at all yet. I guess it hasn't come up yet.

Update-Note: Some unused functions were removed.
Change-Id: Ief8ba8ae9e5b339beeb59a7156e0258a7a9e70db
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54385
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

I put them under convenience functions because they're just wrappers
over existing getters and comparison functions. Used very occasionally,
but probably not important enough to put in the front of the header.

I const-corrected all parameters except X509_NAME. X509_NAME is still a
little tricky const-wise. (X509_NAME_cmp actually does take const names,
so it would compile, but it's misleading because it would actually
mutate the names.)

While here, I tidied it up a little. X509_issuer_and_serial_cmp isn't
really pulling its weight here and is forcing
X509_find_by_issuer_and_serial to stack-allocate a fake, mostly
uninitialized X509 object. The NULL check is also redundant because
STACK_OF(T) treats NULL as the empty list anyway.

With that, X509_issuer_and_serial_cmp is unused (I found no external
callers), so remove it. It's not a particularly problematic function, so
we can easily put it back, but if unused, one less to document.

Update-Note: Removed X509_issuer_and_serial_cmp as it's unused.
Bug: 426
Change-Id: I8785dea9b96265c1fea0c3c7b59e2979e223d819
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54386
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

This is far from all of it, but finishes a good chunk of bcm.c.

Bug: 516
Change-Id: If764e5af1c6b62e8342554502ecc4d563e44bc50
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54207
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

At the time, there was no documentation (or I just couldn't find it) on
the correct sysctl names to query CPU features on Apple aarch64
platforms, so it was unclear what the relationship was between
"hw.optional.arm.FEAT_SHA512" and "hw.optional.armv8_2_sha512". There is
documentation now:
https://developer.apple.com/documentation/kernel/1387446-sysctlbyname/determining_instruction_set_characteristics

However, the documented names weren't available in macOS 11, and some
Arm Macs did ship with macOS 11. So query both names for macOS 11 compat
and in case some future version of macOS removes the old names.

Change-Id: I671d47576721b4c172feeb2e3f138c6bc55e39d6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54445
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

The cookie is only needed in SSL_HANDSHAKE, so there's no need to retain
it for the lifetime of the connection. (SSL_HANDSHAKE is released after
the handshake completes.)

Back when DTLS1_COOKIE_LENGTH was 32, storing it inline made some sense.
Now that RFC 6347 increased the maximum to 255 bytes, just indirect it
with an Array<uint8_t>. Along the way, remove the DTLS1_COOKIE_LENGTH
checks. The new limit is the largest that fits in the length prefix, so
it's always redundant. In fact, the constant was one higher was allowed
anyway. Add some tests for the maximum length, as well as zero-length
cookies.

I considered just repurposing the plain cookie field, used in
HelloRetryRequest (as opposed to HelloVerifyRequest), as they're
mutually exclusive, even in DTLS 1.3. But, when we get to DTLS 1.3,
that'll get a little hairy because ssl_write_client_hello will need
extra checks to know whether hs->cookie is meant to go in the
ClientHello directly or in extensions.

Change-Id: I1afedc7ce31414879545701bf8fe4658657ba66f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54466
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

The destructor is automatic but, as a bonus, it becomes size_t-clean.
Costs us 8 more bytes of per-connection memory per outgoing message,
which isn't ideal but the previous commit saved even more, and DTLS
isn't as important as TLS in that regard.

Bug: 516
Change-Id: I69f881169088a11b9f09c4dc3577c47c4b48ce60
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54467
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

This doesn't seem to be used anywhere and unexporting it lets us make it
size_t-clean.

Update-Note: CONF_parse_list was removed. If parsing strings, use a
dedicated string library.

Bug: 516
Change-Id: I86fb353bb95268f7234fddf5563ecf2a27da99bd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54468
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Bug: 516
Change-Id: I97f98eb6bd3ebf1d517f63be9fe5df6e7e469f1a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54469
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Change-Id: Ied447b1e852b3b9b2bdc9617fa65a0cc1f425f7f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54470
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

ASN1_STRING's representation is confusing. For specifically INTEGER and
ENUMERATED, it lifts the sign bit into the type. While negative serial
numbers aren't actually valid, we do accept them and test code sometimes
uses these APIs to construct them, so amend
https://boringssl-review.googlesource.com/c/boringssl/+/54286 to allow
them.

I've also switched the CRL one to an assert. On reflection, returning 0
for a CRL lookup is failing closed, so it seems better to just continue
to accept the ASN1_STRING, even if it's the wrong type.

Change-Id: I1e81a89700ef14407a78bd3798cdae28a80640cd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54525
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

https://boringssl-review.googlesource.com/c/boringssl/+/54307 added just
the getters because no one was using the setters yet. But our long
setter *already* implements the int64 version, so just complete the
whole set and deprecate the old long-based APIs.

Change-Id: Ieb793f3cf90d4214c6416ba2f10e641c46403188
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54526
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

We spend a lot of effort implementing a big-endian sequence number
update, etc., when the sequence number is just a 64-bit counter. (Or
48-bit counter in DTLS because we currently retain the epoch
separately. We can probably tidy that a bit too, but I'll leave that
for later. Right now the DTLS record layer state is a bit entwined
with the TLS one.)

Just store it as uint64_t. This should also simplify
https://boringssl-review.googlesource.com/c/boringssl/+/54325 a little.

Change-Id: I95233f924a660bc523b21496fdc9211055b75073
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54505
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

We never ended up using these, or making them work with TLS 1.3 (which
has KeyUpdates and NewSessionTickets). It'd still be nice to have an
in-place API, but for now unwind these ones until we have time to give
it another go. Supporting TLS 1.3's post-handshake messages will
probably require a slightly more involved design.

(I suspect some of the seal_scatter bits in tls_record.cc can also be
simplified with these removed, but I've left them alone here.)

Update-Note: Removed some unused, experimental APIs.
Change-Id: Iad1245fa467cc6e599d20561f5db44d236219e06
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54527
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

OHTTP will also need EVP_HPKE_KEM_public_key_len because the OHTTP Key
Config structure simply concatenates the public key with other fields.
I don't think it needs EVP_HPKE_KEM_private_key_len, but at this point
we may as well add it for consistency.

Change-Id: I7fb8fc1cc5e65b8531fd9da53f18aa99ec85386c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54605
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>

These are the analogs of the test added in
https://boringssl-review.googlesource.com/c/boringssl/+/43784

Change-Id: I3bde53f53a865d2c298e62779e6a5cf3eec2bb60
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54607
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>

Update-Note: Clients will now require RSA server certificates used in
TLS 1.2 and earlier to include the keyEncipherment or digitalSignature
bit. keyEncipherment is required if using RSA key exchange.
digitalSignature is required if using ECDHE_RSA key exchange.

We already required this for each of ECDSA, TLS 1.3, and servers
verifying client certificates, so this just fills in the remaining hole.
Chrome has also enforced this for some time with publicly-trusted
certificates. For now, the SSL_set_enforce_rsa_key_usage API still
exists where we need to turn this off.

Fixed: 519
Change-Id: Ia440b00b60a224fa608702439aa120d633d81ddc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54606
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

In doing so, remove some X9.42 placeholder fields, since it's impossible
to set them. I switched dh_test.cc to the getters where it was easy, but
OpenSSL's new setters are so tedious that I just gave it access to the
internal struct.

With this, there are now only two public structs (DSA and RSA) that
reference CRYPTO_MUTEX. After that's removed, we can stop worrying about
pthread_rwlock_t feature flags in the public headers.

Update-Note: DH is now an opaque structure. Callers should use accessors
instead of accessing fields.

Change-Id: Ia53702f8ab58884a90d85718ee26eb03d062d234
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54625
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

CBB_add_asn1_uint64 doesn't work if you're encoding an implicitly-tagged
INTEGER. Take a leaf from Go cryptobyte and add a "with tag" variant,
rather than a "contents" variant, which is a little more convenient to
use. It also avoids us having to decide how to name the contents field.

Change-Id: I6072e55017230c513577c44c5a7ed86e778255b3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54685
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>

For testing vector sets, NIST supports fetching the expected results,
which can be helpful for debugging.

Change-Id: Ida1f884520b1d0600b369f705a184624fa055a52
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54665
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

SSL_generate_key_block is specific to TLS 1.2. It will output garbage in
TLS 1.3 (wrong KDF), so fail instead.

Update-Note: SSL_generate_key_block gets a new error case, but callers
that hit this were getting back useless output anyway.

Change-Id: Ib35384f902e03cd4654d25b39ca1808c4d878c3d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54705
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>

Today, every use of CBB, even CBB_init_fixed, requires a small, fallible
allocation to allocate the top-level CBB's cbb_buffer_st. We could embed
cbb_buffer_st directly in CBB, but then every child CBB wastes that
space, and needs an extra pointer to point back to the cbb_buffer_st.

But top-level and child CBBs have disjoint representations anyway. We
share a cbb_buffer_st pointer, but it's owning in one case and
borrowed in another. Child CBBs have length prefix information, but it's
never filed in for a top-level CBB.

Make this a sum type, with is_child as the discriminator and a union for
the two structures. (Elsewhere I've been trying to get rid of unions,
but this isn't using unions for type-punning, so it should valid even in
C++. We never access inactive arms.)

The implementation gains a few more branches, but now CBB_init_fixed is
infallible and allocation-less. I'm hoping this will let us more freely
convert functions like UTF8_putc into CBB because we don't need to worry
about cleanup or introducing allocations.

Change-Id: If0b28cd9e079418f35d5a614058c0aa73658822e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54645
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

CBB_init_fixed callers no longer need to check the return value, or
handle any cleanup. The hpke.c instance was even already (incorrectly at
the time) assuming this.

Change-Id: I2f4cb124454fc7ba7ff6d2075d99f537a58c6c6b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54647
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

cbb_add_utf8 is CBB-based, so it is bounds-checked.

Change-Id: Ib30272255894d7d3a35a164a5eefcdce9e8e7991
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54646
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Change-Id: Ib3495ddedec533b78884100ff2ff76f7370e7dc6
Bug: 505
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54105
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

We didn't actually have a test that would have caught
openssl/openssl#19388. This fixes this by
further generalizing the signing tests to run through all combinations
of {new object, reused object} x {X509_sign, X509_set_signature_value}.

In doing so, align X509_REQ_sign and X509_REQ_sign_ctx, which were
missing the TBS invalidation.

Change-Id: I5028aa2a00e71da0ebc7a03b23823b1337a56fca
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54726
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

We don't actually refresh the cache most of the time, just drop it and
live without it. The distinction isn't really visible by callers, but
make the comments accurate.

Change-Id: I7321695337125ca648ab57667564d9578a6fd549
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54727
Commit-Queue: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

A small follow-up to
https://boringssl-review.googlesource.com/c/boringssl/+/54105, to bring
it up to feature parity with the other aarch64 backends.
ID_AA64ISAR0_SHA2_512 seems to be present in FreeBSD 12.0, so I don't
believe this needs any compatibility ifdefs.

Bug: 505
Change-Id: I44891cf635adfd2ae26d4113fdc910549cf89193
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54725
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Robert Clausecker <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

I messed up the indentation in one place, and Shane Lontis pointed out
that the comment is slightly out of date now that there are two
codepaths involved.

Change-Id: I1be69f3f9a3835fffc4801b4464b9fb8ecb092cc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54745
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

While I'm here, use a fixed-size uint64_t in RSA_generate_key, rather
than unsigned long. This code also assumes unsigned long fits in
BN_ULONG, which is probably true on all platforms we care about, but
unnecessarily fussy.

The RSA_sign -> RSA_METHOD transition does require a cast. Go ahead and
check length/hash_nid consistency so we know it fits in the cast. This
does mean RSA_METHOD-backed keys are restricted to implementing digests
that we support, but that's probably fine. If anything, I think we
should try to shift away from RSA_METHOD as a story for custom keys.

Bug: 516
Change-Id: I3969da67d1daeff882279a534eb48ca831eb16cd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54465
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Newer versions of Clang have a warning to detect "suspicious" uses of
string concatenation, where they think a comma or so was missing. It
flags a false positive in x509_test.cc, which we can silence with
parentheses. Fuchsia builds with this warning enabled, so enable it to
catch future instances.

I couldn't find official documentation on when this was added, but
empirically it's in my clang-12 but not my clang-11. That's recent
enough that adding a version check seems prudent. Unfortunately,
version-detecting Clang is complex because AppleClang uses completely
different versions. There's a handy table on Wikipedia that maps them.

Change-Id: I503c21d39bb5c68dda9bda6da693c7208f3af561
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54785
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

I think it never picked up the fix in
https://boringssl-review.googlesource.com/c/boringssl/+/52345 for older
CMakes, but it doesn't have much reason to be part of the install in the
first place.

Bug: 524
Change-Id: Ifbb898b1e4686194c85e9902ee3d59d83b55b78a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54786
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

Previously the code just flipped one bit. But, empirically, modern Clang
will sometimes produce code that doesn't depend on the first 16 bytes of
the data; they are encoded in the instructions instead. Thus zero out
the full value.

(If Clang ever starts embedding complete values into the instruction
stream then we're going to have to do something more complex. Self tests
are a bit funny: the compiler could reasonably optimise them away
completely given that it sees all the inputs. Perhaps the inputs would
have to be moved into a different object file.)

Change-Id: I7bfb18cb7868def67fc791dcc31c5915c7728ac4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54825
Commit-Queue: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

This adds the link libraries in CMakeLists.txt file. If the libraries
are not in CMake files linking failed with undefined reference error.

Change-Id: I8f8352f6149a6332eedc0be51f36634890e3db60
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54805
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

Some third-party code requires it.

For now, I've just introduced a new hook on the method table. This is
rather goofy though. First, making EVP know about TLS is a layering
violation that OpenSSL introduced. They've since fixed this and added
EVP_PKEY_get1_encoded_public_key in OpenSSL 3.0, but callers expect the
TLS one to exist in OpenSSL 1.1.1, so implement that one.

Along the way, implement EC_KEY_oct2key from upstream, which is slightly
less tedious when you're already working in EC_KEY.

To make this third-party code work (and to write a test without dipping
out of EVP, or using the very tedious EVP_PKEY_paramgen API), we also
need to change EVP_PKEY_copy_parameters to work when the source EVP_PKEY
is empty, per upstream's 2986ecdc08016de978f1134315623778420b51e5.
OpenSSL's API has *multiple* levels of empty states to worry about!
Something to avoid when we get to rethinking this error-prone API.

Bug: b:238920520
Change-Id: I3fd99be560db313c1bf549a4e46ffccc31e746e1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54905
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>

Bug: 516
Change-Id: Iba2014da414658c08e42e0993912fa73848832d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54945
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

We shouldn't print different things depending on sizeof(long).

Change-Id: I5f97e17b838f8c9b119421b9ce0e93e95bd33dc0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54946
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>

These are mostly to ensure they don't crash, and that subsequent changes
don't unintentionally change the output. The current output is a little
weird but, for now, I've just captured the current output, bugs and all.

Change-Id: I9f1a4910ccc717764ef44551de9b3e0f9f2a1b40
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54947
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

First, stop trying to pre-size the buffer and just have bn_print
allocate the buffer internally. That removes the need for all the
algorithms being two-pass.

While I'm here, stop passing the unused ASN1_PCTX parameters in
everywhere.

As a side effect, this fixes a int vs size_t instance that flagged
-Wshorten-64-32, but it ended up being a much more substantial change.

Bug: 516
Change-Id: Ic210604de85539559b1ed88889ca6a08dfb20bde
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54948
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>