This was an area of some ambiguity in the TLSv1.2 spec where only
signature_algorithms exists. I believe it was common practice for
implementations to not check the signatures in certificates for conformance with
this (certainly that is the way OpenSSL behaves). The TLSv1.3 spec seems to be
more explicit about this. I would expect our TLSv1.2 implementation to continue
to operate as it did before so this additional checking of signatures in
certificates where only the signature_algorithms extensions is present should
only apply to TLSv1.3.
I don't see any code to do this in has_usable_cert so this looks like a
potential bug. Although possibly it was left out on purpose.
Yes. Currently this check is missing for both TLSv1.2 and TLSv1.3. I feel we may need to add for both TLSv1.2 and TLSv1.3, because TLSv1.2 RFC 5246 also mandates to do this check.
If the client provided a "signature_algorithms" extension, then all certificates provided by the server MUST be signed by a hash/signature algorithm pair that appears in that extension.
Fix should be something like https://github.com/raja-ashok/openssl/commit/2eb916d6048f54fd6109329d31850f8eb4407780
Atleast we should add this check for TLSv1.3. Otherwise cert with rsa_pkcsv1_5_xxx signature getting selected even if client has not included rsa_pkcsv1_5_xxx in signature_algorithm ext, but included rsa_pss_rsae_xxx. This looks weird.
Apart from this I am having one more doubt, that TLSv1.3 RFC 8446 says certificate with legacy signature(rsa_pkcsv1_5_sha1 and ecdsa_sha1) can be selected if no other valid certificate present on TLS1.3 server. But in tls_choose_sigalgs() function for TLSv1.3 we are currently skipping all SHA1 based and RSA_PKCSv1_5 based signature algorithm. I feel instead of avoiding we should consider as a low priority in this function for selecting legacy certificates with rsa_pkcsv1_5 and ecdsa_sha1.
TLS 1.3 servers MUST NOT offer a SHA-1 signed certificate unless no valid certificate chain can be produced without it (see Section 4.4.2.2).
Ben Kaduk may have a view on this who implemented this code:
https://github.com/openssl/openssl/pull/5068/commits/e639c37bddea48334cb45d88d407c655641e1a35
And also requesting Ben Kaduk to put some light on it.