[Bug]: RFC 8446 violation : WolfSSL doesn't return Alert when receiving encrypted out of order handshake messages

[aeyno]

Version

5.8.4

Description

A WolfSSL TLS 1.3 client receiving encrypted out of order messages will detect the error and log "Out of order message, fatal" without sending an alert.

According to the RFC 8446 section 4: A peer which receives a handshake message in an unexpected order MUST abort the handshake with an "unexpected_message" alert. This means that WolfSSL should return an UnexpectedMessage alert in this case.

Impact

RFC violation

Expected behavior

WolfSSL client should send a UnexpectedMessage Alert and abort the connection.

Reproduction steps

Here is an example of a TLS 1.3 handshake that triggers the described behavior :

• Wait for a ClientHello
• Send a ServerHello
• Compute the handshake keys, and instead of sending EncryptedExtensions, send an encrypted ServerHello message
  You should see the WolfSSL client aborting the connection without an Alert.

Acknowledgements

This bug was found thanks to the tlspuffin fuzzer designed and developed by the tlspuffin team:

• Max Ammann
• Olivier Demengeon - Loria, Inria
• Tom Gouville - Loria, Inria
• Lucca Hirschi - Loria, Inria
• Steve Kremer - Loria, Inria
• Michael Mera - Loria, Inria

[kareem-wolfssl]

Hi @aeyno,

Thanks for the report and my apologies for the delayed response.
We're looking into this now.
Would it be possible for you to attach a reproducer?

[aeyno]

Building a simple reproducer for this is quite hard since it require computing all the cryptographic material of a normal TLS 1.3 handshake and the tool I use to reproduce this is quite hard to package. I will try to provide a packet capture of the message sequence, but I am currently struggling to export an SSLKEYLOG file from WolfSSL, can you point me to a documentation on how to do that ?

[kareem-wolfssl]

To export an SSLKEYLOG from wolfSSL using TLS 1.3, you'll need to build wolfSSL with --enable-keylog-export. This will export secrets to a file named sslkeylog.log in the current directory.

[kareem-wolfssl]

Hi @aeyno,

I've added a case to TranslateErrorToAlert for this which should cover the cases where OUT_OF_ORDER_E is being returned.

Here is a reproducer I came up with, with the patch applied I see an unexpected_message alert sent as expected:

diff --git a/src/tls13.c b/src/tls13.c
index 9f25f47a0..bba87be7c 100644
--- a/src/tls13.c
+++ b/src/tls13.c
@@ -7743,6 +7743,179 @@ static int SendTls13EncryptedExtensions(WOLFSSL* ssl)
     return ret;
 }

+/* Send an encrypted ServerHello message instead of EncryptedExtensions.
+ * This is a non-standard modification for testing purposes.
+ *
+ * ssl  The SSL/TLS object.
+ * returns 0 on success, otherwise failure.
+ */
+static int SendTls13EncryptedServerHello(WOLFSSL* ssl)
+{
+    int    ret;
+    byte*  output;
+    word16 length;
+    word32 idx = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ;
+    int    sendSz;
+
+    WOLFSSL_START(WC_FUNC_SERVER_HELLO_SEND);
+    WOLFSSL_ENTER("SendTls13EncryptedServerHello");
+
+    ssl->options.buildingMsg = 1;
+    ssl->keys.encryptionOn = 1;
+
+#ifdef WOLFSSL_DTLS13
+    if (ssl->options.dtls) {
+        idx = DTLS_RECORD_HEADER_SZ + DTLS_HANDSHAKE_HEADER_SZ;
+    }
+#endif /* WOLFSSL_DTLS13 */
+
+    /* Derive the handshake secret now that we are at first message to be
+     * encrypted under the keys.
+     */
+    if ((ret = DeriveHandshakeSecret(ssl)) != 0)
+        return ret;
+    if ((ret = DeriveTls13Keys(ssl, handshake_key,
+                               ENCRYPT_AND_DECRYPT_SIDE, 1)) != 0)
+        return ret;
+
+    /* Setup encrypt/decrypt keys for following messages. */
+#ifdef WOLFSSL_EARLY_DATA
+    if ((ret = SetKeysSide(ssl, ENCRYPT_SIDE_ONLY)) != 0)
+        return ret;
+    if (ssl->earlyData != process_early_data) {
+        if ((ret = SetKeysSide(ssl, DECRYPT_SIDE_ONLY)) != 0)
+            return ret;
+    }
+#else
+    if ((ret = SetKeysSide(ssl, ENCRYPT_AND_DECRYPT_SIDE)) != 0)
+        return ret;
+#endif
+
+#ifdef WOLFSSL_DTLS13
+    if (ssl->options.dtls) {
+        w64wrapper epochHandshake = w64From32(0, DTLS13_EPOCH_HANDSHAKE);
+        ssl->dtls13Epoch = epochHandshake;
+
+        ret = Dtls13SetEpochKeys(
+            ssl, epochHandshake, ENCRYPT_AND_DECRYPT_SIDE);
+        if (ret != 0)
+            return ret;
+    }
+#endif /* WOLFSSL_DTLS13 */
+
+    /* Protocol version, server random, session id, cipher suite, compression
+     * and extensions.
+     */
+    length = VERSION_SZ + RAN_LEN + ENUM_LEN + ssl->session->sessionIDSz +
+             SUITE_LEN + COMP_LEN;
+    ret = TLSX_GetResponseSize(ssl, server_hello, &length);
+    if (ret != 0)
+        return ret;
+    sendSz = (int)(idx + length);
+    /* Encryption always on. */
+    sendSz += MAX_MSG_EXTRA;
+
+    /* Check buffers are big enough and grow if needed. */
+    if ((ret = CheckAvailableSize(ssl, sendSz)) != 0)
+        return ret;
+
+    /* Get position in output buffer to write new message to. */
+    output = GetOutputBuffer(ssl);
+
+    /* Put the record and handshake headers on. */
+    AddTls13Headers(output, length, server_hello, ssl);
+
+    /* The protocol version must be TLS v1.2 for middleboxes. */
+    output[idx++] = ssl->version.major;
+    output[idx++] = ssl->options.dtls ? DTLSv1_2_MINOR : TLSv1_2_MINOR;
+
+    /* Use the same server random that was sent in the first ServerHello */
+    XMEMCPY(output + idx, ssl->arrays->serverRandom, RAN_LEN);
+    idx += RAN_LEN;
+
+    output[idx++] = ssl->session->sessionIDSz;
+    if (ssl->session->sessionIDSz > 0) {
+        XMEMCPY(output + idx, ssl->session->sessionID, ssl->session->sessionIDSz);
+        idx += ssl->session->sessionIDSz;
+    }
+
+    /* Chosen cipher suite */
+    output[idx++] = ssl->options.cipherSuite0;
+    output[idx++] = ssl->options.cipherSuite;
+
+    /* Compression not supported in TLS v1.3. */
+    output[idx++] = 0;
+
+    /* Extensions */
+    ret = TLSX_WriteResponse(ssl, output + idx, server_hello, NULL);
+    if (ret != 0)
+        return ret;
+
+    /* Hash the handshake message before encryption */
+#ifdef WOLFSSL_DTLS13
+    if (ssl->options.dtls) {
+        ret = Dtls13HashHandshake(
+            ssl,
+            output + Dtls13GetRlHeaderLength(ssl, 0),
+            (word16)sendSz - Dtls13GetRlHeaderLength(ssl, 0));
+    }
+    else
+#endif /* WOLFSSL_DTLS13 */
+    {
+        if (ret == 0)
+            ret = HashOutput(ssl, output, sendSz, 0);
+    }
+
+    if (ret != 0)
+        return ret;
+
+#if defined(WOLFSSL_CALLBACKS) || defined(OPENSSL_EXTRA)
+    if (ssl->hsInfoOn)
+        AddPacketName(ssl, "EncryptedServerHello");
+    if (ssl->toInfoOn) {
+        ret = AddPacketInfo(ssl, "EncryptedServerHello", handshake, output,
+                      sendSz, WRITE_PROTO, 0, ssl->heap);
+        if (ret != 0)
+            return ret;
+    }
+#endif
+
+#ifdef WOLFSSL_DTLS13
+    if (ssl->options.dtls) {
+        ssl->options.buildingMsg = 0;
+        ret = Dtls13HandshakeSend(ssl, output, (word16)sendSz, (word16)idx,
+                                  server_hello, 1);
+
+        if (ret == 0)
+            ssl->options.serverState = SERVER_ENCRYPTED_EXTENSIONS_COMPLETE;
+
+        WOLFSSL_LEAVE("SendTls13EncryptedServerHello", ret);
+        WOLFSSL_END(WC_FUNC_SERVER_HELLO_SEND);
+
+        return ret;
+    }
+#endif /* WOLFSSL_DTLS13 */
+
+    /* This handshake message is always encrypted. */
+    sendSz = BuildTls13Message(ssl, output, sendSz, output + RECORD_HEADER_SZ,
+                               (int)(idx - RECORD_HEADER_SZ),
+                               handshake, 1, 0, 0);
+    if (sendSz < 0)
+        return sendSz;
+
+    ssl->buffers.outputBuffer.length += (word32)sendSz;
+    ssl->options.buildingMsg = 0;
+    ssl->options.serverState = SERVER_ENCRYPTED_EXTENSIONS_COMPLETE;
+
+    if (!ssl->options.groupMessages)
+        ret = SendBuffered(ssl);
+
+    WOLFSSL_LEAVE("SendTls13EncryptedServerHello", ret);
+    WOLFSSL_END(WC_FUNC_SERVER_HELLO_SEND);
+
+    return ret;
+}
+
 #ifndef NO_CERTS
 /* handle generation TLS v1.3 certificate_request (13) */
 /* Send the TLS v1.3 CertificateRequest message.
@@ -14664,7 +14837,8 @@ int wolfSSL_accept_TLSv13(WOLFSSL* ssl)
             }
     #endif

-            if ((ssl->error = SendTls13EncryptedExtensions(ssl)) != 0) {
+            /* Modified: Send encrypted ServerHello instead of EncryptedExtensions */
+            if ((ssl->error = SendTls13EncryptedServerHello(ssl)) != 0) {
                 WOLFSSL_ERROR(ssl->error);
                 return WOLFSSL_FATAL_ERROR;
             }

./examples/server/server -v 4
./examples/client/client -v 4