ssl: server selecting wrong certificate when multiple `cert_keys` are configured

[v0idpwn]

Describe the bug
When a TLS 1.3 server is configured with multiple certificate/key pairs via certs_keys, the server may select a certificate whose public key type is incompatible with the client's signature_algorithms extension. This causes the handshake to fail with insufficient_security / no_suitable_signature_algorithm even though a suitable certificate exists in the server's configuration.

In my tests, this happened when I used two certs: one ECDSA and one RSA, both signed by the same RSA CA. When using a client that only accepts RSA, ssl prioritizes using ECDSA and then fails with:

TLS server: In state start at tls_handshake_1_3.erl:1505 generated SERVER ALERT: Fatal - Insufficient Security
      - no_suitable_signature_algorithm

To Reproduce

Have a server listening with multiple cert types:

CertsKeys = [
  #{cert => ECDSACert, key => ECDSAKey},
  #{cert => RSACert,   key => RSAKey}
],
ServerOpts = [{certs_keys, CertsKeys}, {versions, ['tlsv1.3']}],
{ok, LSock} = ssl:listen(Port, ServerOpts).

Connect to it with restricted signature_algs:

ClientOpts = [
  {verify, verify_peer},
  {cacerts, [RSACACert]},
  {versions, ['tlsv1.3']},
  {signature_algs, [rsa_pss_rsae_sha256]},
  {signature_algs_cert, [rsa_pkcs1_sha256, rsa_pss_rsae_sha256]}
],
{ok, Sock} = ssl:connect(Host, Port, ClientOpts).

A script in this repository does all the setup and compares the erlang server with openssl_s, which succeeds with the same client configuration.

Expected behavior
Sucessfully handshake, like openssl_s.

Affected versions
At least 27.3 and main.

[v0idpwn]

I have a potential fix here, where I do a select_sign_algo check earlier in the process, and skip a pair if it can't be used.

It makes my test pass, I'm not fully confident in the approach, sharing in case it makes the investigation easier :)

[IngelaAndin]

Actually TLS-1.2 has a quirk : "Fixed DH certificates MAY be signed with any hash/signature
algorithm pair appearing in the hash_sign extension. The names
DH_DSS, DH_RSA, ECDH_ECDSA, and ECDH_RSA are historical."

And I guess your counter part may not handle that correctly. So our implementation is probably just choosing the preferred certificate algorithm.

You can test this for instance by doing something like:

 Filter = [{key_exchange, fun(dh_rsa) -> false;
			        (ecdh_ecdsa) -> false;
			        (ecdh_rsa) -> false;
				(_) -> true
				end}]).
And setting the ciphers option to  ssl:filter_cipher_suites(ssl:cipher_suites(default, 'tlsv1.2'), Filter).
 

And then you will be able to interop.

So it is my suspicion that this is not our bug,.

[v0idpwn]

Thanks for looking into this, @IngelaAndin. In my example above the client is using tls1.3 and the signature_algorithms extension, so I'm not sure if the TLS 1.2 quirk applies. Client source, server source

[IngelaAndin]

@v0idpwn Then I will take a closer look at your fix suggestion, then it could be a bug. It was just that description fitted this quirk that I happened to come across while performing negative testing. I guess I skimmed the description too much and missed you only used TLS-1.3

[IngelaAndin]

@v0idpwn Would you pleas make a PR of your suggestion. I think it makes sense. Maybe you could have a look at also adding a test to openssl_client_cert_SUITE.erl to get an interop test.

[v0idpwn]

Sounds good, will do! I'll just do another pass on it first :)