Lifetime elision infers the wrong bounds. The code could be fixed by using
explicit lifetime annotations, but it's safer to just avoid the issue
altogether. The problem doesn't seem to affect any of the current code; the
problem was only noticed when trying to use the removed code for new uses.
Have the `limbs!` macros accept the limbs least-significant-first to be consistent
with how they are represented in memory. This has the nice side effect of making
them much simpler.
In TLS 1.3, if the client doesn't read from the server, the server might hang
from a filled buffer while waiting for the client to read. Instead we avoid
flushing the NewSessionTicket until there is a write from the server.
Update-Note: This delays the flushing of the NewSessionTicket until the first
write. Consumers may need to force an empty write to send the tickets if they
aren't writing any data to the client.
Change-Id: Iec92043567e9a68c0a250533b7745eddeeae2341
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/34948
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Including ssl.h is quite a chunk of code and #defines, so we've tried to
limit its spread internally in the interests of code hygine given that
we have a multi-billion-line repo.
However, header files that mention enums from ssl.h currently need to
include ssl.h. For example, your class may have static class member
functions intended to be callbacks, and they need to be class members
because they'll call other private methods.
C cannot predeclare enums, but C++ can if you explicitly type them.
Sadly C doesn't support explicit types. So option one is to move the
enums into base.h. That works, but the enums properly live in ssl.h and
reading the header file is a lot clearer if you don't have to jump
around to see all the pieces.
So option two (this change) is to explicitly type and predelcare the
enums in base.h for C++ only. The worry now is that C and C++ might
disagree about the type of the enums. However, this has already
happened: at least for |ssl_private_key_result_t|, g++ thinks that it's
an |int| (without any explicit type) and gcc thinks that it's an
|unsigned|. At least they're the same length, I guess?
So, to make sure that this doesn't slip any more, this change also adds
|ssl_test_c.c| which tests that C views the enums as having the same
size as an |int|, at least.
Change-Id: I8248583ec997021f8226d5a798609f6afc96dac4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35664
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
The common name fallback does not interact well with name constraints.
Until we remove this fallback, we must resolve this conflict.
Blindly applying name constraints to the common name will reject
"decorative" common names that aren't intended to be hostnames (e.g.
[0]). We need to guess based on format whether the common name is a DNS
name. It is important this same check is applied to *both* name
constraints and name matching, which means the OpenSSL version (see
5bd5dcd49605ca2aa7931599894302a3ac4b0b04,
d02d80b2e80adfdde49f76cf7c7af4e013f45005, and
55a6250f1e7336e8a7d89fb609eb23398715ff6f) is unsuitable as a
compatibility data point.
In theory we could limit this to chains with name constraints, which are
uncommon, but X509_check_host sees only the leaf. We must apply it
uniformly. That means a strict check risks problems with malformed
non-WebPKI setups like [1].
For a first pass, mirror Go's behavior. Like Go, rather than run
SAN-less DNS-like common names through name constraints, we simply
reject all such certificates. Name constraints now exclude all leaf
certificates that can trigger the common name fallback. They are rare
enough that we can hopefully hold them to a higher standard.
Note this does not make misclassified decorative common names any worse,
compared to the checking the name constraint. Such names would not have
matched the constraint anyway.
Update-Note: This can may cause two kinds of errors:
1. Leaf certificates whose chain contains a name constraint and lack
SANs may be rejected with X509_V_ERR_NAME_CONSTRAINTS_WITHOUT_SANS.
2. Leaf certificates which use the common name fallback and verify
against an insufficiently DNS-looking hostname may fail with
X509_V_ERR_HOSTNAME_MISMATCH.
In both cases, the fix is to include the subjectAltName in the
certificate, rather than rely on the common name fallback. (Refining the
heuristic is also an option, but the two failure modes pull it in
opposite directions, so this is tricky.)
[0] https://github.com/golang/go/issues/24151
[1] https://github.com/GoogleCloudPlatform/cloudsql-proxy/issues/194
Change-Id: If25557de428768292a14ba3bdeeffbd74e3a3bf8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35665
Reviewed-by: Adam Langley <agl@google.com>
If the error is unknown, we should not return a static buffer. See also
c0a445a9f279d8c4a519b58e52a50112f2341070 from upstream.
Change-Id: I23e1a3b9e29b34ab3dff41b8a58155683bbb9bd2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35684
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This aligns with the Go crypto/x509 behavior and reduces the cases when
the SAN to CN fallback occurs. If the certificate is new enough to have
a SAN list, even if it only contains email or IP addresses, it is
reasonable to assume the certificate is new enough that the common name
is not a DNS name.
Update-Note: Our certificate verification is getting slightly stricter.
Change-Id: I9e3466d8dd8a722405c546181a589f797efa43f9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35647
Reviewed-by: Adam Langley <agl@google.com>
This flag is backwards. We want to check the common name less, not more. See if
anything was actually relying on this.
Update-Note: X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT is now ignored.
Change-Id: I8288d57540f8117059e58d72cc173aa4d3077fb6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35646
Reviewed-by: Adam Langley <agl@google.com>
cryptography.io uses this and it's also the correct behavior. Ideally it would
be default, but start with just adding the flag. See also
dd60efea955e41a6f0926f93ec1503c6f83c4e58 from upstream.
Change-Id: I9e13cdbfd44c904ba5bd69a5a66c68c4b7596867
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35645
Reviewed-by: Adam Langley <agl@google.com>
This simplifies building against cryptography.io, which expects
ENGINE_free to return something.
Change-Id: Id1590abab7f47dae6b3a9d593fa7b0fe371c9912
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35644
Reviewed-by: Adam Langley <agl@google.com>
This will allow edge servers to pass judgement on the ClientHello before
completing the handoff process. This also means that edge servers will
now enforce ClientHello well-formedness — previously that check didn't
occur until the handshaker tried to parse the handoff submission.
Change-Id: I9804ac0224632b4b4381c1a81f434d188e0b9376
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35584
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The RSA-PSS salt length was not being copied, and copying an Ed25519
EVP_MD_CTX did not work.
This is rather pointless (an EVP_PKEY_CTX is just a bundle of
parameters), and it's unlikely anyone ever will use this. But since
OpenSSL's EVP_PKEY signing API reuses EVP_MD_CTX and EVP_MD_CTX_copy_ex
is plausible in that scenario, we're stuck making EVP_MD_CTX_copy_ex
reachable for EVP_PKEY too. That then implies EVP_PKEY_dup should exist,
and if it exists we should be testing it.
Change-Id: I189435d0c716a83f58e1d8ac4abc2c409ecfea64
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35626
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
