Test that swapping the first app data record with Finished msg works

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 openssl#18976)

(cherry picked from commit d87e99d)

test/dtlstest.c

@@ -105,6 +105,84 @@ static int test_dtls_unprocessed(int testidx)
     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 (!create_ssl_ctx_pair(DTLS_server_method(),
+                             DTLS_client_method(),
+                             DTLS1_VERSION, 0,
+                             &sctx, &cctx, cert, privkey))
+        return 0;
+
+    if (!SSL_CTX_set_cipher_list(cctx, "AES128-SHA"))
+        goto end;
+
+    if (!create_ssl_objects(sctx, cctx, &sssl, &cssl,
+                            NULL, NULL))
+        goto end;
+
+    /* Send flight 1: ClientHello */
+    if (!(SSL_connect(cssl) <= 0))
+        goto end;
+
+    /* Recv flight 1, send flight 2: ServerHello, Certificate, ServerHelloDone */
+    if (!(SSL_accept(sssl) <= 0))
+        goto end;
+
+    /* Recv flight 2, send flight 3: ClientKeyExchange, CCS, Finished */
+    if (!(SSL_connect(cssl) <= 0))
+        goto end;
+
+    /* Recv flight 3, send flight 4: datagram 1(NST, CCS) datagram 2(Finished) */
+    if (!(SSL_accept(sssl) > 0))
+        goto end;
+
+    /* Send flight 5: app data */
+    if (!(SSL_write(sssl, msg, sizeof(msg)) == (int)sizeof(msg)))
+        goto end;
+
+    bio = SSL_get_wbio(sssl);
+    if (!bio || !mempacket_swap_recent(bio))
+        goto end;
+
+    /*
+     * Recv flight 4 (datagram 1): NST, CCS, + flight 5: app data
+     *      + flight 4 (datagram 2): Finished
+     */
+    if (!(SSL_connect(cssl) > 0))
+        goto end;
+
+    /* The app data should be buffered already */
+    if (!(SSL_pending(cssl) == (int)sizeof(msg))
+            || !SSL_has_pending(cssl))
+        goto end;
+
+    /*
+     * Recv flight 5 (app data)
+     * We already buffered this so it should be available.
+     */
+    if (!(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 main(int argc, char *argv[])
 {
     BIO *err = NULL;
@@ -124,6 +202,7 @@ int main(int argc, char *argv[])
     CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
 
     ADD_ALL_TESTS(test_dtls_unprocessed, NUM_TESTS);
+    ADD_TEST(test_swap_app_data);
 
     testresult = run_tests(argv[0]);
 

test/ssltestlib.c

@@ -252,6 +252,7 @@ typedef struct mempacket_test_ctx_st {
     unsigned int currrec;
     unsigned int currpkt;
     unsigned int lastpkt;
+    unsigned int injected;
     unsigned int noinject;
 } MEMPACKET_TEST_CTX;
 
@@ -344,7 +345,8 @@ static int mempacket_test_read(BIO *bio, char *out, int outl)
     if (outl > thispkt->len)
         outl = thispkt->len;
 
-    if (thispkt->type != INJECT_PACKET_IGNORE_REC_SEQ) {
+    if (thispkt->type != INJECT_PACKET_IGNORE_REC_SEQ
+            && ctx->injected) {
         /*
          * Overwrite the record sequence number. We strictly number them in
          * the order received. Since we are actually a reliable transport
@@ -386,6 +388,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)
 {
@@ -400,6 +435,7 @@ int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
     if (pktnum >= 0) {
         if (ctx->noinject)
             return -1;
+        ctx->injected = 1;
     } else {
         ctx->noinject = 1;
     }

test/ssltestlib.h

@@ -31,6 +31,7 @@ void bio_s_mempacket_test_free(void);
 #define INJECT_PACKET                   1
 #define INJECT_PACKET_IGNORE_REC_SEQ    2
 
+int mempacket_swap_recent(BIO *bio);
 int mempacket_test_inject(BIO *bio, const char *in, int inl, int pktnum,
                           int type);