From: Matt Caswell <matt@openssl.org>
Date: Fri, 2 May 2025 15:40:50 +0000 (+0100)
Subject: Add a test for app data received too early
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1afcc27f945272f29905c32ba725757470fb0e6e;p=thirdparty%2Fopenssl.git

Add a test for app data received too early

Add a test for app data which was received prior to the Finished is read
correctly, and that if we continue to read we get the expected result.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from https://github.com/openssl/openssl/pull/27543)
---

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 270dfcde965..ddddc09c951 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -551,6 +551,10 @@ int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int writing, int ret,
     return ret;
 }
 
+/*
+ * Release data from a record.
+ * If length == 0 then we will release the entire record.
+ */
 int ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr, size_t length)
 {
     assert(rr->length >= length);
diff --git a/test/dtlstest.c b/test/dtlstest.c
index 011d8775c15..5a6a6b7aaeb 100644
--- a/test/dtlstest.c
+++ b/test/dtlstest.c
@@ -587,6 +587,105 @@ static int test_swap_records(int idx)
     return testresult;
 }
 
+static int test_duplicate_app_data(void)
+{
+    SSL_CTX *sctx = NULL, *cctx = NULL;
+    SSL *sssl = NULL, *cssl = NULL;
+    int testresult = 0;
+    BIO *bio;
+    char msg[] = { 0x00, 0x01, 0x02, 0x03 };
+    char buf[10];
+    int ret;
+
+    if (!TEST_true(create_ssl_ctx_pair(NULL, DTLS_server_method(),
+                                       DTLS_client_method(),
+                                       DTLS1_VERSION, 0,
+                                       &sctx, &cctx, cert, privkey)))
+        return 0;
+
+#ifndef OPENSSL_NO_DTLS1_2
+    if (!TEST_true(SSL_CTX_set_cipher_list(cctx, "AES128-SHA")))
+        goto end;
+#else
+    /* Default sigalgs are SHA1 based in <DTLS1.2 which is in security level 0 */
+    if (!TEST_true(SSL_CTX_set_cipher_list(sctx, "AES128-SHA:@SECLEVEL=0"))
+            || !TEST_true(SSL_CTX_set_cipher_list(cctx,
+                                                  "AES128-SHA:@SECLEVEL=0")))
+        goto end;
+#endif
+
+    if (!TEST_true(create_ssl_objects(sctx, cctx, &sssl, &cssl,
+                                      NULL, NULL)))
+        goto end;
+
+    /* Send flight 1: ClientHello */
+    if (!TEST_int_le(SSL_connect(cssl), 0))
+        goto end;
+
+    /* Recv flight 1, send flight 2: ServerHello, Certificate, ServerHelloDone */
+    if (!TEST_int_le(SSL_accept(sssl), 0))
+        goto end;
+
+    /* Recv flight 2, send flight 3: ClientKeyExchange, CCS, Finished */
+    if (!TEST_int_le(SSL_connect(cssl), 0))
+        goto end;
+
+    /* Recv flight 3, send flight 4: datagram 0(NST, CCS) datagram 1(Finished) */
+    if (!TEST_int_gt(SSL_accept(sssl), 0))
+        goto end;
+
+    bio = SSL_get_wbio(sssl);
+    if (!TEST_ptr(bio))
+        goto end;
+
+    /*
+     * Send flight 4 (cont'd): datagram 2(app data)
+     * + datagram 3 (app data duplicate)
+     */
+    if (!TEST_int_eq(SSL_write(sssl, msg, sizeof(msg)), (int)sizeof(msg)))
+        goto end;
+
+    if (!TEST_true(mempacket_dup_last_packet(bio)))
+        goto end;
+
+    /* App data comes before NST/CCS */
+    if (!TEST_true(mempacket_move_packet(bio, 0, 2)))
+        goto end;
+
+    /*
+     * Recv flight 4 (datagram 2): app data + flight 4 (datagram 0): NST, CCS, +
+     *      + flight 4 (datagram 1): Finished
+     */
+    if (!TEST_int_gt(SSL_connect(cssl), 0))
+        goto end;
+
+    /*
+     * Read flight 4 (app data)
+     */
+    if (!TEST_int_eq(SSL_read(cssl, buf, sizeof(buf)), (int)sizeof(msg)))
+        goto end;
+
+    if (!TEST_mem_eq(buf, sizeof(msg), msg, sizeof(msg)))
+        goto end;
+
+    /*
+     * Read flight 4, datagram 3. We expect the duplicated app data to have been
+     * dropped, with no more data available
+     */
+    if (!TEST_int_le(ret = SSL_read(cssl, buf, sizeof(buf)), 0)
+            || !TEST_int_eq(SSL_get_error(cssl, ret), SSL_ERROR_WANT_READ))
+        goto end;
+
+    testresult = 1;
+ end:
+    SSL_free(cssl);
+    SSL_free(sssl);
+    SSL_CTX_free(cctx);
+    SSL_CTX_free(sctx);
+
+    return testresult;
+}
+
 /* Confirm that we can create a connections using DTLSv1_listen() */
 static int test_listen(void)
 {
@@ -658,6 +757,7 @@ int setup_tests(void)
     ADD_TEST(test_just_finished);
     ADD_ALL_TESTS(test_swap_records, 4);
     ADD_TEST(test_listen);
+    ADD_TEST(test_duplicate_app_data);
 
     return 1;
 }
diff --git a/test/helpers/ssltestlib.c b/test/helpers/ssltestlib.c
index e9a51c9c30d..10618905c4c 100644
--- a/test/helpers/ssltestlib.c
+++ b/test/helpers/ssltestlib.c
@@ -536,6 +536,40 @@ int mempacket_move_packet(BIO *bio, int d, int s)
     return 1;
 }
 
+int mempacket_dup_last_packet(BIO *bio)
+{
+    MEMPACKET_TEST_CTX *ctx = BIO_get_data(bio);
+    MEMPACKET *thispkt, *duppkt;
+    int numpkts = sk_MEMPACKET_num(ctx->pkts);
+
+    /* We can only duplicate a packet if there is at least 1 pending */
+    if (numpkts <= 0)
+        return 0;
+
+    /* Get the last packet */
+    thispkt = sk_MEMPACKET_value(ctx->pkts, numpkts - 1);
+    if (thispkt == NULL)
+        return 0;
+
+    duppkt = OPENSSL_malloc(sizeof(*duppkt));
+    if (duppkt == NULL)
+        return 0;
+
+    *duppkt = *thispkt;
+    duppkt->data = OPENSSL_memdup(thispkt->data, thispkt->len);
+    if (duppkt->data == NULL) {
+        mempacket_free(duppkt);
+        return 0;
+    }
+    duppkt->num++;
+    if (sk_MEMPACKET_insert(ctx->pkts, duppkt, numpkts) <= 0) {
+        mempacket_free(duppkt);
+        return 0;
+    }
+
+    return 1;
+}
+
 int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
                           int type)
 {
diff --git a/test/helpers/ssltestlib.h b/test/helpers/ssltestlib.h
index 5369bb6e925..16f679cf5f0 100644
--- a/test/helpers/ssltestlib.h
+++ b/test/helpers/ssltestlib.h
@@ -70,6 +70,7 @@ void bio_s_maybe_retry_free(void);
 
 int mempacket_swap_epoch(BIO *bio);
 int mempacket_move_packet(BIO *bio, int d, int s);
+int mempacket_dup_last_packet(BIO *bio);
 int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
                           int type);