Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cipher preference may break SNI if certificates have different key types #6099

Closed
izstas opened this issue Mar 24, 2021 · 6 comments · Fixed by #6243 or #6246
Closed

Cipher preference may break SNI if certificates have different key types #6099

izstas opened this issue Mar 24, 2021 · 6 comments · Fixed by #6243 or #6246
Assignees

Comments

@izstas
Copy link

izstas commented Mar 24, 2021

Jetty version
10.0.1

Java version
15.0.2

OS type/version
Windows 10.0.19042.867

Description
When the keystore contains certificates with keys of different types, cipher preference seems to take precedence over SNI.

Example setup:

  • a keystore with:
    • an EC certificate with CN=ec.example.com, SAN = DNS:ec.example.com
    • an RSA certificate with CN=rsa.example.com, SAN = DNS:rsa.example.com
  • jetty.sslContext.sniRequired=false (the default)

Test 1 (fail)

curl -v -k --tlsv1.2 --resolve "rsa.example.com:443:127.0.0.1" https://rsa.example.com/

Expected result
The certificate for rsa.example.com is presented in the TLS handshake.

Actual result
ECDHE-ECDSA-AES256-GCM-SHA384 is the selected cipher.
The certificate for ec.example.com is presented in the TLS handshake, and a 400 Invalid SNI is returned. The following can be seen in the logs:

DEBUG:oejus.SslContextFactory:qtp1287934450-76: SNI matching for type=host_name (0), value=rsa.example.com
DEBUG:oejus.SslContextFactory:qtp1287934450-76: SNI host name rsa.example.com
DEBUG:oejus.SniX509ExtendedKeyManager:qtp1287934450-76: Chose delegate alias ec/EC on sun.security.ssl.SSLEngineImpl@44f79dfb

Test 2 (success)

curl -v -k --tlsv1.2 --cipher ECDHE-RSA-AES256-GCM-SHA384 --resolve "rsa.example.com:443:127.0.0.1" https://rsa.example.com/

Actual result
The certificate for rsa.example.com is presented in the TLS handshake.

Workaround

Setting jetty.sslContext.sniRequired=true fixes this behavior.

@sbordet
Copy link
Contributor

sbordet commented Mar 25, 2021

@izstas can you please attach the DEBUG logs for both cases?

@izstas
Copy link
Author

izstas commented Mar 30, 2021

@sbordet

sniRequiredFalse.log - the log where the Test 1 and then Test 2 are executed, with jetty.sslContext.sniRequired=false
sniRequiredTrue.log - the log where the Test 1 is executed, with jetty.sslContext.sniRequired=true

I also can confirm that Jetty 10.0.2 shows the same behavior.

@sbordet sbordet self-assigned this May 6, 2021
@sbordet
Copy link
Contributor

sbordet commented May 6, 2021

The problem is as follows.

The server receives a ClientHello with the signature_algorithms extension.
These algorithms were only 2 in TLS 1.1, but became many more in TLS 1.2 and TLS 1.3, and in TLS 1.3 they are called "signature schemes" and are tied to a specific function (i.e. it's not just a default function such as "sign the SHA-1 hash of a random number concatenated with server parameters", but the function itself may change depending on the scheme).

The OpenJDK implementation loops over the signature_algorithms received and calls X509ExtendedKeyManager.chooseEngineServerAlias(...), once per algorithm, until it receives back a non-null alias.
Failing to get an alias results in the handshake to be aborted by the OpenJDK implementation.

Unfortunately, X509ExtendedKeyManager in general, and our SniX509ExtendedKeyManager cannot know how many times will be called.

In this particular case, it will be called the first time with keyType="EC"; we first ask to the OpenJDK X509ExtendedKeyManager the aliases suitable for that keyType, and we get back the ec alias.
Then we retrieve the SNI, in this case rsa.domain.com, and we call the SNI selector, by default SslContextFactory.Server.sniSelect(...), where we figure out that the SNI does not match the certificate correspondent to the ec alias.

At this point we have no match, but we don't know if we will ever be called again.
Our only option is therefore to either return a null alias, hoping to be called again, or to delegate to the OpenJDK X509ExtendedKeyManager, which picks the first certificate, and that's why the OP sees the wrong certificate.

Turns out that if jetty.sslContext.sniRequired=true we return a null alias (with the idea to fail the TLS handshake rather than defaulting to a random certificate), and it just happens that we are called again (because there are other signature schemes), and few calls later we can actually find a certificate that happens to match the SNI.

@gregw ideas?

sbordet added a commit that referenced this issue May 6, 2021
…fferent key types.

Updated the logic in SslContextFactory.Server.sniSelect(...) to check if there is
any certificate that matches, and if so return a null alias in the hope to be called
again and pick the right alias for the SNI.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented May 6, 2021

@gregw I have a tentative fix at #6243.

@sbordet
Copy link
Contributor

sbordet commented May 6, 2021

@izstas are you able to try branch jetty-9.4.x-6099-sni-with-different-keyTypes to verify if it solves the problem for you?

@izstas
Copy link
Author

izstas commented May 7, 2021

@sbordet, thank you, I have tried jetty-9.4.x-6099-sni-with-different-keyTypes as of ee1924c and it solves the problem.

sbordet added a commit that referenced this issue May 10, 2021
…fferent key types.

Updated the logic in SslContextFactory.Server.sniSelect(...) to check if there is
any certificate that matches, and if so return a null alias in the hope to be called
again and pick the right alias for the SNI.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue May 10, 2021
…fferent key types.

Updated the logic in SslContextFactory.Server.sniSelect(...) to check if there is
any certificate that matches, and if so return a null alias in the hope to be called
again and pick the right alias for the SNI.

Signed-off-by: Simone Bordet <[email protected]>
(cherry picked from commit 6829691)
sbordet added a commit that referenced this issue May 10, 2021
…fferent key types.

Updated the logic in SslContextFactory.Server.sniSelect(...) to check if there is
any certificate that matches, and if so return a null alias in the hope to be called
again and pick the right alias for the SNI.

Signed-off-by: Simone Bordet <[email protected]>
(cherry picked from commit 6829691)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants