rabbitmq / rabbitmq-java-client

RabbitMQ Java client
https://www.rabbitmq.com/java-client.html
Other
1.24k stars 575 forks source link

Handshake error when connecting to AWS NLB using TLS 1.2 and NIO #1280

Closed sergio91pt closed 6 months ago

sergio91pt commented 6 months ago

Describe the bug

When using a recent version of rabbitmq-java-client, we cannot connect to a AWS Load Balancer using TLS 1.2 and NIO due to an "handshake error".

We were unable to replicate using a local RMQ instance with TLS 1.2, only when connecting to the load balancer. It also does not occur when connecting using TLS 1.3 or when using TLS 1.2 without NIO.

Downgrading rabbitmq-java-client to 5.13.1 fixes the issue, so we believe it is caused by https://github.com/rabbitmq/rabbitmq-java-client/pull/716.

Reproduction steps

  1. Setup an AWS Network Load balancer in "front of" your RabbitMQ cluster (ELBSecurityPolicy-TLS13-1-2-2021-06).
  2. Connect to the LB using useNio() and useSslProtocol() (defaults to TLSv1.2 and trusts every certificate).
  3. Application receives the following exception:
javax.net.ssl|ERROR|10|main|2024-03-28 18:51:38.875 UTC|null:-1|Fatal (UNEXPECTED_MESSAGE): Unexpected handshake message: server_hello (
"throwable" : {
  javax.net.ssl.SSLProtocolException: Unexpected handshake message: server_hello
    at java.base/sun.security.ssl.Alert.createSSLException(Unknown Source)
    at java.base/sun.security.ssl.Alert.createSSLException(Unknown Source)
    at java.base/sun.security.ssl.TransportContext.fatal(Unknown Source)
    at java.base/sun.security.ssl.TransportContext.fatal(Unknown Source)
    at java.base/sun.security.ssl.TransportContext.fatal(Unknown Source)
    at java.base/sun.security.ssl.HandshakeContext.dispatch(Unknown Source)
    at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction.run(Unknown Source)
    at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction.run(Unknown Source)
    at java.base/java.security.AccessController.doPrivileged(Unknown Source)
    at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask.run(Unknown Source)
    at com.rabbitmq.client.impl.nio.SslEngineHelper.runDelegatedTasks(SslEngineHelper.java:85)
    at com.rabbitmq.client.impl.nio.SslEngineHelper.unwrap(SslEngineHelper.java:120)
    at com.rabbitmq.client.impl.nio.SslEngineHelper.doHandshake(SslEngineHelper.java:60)
    at com.rabbitmq.client.impl.nio.SocketChannelFrameHandlerFactory.create(SocketChannelFrameHandlerFactory.java:112)
    at com.rabbitmq.client.impl.recovery.RecoveryAwareAMQConnectionFactory.newConnection(RecoveryAwareAMQConnectionFactory.java:63)
    at com.rabbitmq.client.impl.recovery.AutorecoveringConnection.init(AutorecoveringConnection.java:160)
    at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1227)
    at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1184)
    at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1322)

Expected behavior

Application is able to connect to the load balancer with TLSv1.2 and NIO.

Additional context

This occurs after client_hello and server_hello. TLS 1.2 is negotiated. It occurs just after the certificate chain is received.

Details

``` Starting TLS handshake Initial handshake status is NEED_WRAP Handshake status is NEED_WRAP Wrapping... Handshake status is NEED_WRAP before wrapping SSL engine result is Status = OK HandshakeStatus = NEED_UNWRAP bytesConsumed = 0 bytesProduced = 365 sequenceNumber = 0 after wrapping Wrote 365 byte(s) Handshake status is NEED_UNWRAP Unwrapping... Handshake status is NEED_UNWRAP before unwrapping Cipher in position 0 Reading from channel Read 5084 byte(s) from channel SSL engine result is Status = OK HandshakeStatus = NEED_TASK bytesConsumed = 100 bytesProduced = 0 after unwrapping Running delegated task javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.853 UTC|null:-1|Consuming ServerHello handshake message ( "ServerHello": { "server version" : "TLSv1.2", "random" : "BFA3287C7BA4DFED44C112B2580EBBE2BD26BAA852C4D49F444F574E47524401", "session id" : "AF42501CF28AA3F4F6A4A7DD855314586F0CCBBD810F826A4FFEDA27B4CF7D6D", "cipher suite" : "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256(0xC02F)", "compression methods" : "00", "extensions" : [ "server_name (0)": { }, "ec_point_formats (11)": { "formats": [uncompressed] }, "renegotiation_info (65,281)": { "renegotiated connection": [] }, "extended_master_secret (23)": { } ] } ) javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.853 UTC|null:-1|Ignore unavailable extension: supported_versions javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.853 UTC|null:-1|Negotiated protocol version: TLSv1.2 javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Consumed extension: renegotiation_info javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Consumed extension: server_name javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Ignore unavailable extension: max_fragment_length javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Ignore unavailable extension: status_request javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Consumed extension: ec_point_formats javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Ignore unavailable extension: status_request_v2 javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Consumed extension: extended_master_secret javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.854 UTC|null:-1|Ignore unavailable extension: session_ticket javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Consumed extension: renegotiation_info javax.net.ssl|WARNING|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore impact of unsupported extension: server_name javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore unavailable extension: max_fragment_length javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore unavailable extension: status_request javax.net.ssl|WARNING|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore impact of unsupported extension: ec_point_formats javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore unavailable extension: application_layer_protocol_negotiation javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore unavailable extension: status_request_v2 javax.net.ssl|WARNING|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore impact of unsupported extension: extended_master_secret javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore unavailable extension: session_ticket javax.net.ssl|WARNING|10|main|2024-03-28 18:51:38.855 UTC|null:-1|Ignore impact of unsupported extension: renegotiation_info Setting cipherIn position to 100 (limit is 5084) SSL engine result is Status = OK HandshakeStatus = NEED_TASK bytesConsumed = 4984 bytesProduced = 0 after unwrapping Running delegated task javax.net.ssl|DEBUG|10|main|2024-03-28 18:51:38.868 UTC|null:-1|Consuming server Certificate handshake message ( "Certificates": [ // Omitted for brevity ] ) Clearing cipherIn because all bytes have been read and unwrapped SSL engine result is Status = OK HandshakeStatus = NEED_TASK bytesConsumed = 100 bytesProduced = 0 after unwrapping Running delegated task javax.net.ssl|ERROR|10|main|2024-03-28 18:51:38.875 UTC|null:-1|Fatal (UNEXPECTED_MESSAGE): Unexpected handshake message: server_hello ( "throwable" : { javax.net.ssl.SSLProtocolException: Unexpected handshake message: server_hello at java.base/sun.security.ssl.Alert.createSSLException(Unknown Source) at java.base/sun.security.ssl.Alert.createSSLException(Unknown Source) at java.base/sun.security.ssl.TransportContext.fatal(Unknown Source) at java.base/sun.security.ssl.TransportContext.fatal(Unknown Source) at java.base/sun.security.ssl.TransportContext.fatal(Unknown Source) at java.base/sun.security.ssl.HandshakeContext.dispatch(Unknown Source) at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction.run(Unknown Source) at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction.run(Unknown Source) at java.base/java.security.AccessController.doPrivileged(Unknown Source) at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask.run(Unknown Source) at com.rabbitmq.client.impl.nio.SslEngineHelper.runDelegatedTasks(SslEngineHelper.java:85) at com.rabbitmq.client.impl.nio.SslEngineHelper.unwrap(SslEngineHelper.java:120) at com.rabbitmq.client.impl.nio.SslEngineHelper.doHandshake(SslEngineHelper.java:60) at com.rabbitmq.client.impl.nio.SocketChannelFrameHandlerFactory.create(SocketChannelFrameHandlerFactory.java:112) at com.rabbitmq.client.impl.recovery.RecoveryAwareAMQConnectionFactory.newConnection(RecoveryAwareAMQConnectionFactory.java:63) at com.rabbitmq.client.impl.recovery.AutorecoveringConnection.init(AutorecoveringConnection.java:160) at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1227) at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1184) at com.rabbitmq.client.ConnectionFactory.newConnection(ConnectionFactory.java:1322) ```

bmleite commented 6 months ago

I believe the cause for the exception is that the SslEngineHelper implementation calls the delegated task twice for the server_hello handshake message. The reason for this double call is due to the changes introduced in https://github.com/rabbitmq/rabbitmq-java-client/pull/716, in particular these ones.

Affected code: https://github.com/rabbitmq/rabbitmq-java-client/blob/ca510a078092a3c64b1219feac527bc738b63313/src/main/java/com/rabbitmq/client/impl/nio/SslEngineHelper.java#L111-L153

The code flow for this particular scenario is as follows:

I'm not familiar with all the TLS handshake flows and details but, from a mere ByteArray perspective, I don't think we need the if (newPosition == cipherIn.limit()) condition. I've tested using an instrumented version of the SslEngineHelper with the following changes and it worked:

if (unwrapResult.getHandshakeStatus() == NEED_TASK) {
    handshakeStatus = runDelegatedTasks(sslEngine);
    // removed the IF...ELSE condition and now always updates the cipherIn position
    cipherIn.position(positionBeforeUnwrapping + unwrapResult.bytesConsumed());
} else {
    handshakeStatus = unwrapResult.getHandshakeStatus();
}

Please check if this analysis makes sense. 🙏

michaelklishin commented 6 months ago

@bmleite it does make sense. Please submit a PR and we will test it some more. Thank you!