Test that swapping the first app data record with Finished msg works
authorMatt Caswell <matt@openssl.org>
Mon, 25 Jul 2022 11:39:52 +0000 (12:39 +0100)
committerMatt Caswell <matt@openssl.org>
Wed, 10 Aug 2022 10:42:29 +0000 (11:42 +0100)
If the first app data record arrives before the Finished message we should
be able to buffer it and move on to the Finished message.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18976)

test/dtlstest.c
test/ssltestlib.c
test/ssltestlib.h

index 1d7b105fb6ac5560fc0f81bc62dce7a38cc36cd0..f5c9dcfcd87331b55ef657684761c96d9807c62f 100644 (file)
@@ -328,6 +328,93 @@ static int test_dtls_duplicate_records(void)
     return testresult;
 }
 
+/*
+ * Test that swapping an app data record so that it is received before the
+ * Finished message still works.
+ */
+static int test_swap_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];
+
+    if (!TEST_true(create_ssl_ctx_pair(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 1(NST, CCS) datagram 2(Finished) */
+    if (!TEST_int_gt(SSL_accept(sssl), 0))
+        goto end;
+
+    /* Send flight 5: app data */
+    if (!TEST_int_eq(SSL_write(sssl, msg, sizeof(msg)), (int)sizeof(msg)))
+        goto end;
+
+    bio = SSL_get_wbio(sssl);
+    if (!TEST_ptr(bio)
+            || !TEST_true(mempacket_swap_recent(bio)))
+        goto end;
+
+    /*
+     * Recv flight 4 (datagram 1): NST, CCS, + flight 5: app data
+     *      + flight 4 (datagram 2): Finished
+     */
+    if (!TEST_int_gt(SSL_connect(cssl), 0))
+        goto end;
+
+    /* The app data should be buffered already */
+    if (!TEST_int_eq(SSL_pending(cssl), (int)sizeof(msg))
+            || !TEST_true(SSL_has_pending(cssl)))
+        goto end;
+
+    /*
+     * Recv flight 5 (app data)
+     * We already buffered this so it should be available.
+     */
+    if (!TEST_int_eq(SSL_read(cssl, buf, sizeof(buf)), (int)sizeof(msg)))
+        goto end;
+
+    testresult = 1;
+ end:
+    SSL_free(cssl);
+    SSL_free(sssl);
+    SSL_CTX_free(cctx);
+    SSL_CTX_free(sctx);
+    return testresult;
+}
+
 int setup_tests(void)
 {
     if (!TEST_ptr(cert = test_get_argument(0))
@@ -338,6 +425,7 @@ int setup_tests(void)
     ADD_ALL_TESTS(test_dtls_drop_records, TOTAL_RECORDS);
     ADD_TEST(test_cookie);
     ADD_TEST(test_dtls_duplicate_records);
+    ADD_TEST(test_swap_app_data);
 
     return 1;
 }
index 456afdf4716e072d97cf63260447b87a2f387054..44d435454bf7d1bf08b98f14b0e22fa03f564d13 100644 (file)
@@ -435,6 +435,39 @@ static int mempacket_test_read(BIO *bio, char *out, int outl)
     return outl;
 }
 
+/* Take the last and penultimate packets and swap them around */
+int mempacket_swap_recent(BIO *bio)
+{
+    MEMPACKET_TEST_CTX *ctx = BIO_get_data(bio);
+    MEMPACKET *thispkt;
+    int numpkts = sk_MEMPACKET_num(ctx->pkts);
+
+    /* We need at least 2 packets to be able to swap them */
+    if (numpkts <= 1)
+        return 0;
+
+    /* Get the penultimate packet */
+    thispkt = sk_MEMPACKET_value(ctx->pkts, numpkts - 2);
+    if (thispkt == NULL)
+        return 0;
+
+    if (sk_MEMPACKET_delete(ctx->pkts, numpkts - 2) != thispkt)
+        return 0;
+
+    /* Re-add it to the end of the list */
+    thispkt->num++;
+    if (sk_MEMPACKET_insert(ctx->pkts, thispkt, numpkts - 1) <= 0)
+        return 0;
+
+    /* We also have to adjust the packet number of the other packet */
+    thispkt = sk_MEMPACKET_value(ctx->pkts, numpkts - 2);
+    if (thispkt == NULL)
+        return 0;
+    thispkt->num--;
+
+    return 1;
+}
+
 int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
                           int type)
 {
index 17b278219a6e766cb336fca8d11b8d4d6820a1a9..b47004f62e3919922979a2afa42904c319b17ac7 100644 (file)
@@ -46,6 +46,7 @@ void bio_s_always_retry_free(void);
 #define MEMPACKET_CTRL_GET_DROP_REC         (3 << 15)
 #define MEMPACKET_CTRL_SET_DUPLICATE_REC    (4 << 15)
 
+int mempacket_swap_recent(BIO *bio);
 int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
                           int type);