Place start of ClientHello correctly when calculating binder for DTLS…

… 1.3 Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24426)
quictls · Jun 4, 2024 · 6e1de2b · 6e1de2b
1 parent b659d17
commit 6e1de2b
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 128 deletions.
diff --git a/crypto/packet.c b/crypto/packet.c
@@ -364,6 +364,12 @@ int WPACKET_finish(WPACKET *pkt)
 }
 
 int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)
+{
+    return WPACKET_start_sub_packet_at_offset_len__(pkt, lenbytes, 0);
+}
+
+int WPACKET_start_sub_packet_at_offset_len__(WPACKET *pkt, size_t lenbytes,
+                                             size_t offset)
 {
     WPACKET_SUB *sub;
     unsigned char *lenchars;
@@ -381,7 +387,7 @@ int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)
 
     sub->parent = pkt->subs;
     pkt->subs = sub;
-    sub->pwritten = pkt->written + lenbytes;
+    sub->pwritten = pkt->written + lenbytes + offset;
     sub->lenbytes = lenbytes;
 
     if (lenbytes == 0) {
@@ -391,7 +397,8 @@ int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)
 
     sub->packet_len = pkt->written;
 
-    if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars))
+    if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars)
+            || (offset > 0 && !WPACKET_allocate_bytes(pkt, offset, &lenchars)))
         return 0;
 
     return 1;

diff --git a/include/internal/common.h b/include/internal/common.h
@@ -184,6 +184,10 @@ __owur static ossl_inline int ossl_assert_int(int expr, const char *exprstr,
                            (c)[1]=(unsigned char)(((l)>> 8)&0xff), \
                            (c)[2]=(unsigned char)(((l)    )&0xff)),(c)+=3)
 
+#define l3n2(c,l)       (l =((uint64_t)(*((c)++)))<<16, \
+                         l|=((uint64_t)(*((c)++)))<< 8, \
+                         l|=((uint64_t)(*((c)++))))
+
 static ossl_inline int ossl_ends_with_dirsep(const char *path)
 {
     if (*path != '\0')

diff --git a/include/internal/packet.h b/include/internal/packet.h
@@ -764,6 +764,18 @@ int WPACKET_finish(WPACKET *pkt);
  */
 int WPACKET_fill_lengths(WPACKET *pkt);
 
+/*
+ * Initialise a new sub-packet. Additionally |lenbytes| of data is preallocated
+ * at the current position in |pkt| to store the sub-packets length once we know
+ * it. The sub-packet start is set at |offset| from current position in |pkt|
+ * Don't call this directly. Use the convenience macros below instead.
+ */
+int WPACKET_start_sub_packet_at_offset_len__(WPACKET *pkt, size_t lenbytes,
+                                             size_t offset);
+
+#define WPACKET_start_sub_packet_u24_at_offset(pkt, offset) \
+    WPACKET_start_sub_packet_at_offset_len__((pkt), 3, (offset))
+
 /*
  * Initialise a new sub-packet. Additionally |lenbytes| of data is preallocated
  * at the start of the sub-packet to store its length once we know it. Don't

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
@@ -1535,8 +1535,8 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md,
     static const unsigned char resumption_label[] = "\x72\x65\x73\x20\x62\x69\x6E\x64\x65\x72";
     /* ASCII: "ext binder", in hex for EBCDIC compatibility */
     static const unsigned char external_label[] = "\x65\x78\x74\x20\x62\x69\x6E\x64\x65\x72";
-    const unsigned char *label;
-    size_t bindersize, labelsize, hashsize;
+    const unsigned char *label, *msgbodystart;
+    size_t bindersize, labelsize, hashsize, msgbodylen;
     int hashsizei = EVP_MD_get_size(md);
     int ret = -1;
     int usepskfored = 0;
@@ -1654,7 +1654,23 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md,
         }
     }
 
-    if (EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0
+    if (SSL_CONNECTION_IS_DTLS(s)) {
+        msgbodystart = msgstart + DTLS1_HM_HEADER_LENGTH;
+        msgbodylen = binderoffset - DTLS1_HM_HEADER_LENGTH;
+    } else {
+        msgbodystart = msgstart + SSL3_HM_HEADER_LENGTH;
+        msgbodylen = binderoffset - SSL3_HM_HEADER_LENGTH;
+    }
+
+    /*
+     * RFC9147 (DTLSv1.3)
+     * The transcript consists of complete TLS Handshake messages
+     * (reassembled as necessary). Note that this requires removing the
+     * message_seq, fragment_offset, and fragment_length fields to create
+     * the Handshake structure.
+     */
+    if (EVP_DigestUpdate(mctx, msgstart, SSL3_HM_HEADER_LENGTH) <= 0
+            || EVP_DigestUpdate(mctx, msgbodystart, msgbodylen) <= 0
             || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         goto err;

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
@@ -1206,9 +1206,8 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt,
 
     msgstart = WPACKET_get_curr(pkt) - msglen;
 
-    if (dores
-            && tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL,
-                                 resbinder, s->session, 1, 0) != 1) {
+    if (dores && tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL,
+                                   resbinder, s->session, 1, 0) != 1) {
         /* SSLfatal() already called */
         return EXT_RETURN_FAIL;
     }