dtls 1.3: pad plaintext when too short for record header protection

wolfSSL · Sep 17, 2024 · e6e85a8 · e6e85a8
1 parent 69e57dc
commit e6e85a8
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 13 deletions.
diff --git a/src/dtls13.c b/src/dtls13.c
@@ -1209,6 +1209,11 @@ int Dtls13HandshakeAddHeader(WOLFSSL* ssl, byte* output,
     return 0;
 }
 
+int Dtls13MinimumRecordLength(WOLFSSL* ssl)
+{
+    return Dtls13GetRlHeaderLength(ssl, 1) + DTLS13_MIN_CIPHERTEXT;
+}
+
 /**
  * Dtls13EncryptRecordNumber() - encrypt record number in the header
  * @ssl: ssl object
@@ -1225,9 +1230,15 @@ int Dtls13EncryptRecordNumber(WOLFSSL* ssl, byte* hdr, word16 recordLength)
     if (ssl == NULL || hdr == NULL)
         return BAD_FUNC_ARG;
 
+#ifdef HAVE_NULL_CIPHER
+    /* Do not encrypt record numbers with null cipher. See RFC 9150 Sec 9 */
+    if (ssl->specs.bulk_cipher_algorithm == wolfssl_cipher_null)
+        return 0;
+#endif /*HAVE_NULL_CIPHER */
+
     /* we need at least a 16 bytes of ciphertext to encrypt record number see
        4.2.3*/
-    if (recordLength < Dtls13GetRlHeaderLength(ssl, 1) + DTLS13_MIN_CIPHERTEXT)
+    if (recordLength < Dtls13MinimumRecordLength(ssl))
         return BUFFER_ERROR;
 
     seqLength = (*hdr & DTLS13_LEN_BIT) ? DTLS13_SEQ_16_LEN : DTLS13_SEQ_8_LEN;
@@ -1453,17 +1464,24 @@ int Dtls13ParseUnifiedRecordLayer(WOLFSSL* ssl, const byte* input,
         hdrInfo->recordLength = inputSize - idx;
     }
 
-    /* minimum size for a dtls1.3 packet is 16 bytes (to have enough ciphertext
-       to create record number xor mask). (draft 43 - Sec 4.2.3) */
-    if (hdrInfo->recordLength < DTLS13_RN_MASK_SIZE)
-        return LENGTH_ERROR;
-    if (inputSize < idx + DTLS13_RN_MASK_SIZE)
-        return BUFFER_ERROR;
+#ifdef HAVE_NULL_CIPHER
+    /* Do not encrypt record numbers with null cipher. See RFC 9150 Sec 9 */
+    if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null)
+#endif /*HAVE_NULL_CIPHER */
+    {
+        /* minimum size for a dtls1.3 packet is 16 bytes (to have enough
+         * ciphertext to create record number xor mask).
+         * (draft 43 - Sec 4.2.3) */
+        if (hdrInfo->recordLength < DTLS13_RN_MASK_SIZE)
+            return LENGTH_ERROR;
+        if (inputSize < idx + DTLS13_RN_MASK_SIZE)
+            return BUFFER_ERROR;
 
-    ret = Dtls13EncryptDecryptRecordNumber(ssl, seqNum, seqLen, input + idx,
-        DEPROTECT);
-    if (ret != 0)
-        return ret;
+        ret = Dtls13EncryptDecryptRecordNumber(ssl, seqNum, seqLen, input + idx,
+            DEPROTECT);
+        if (ret != 0)
+            return ret;
+    }
 
     if (seqLen == DTLS13_SEQ_16_LEN) {
         hdrInfo->seqHiPresent = 1;

diff --git a/src/tls13.c b/src/tls13.c
@@ -3199,6 +3199,7 @@ typedef struct BuildMsg13Args {
     word32 idx;
     word32 headerSz;
     word16 size;
+    word32 paddingSz;
 } BuildMsg13Args;
 
 static void FreeBuildMsg13Args(WOLFSSL* ssl, void* pArgs)
@@ -3304,7 +3305,14 @@ int BuildTls13Message(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
             args->sz++;
             /* Authentication data at the end. */
             args->sz += ssl->specs.aead_mac_size;
-
+#ifdef WOLFSSL_DTLS13
+            /* Pad to minimum length */
+            if (ssl->options.dtls &&
+                    args->sz < (word32)Dtls13MinimumRecordLength(ssl)) {
+                args->paddingSz = Dtls13MinimumRecordLength(ssl) - args->sz;
+                args->sz = Dtls13MinimumRecordLength(ssl);
+            }
+#endif
             if (sizeOnly)
                 return (int)args->sz;
 
@@ -3348,6 +3356,9 @@ int BuildTls13Message(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
 
             /* The real record content type goes at the end of the data. */
             output[args->idx++] = (byte)type;
+            /* Double check that any necessary padding is zero'd out */
+            XMEMSET(output + args->idx, 0, args->paddingSz);
+            args->idx += args->paddingSz;
 
             ssl->options.buildMsgState = BUILD_MSG_ENCRYPT;
         }
@@ -3393,7 +3404,8 @@ int BuildTls13Message(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
 #ifdef WOLFSSL_DTLS13
                 if (ret == 0 && ssl->options.dtls) {
                     /* AAD points to the header. Reuse the variable  */
-                    ret = Dtls13EncryptRecordNumber(ssl, (byte*)aad, (word16)args->sz);
+                    ret = Dtls13EncryptRecordNumber(ssl, (byte*)aad,
+                                                    (word16)args->sz);
                 }
 #endif /* WOLFSSL_DTLS13 */
             }

diff --git a/wolfssl/internal.h b/wolfssl/internal.h
@@ -6828,6 +6828,7 @@ WOLFSSL_LOCAL int Dtls13RlAddCiphertextHeader(WOLFSSL* ssl, byte* out,
     word16 length);
 WOLFSSL_LOCAL int Dtls13RlAddPlaintextHeader(WOLFSSL* ssl, byte* out,
     enum ContentType content_type, word16 length);
+WOLFSSL_LOCAL int Dtls13MinimumRecordLength(WOLFSSL* ssl);
 WOLFSSL_LOCAL int Dtls13EncryptRecordNumber(WOLFSSL* ssl, byte* hdr,
     word16 recordLength);
 WOLFSSL_LOCAL int Dtls13IsUnifiedHeader(byte header_flags);