Fix SSL_pending() and SSL_has_pending() with DTLS (1.1.1)

ssl/record/rec_layer_s3.c

@@ -115,10 +115,22 @@ size_t ssl3_pending(const SSL *s)
     if (s->rlayer.rstate == SSL_ST_READ_BODY)
         return 0;
 
+    /* Take into account DTLS buffered app data */
+    if (SSL_IS_DTLS(s)) {
+        DTLS1_RECORD_DATA *rdata;
+        pitem *item, *iter;
+
+        iter = pqueue_iterator(s->rlayer.d->buffered_app_data.q);
+        while ((item = pqueue_next(&iter)) != NULL) {
+            rdata = item->data;
+            num += rdata->rrec.length;
+        }
+    }
+
     for (i = 0; i < RECORD_LAYER_get_numrpipes(&s->rlayer); i++) {
         if (SSL3_RECORD_get_type(&s->rlayer.rrec[i])
             != SSL3_RT_APPLICATION_DATA)
-            return 0;
+            return num;
         num += SSL3_RECORD_get_length(&s->rlayer.rrec[i]);
     }
 

ssl/ssl_lib.c

@@ -1510,12 +1510,26 @@ int SSL_has_pending(const SSL *s)
 {
     /*
      * Similar to SSL_pending() but returns a 1 to indicate that we have
-     * unprocessed data available or 0 otherwise (as opposed to the number of
-     * bytes available). Unlike SSL_pending() this will take into account
-     * read_ahead data. A 1 return simply indicates that we have unprocessed
-     * data. That data may not result in any application data, or we may fail
-     * to parse the records for some reason.
+     * processed or unprocessed data available or 0 otherwise (as opposed to the
+     * number of bytes available). Unlike SSL_pending() this will take into
+     * account read_ahead data. A 1 return simply indicates that we have data.
+     * That data may not result in any application data, or we may fail to parse
+     * the records for some reason.
      */
+
+    /* Check buffered app data if any first */
+    if (SSL_IS_DTLS(s)) {
+        DTLS1_RECORD_DATA *rdata;
+        pitem *item, *iter;
+
+        iter = pqueue_iterator(s->rlayer.d->buffered_app_data.q);
+        while ((item = pqueue_next(&iter)) != NULL) {
+            rdata = item->data;
+            if (rdata->rrec.length > 0)
+                return 1;
+        }
+    }
+
     if (RECORD_LAYER_processed_read_pending(&s->rlayer))
         return 1;
 

test/dtlstest.c

@@ -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;
 }

test/ssltestlib.c

@@ -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)
 {

test/ssltestlib.h

@@ -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);