Merge pull request #7214 from julek-wolfssl/zd/17314

DTLS sequence number and cookie fixes

Author: SparkiDev

src/dtls.c

@@ -114,6 +114,7 @@ int DtlsIgnoreError(int err)
     case SOCKET_ERROR_E:
     case WANT_READ:
     case WANT_WRITE:
+    case COOKIE_ERROR:
         return 0;
     default:
         return 1;
@@ -207,6 +208,13 @@ static int CreateDtls12Cookie(const WOLFSSL* ssl, const WolfSSL_CH* ch,
 {
     int ret;
     Hmac cookieHmac;
+
+    if (ssl->buffers.dtlsCookieSecret.buffer == NULL ||
+            ssl->buffers.dtlsCookieSecret.length == 0) {
+        WOLFSSL_MSG("Missing DTLS 1.2 cookie secret");
+        return COOKIE_ERROR;
+    }
+
     ret = wc_HmacInit(&cookieHmac, ssl->heap, ssl->devId);
     if (ret == 0) {
         ret = wc_HmacSetKey(&cookieHmac, DTLS_COOKIE_TYPE,

src/internal.c

@@ -7456,15 +7456,14 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
 
 #if defined(WOLFSSL_DTLS) && !defined(NO_WOLFSSL_SERVER)
     if (ssl->options.dtls && ssl->options.side == WOLFSSL_SERVER_END) {
-        if (!IsAtLeastTLSv1_3(ssl->version)) {
-                ret = wolfSSL_DTLS_SetCookieSecret(ssl, NULL, 0);
-                if (ret != 0) {
-                    WOLFSSL_MSG("DTLS Cookie Secret error");
-                    return ret;
-                }
+        /* Initialize both in case we allow downgrading. */
+        ret = wolfSSL_DTLS_SetCookieSecret(ssl, NULL, 0);
+        if (ret != 0) {
+            WOLFSSL_MSG("DTLS Cookie Secret error");
+            return ret;
         }
 #if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE)
-        else {
+        if (IsAtLeastTLSv1_3(ssl->version)) {
             ret = wolfSSL_send_hrr_cookie(ssl, NULL, 0);
             if (ret != WOLFSSL_SUCCESS) {
                 WOLFSSL_MSG("DTLS1.3 Cookie secret error");

src/tls13.c

@@ -3546,6 +3546,12 @@ int CreateCookieExt(const WOLFSSL* ssl, byte* hash, word16 hashSz,
         return BAD_FUNC_ARG;
     }
 
+    if (ssl->buffers.tls13CookieSecret.buffer == NULL ||
+            ssl->buffers.tls13CookieSecret.length == 0) {
+        WOLFSSL_MSG("Missing DTLS 1.3 cookie secret");
+        return COOKIE_ERROR;
+    }
+
     /* Cookie Data = Hash Len | Hash | CS | KeyShare Group */
     cookie[cookieSz++] = (byte)hashSz;
     XMEMCPY(cookie + cookieSz, hash, hashSz);
@@ -4693,7 +4699,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
 }
 
 #if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_NO_CLIENT)
-static int Dtls13DoDowngrade(WOLFSSL* ssl)
+static int Dtls13ClientDoDowngrade(WOLFSSL* ssl)
 {
     int ret;
     if (ssl->dtls13ClientHello == NULL)
@@ -5099,7 +5105,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
             if (ssl->options.dtls) {
                 ssl->chVersion.minor = DTLSv1_2_MINOR;
                 ssl->version.minor = DTLSv1_2_MINOR;
-                ret = Dtls13DoDowngrade(ssl);
+                ret = Dtls13ClientDoDowngrade(ssl);
                 if (ret != 0)
                     return ret;
             }
@@ -5193,7 +5199,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
         if (ssl->options.dtls) {
             ssl->chVersion.minor = DTLSv1_2_MINOR;
             ssl->version.minor = DTLSv1_2_MINOR;
-            ret = Dtls13DoDowngrade(ssl);
+            ret = Dtls13ClientDoDowngrade(ssl);
             if (ret != 0)
                 return ret;
         }
@@ -5266,7 +5272,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
 
 #ifdef WOLFSSL_DTLS13
             if (ssl->options.dtls) {
-                ret = Dtls13DoDowngrade(ssl);
+                ret = Dtls13ClientDoDowngrade(ssl);
                 if (ret != 0)
                     return ret;
             }
@@ -6321,6 +6327,12 @@ int TlsCheckCookie(const WOLFSSL* ssl, const byte* cookie, word16 cookieSz)
     byte cookieType = 0;
     byte macSz = 0;
 
+    if (ssl->buffers.tls13CookieSecret.buffer == NULL ||
+            ssl->buffers.tls13CookieSecret.length == 0) {
+        WOLFSSL_MSG("Missing DTLS 1.3 cookie secret");
+        return COOKIE_ERROR;
+    }
+
 #if !defined(NO_SHA) && defined(NO_SHA256)
     cookieType = SHA;
     macSz = WC_SHA_DIGEST_SIZE;
@@ -6695,6 +6707,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
      * wolfSSL_accept_TLSv13 when changing this one. */
     if (IsDtlsNotSctpMode(ssl) && ssl->options.sendCookie &&
             !ssl->options.dtlsStateful) {
+        DtlsSetSeqNumForReply(ssl);
         ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0, NULL);
         if (ret != 0 || !ssl->options.dtlsStateful) {
             *inOutIdx += helloSz;

tests/api.c

@@ -68547,6 +68547,81 @@ static int test_dtls_dropped_ccs(void)
 #endif
     return EXPECT_RESULT();
 }
+
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS) \
+    && !defined(WOLFSSL_NO_TLS12)
+static int test_dtls_seq_num_downgrade_check_num(byte* ioBuf, int ioBufLen,
+        byte seq_num)
+{
+    EXPECT_DECLS;
+    DtlsRecordLayerHeader* dtlsRH;
+    byte sequence_number[8];
+
+    XMEMSET(&sequence_number, 0, sizeof(sequence_number));
+
+    ExpectIntGE(ioBufLen, sizeof(*dtlsRH));
+    dtlsRH = (DtlsRecordLayerHeader*)ioBuf;
+    ExpectIntEQ(dtlsRH->type, handshake);
+    ExpectIntEQ(dtlsRH->pvMajor, DTLS_MAJOR);
+    ExpectIntEQ(dtlsRH->pvMinor, DTLSv1_2_MINOR);
+    sequence_number[7] = seq_num;
+    ExpectIntEQ(XMEMCMP(sequence_number, dtlsRH->sequence_number,
+            sizeof(sequence_number)), 0);
+
+    return EXPECT_RESULT();
+}
+#endif
+
+/*
+ * Make sure that we send the correct sequence number after a HelloVerifyRequest
+ * and after a HelloRetryRequest. This is testing the server side as it is
+ * operating statelessly and should copy the sequence number of the ClientHello.
+ */
+static int test_dtls_seq_num_downgrade(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS) \
+    && !defined(WOLFSSL_NO_TLS12)
+    WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
+    WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+        wolfDTLSv1_2_client_method, wolfDTLS_server_method), 0);
+
+    /* CH1 */
+    ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+    ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.s_buff,
+            test_ctx.s_len, 0), TEST_SUCCESS);
+    /* HVR */
+    ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
+    ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.c_buff,
+            test_ctx.c_len, 0), TEST_SUCCESS);
+    /* CH2 */
+    ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+    ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.s_buff,
+            test_ctx.s_len, 1), TEST_SUCCESS);
+    /* Server first flight */
+    ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
+    ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.c_buff,
+            test_ctx.c_len, 1), TEST_SUCCESS);
+
+    ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+
+    wolfSSL_free(ssl_c);
+    wolfSSL_CTX_free(ctx_c);
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
 /**
  * Make sure we don't send RSA Signature Hash Algorithms in the
  * CertificateRequest when we don't have any such ciphers set.
@@ -70789,6 +70864,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_dtls_client_hello_timeout_downgrade),
     TEST_DECL(test_dtls_client_hello_timeout),
     TEST_DECL(test_dtls_dropped_ccs),
+    TEST_DECL(test_dtls_seq_num_downgrade),
     TEST_DECL(test_certreq_sighash_algos),
     TEST_DECL(test_revoked_loaded_int_cert),
     TEST_DECL(test_dtls_frag_ch),