Add tests for the padding extension

Check that the padding extension pads correctly for various scenarios.
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2968)

test/build.info

@@ -182,7 +182,7 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[verify_extra_test]=../include
   DEPEND[verify_extra_test]=../libcrypto
 
-  SOURCE[clienthellotest]=clienthellotest.c
+  SOURCE[clienthellotest]=clienthellotest.c testutil.c test_main_custom.c
   INCLUDE[clienthellotest]=../include
   DEPEND[clienthellotest]=../libcrypto ../libssl
 

test/clienthellotest.c

@@ -15,144 +15,220 @@
 #include <openssl/evp.h>
 #include <openssl/ssl.h>
 #include <openssl/err.h>
+#include <time.h>
 
 #include "../ssl/packet_locl.h"
 
-#define CLIENT_VERSION_LEN      2
+#include "testutil.h"
+#include "test_main_custom.h"
 
+#define CLIENT_VERSION_LEN      2
 
-#define TOTAL_NUM_TESTS                         1
+#define TOTAL_NUM_TESTS                         4
 
 /*
  * Test that explicitly setting ticket data results in it appearing in the
  * ClientHello for a negotiated SSL/TLS version
  */
 #define TEST_SET_SESSION_TICK_DATA_VER_NEG      0
+/* Enable padding and make sure ClientHello is long enough to require it */
+#define TEST_ADD_PADDING                        1
+/* Enable padding and make sure ClientHello is short enough to not need it */
+#define TEST_PADDING_NOT_NEEDED                 2
+/*
+ * Enable padding and add a PSK to the ClientHello (this will also ensure the
+ * ClientHello is long enough to need padding)
+ */
+#define TEST_ADD_PADDING_AND_PSK                3
+
+#define F5_WORKAROUND_MIN_MSG_LEN   0xff
+#define F5_WORKAROUND_MAX_MSG_LEN   0x200
+
+const char *sessionfile = NULL;
 
-int main(int argc, char *argv[])
+static int test_client_hello(int currtest)
 {
     SSL_CTX *ctx;
     SSL *con = NULL;
     BIO *rbio;
     BIO *wbio;
-    BIO *err;
     long len;
     unsigned char *data;
     PACKET pkt, pkt2, pkt3;
     char *dummytick = "Hello World!";
     unsigned int type;
     int testresult = 0;
-    int currtest = 0;
-
-    err = BIO_new_fp(stderr, BIO_NOCLOSE | BIO_FP_TEXT);
-
-    CRYPTO_set_mem_debug(1);
-    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
+    size_t msglen;
+    BIO *sessbio = NULL;
+    SSL_SESSION *sess = NULL;
 
     /*
      * For each test set up an SSL_CTX and SSL and see what ClientHello gets
      * produced when we try to connect
      */
-    for (; currtest < TOTAL_NUM_TESTS; currtest++) {
-        testresult = 0;
-        ctx = SSL_CTX_new(TLS_method());
+    ctx = SSL_CTX_new(TLS_method());
+    if (ctx == NULL)
+        goto end;
 
+    switch(currtest) {
+    case TEST_SET_SESSION_TICK_DATA_VER_NEG:
         /* Testing for session tickets <= TLS1.2; not relevant for 1.3 */
-        if (ctx == NULL || !SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION))
+        if (!SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION))
             goto end;
-
-        con = SSL_new(ctx);
-        if (con == NULL)
+        break;
+
+    case TEST_ADD_PADDING_AND_PSK:
+    case TEST_ADD_PADDING:
+    case TEST_PADDING_NOT_NEEDED:
+        SSL_CTX_set_options(ctx, SSL_OP_TLSEXT_PADDING);
+        /*
+         * Add lots of ciphersuites so that the ClientHello is at least
+         * F5_WORKAROUND_MIN_MSG_LEN bytes long - meaning padding will be
+         * needed.
+         * In the padding not needed case we assume the test will pass, but then
+         * set testresult to 0 if we see the padding extension.
+         */
+        if (currtest == TEST_ADD_PADDING
+                && !SSL_CTX_set_cipher_list(ctx, "ALL"))
             goto end;
+        else if (currtest == TEST_PADDING_NOT_NEEDED)
+            testresult = 1;
+        break;
 
-        rbio = BIO_new(BIO_s_mem());
-        wbio = BIO_new(BIO_s_mem());
-        if (rbio == NULL || wbio == NULL) {
-            BIO_free(rbio);
-            BIO_free(wbio);
-            goto end;
-        }
+    default:
+        goto end;
+    }
 
-        SSL_set_bio(con, rbio, wbio);
-        SSL_set_connect_state(con);
+    con = SSL_new(ctx);
+    if (con == NULL)
+        goto end;
 
-        if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
-            if (!SSL_set_session_ticket_ext(con, dummytick, strlen(dummytick)))
-                goto end;
+    if (currtest == TEST_ADD_PADDING_AND_PSK) {
+        sessbio = BIO_new_file(sessionfile, "r");
+        if (sessbio == NULL) {
+            printf("Unable to open session.pem\n");
+            goto end;
         }
-
-        if (SSL_connect(con) > 0) {
-            /* This shouldn't succeed because we don't have a server! */
+        sess = PEM_read_bio_SSL_SESSION(sessbio, NULL, NULL, NULL);
+        if (sess == NULL) {
+            printf("Unable to load SSL_SESSION\n");
             goto end;
         }
-
-        len = BIO_get_mem_data(wbio, (char **)&data);
-        if (!PACKET_buf_init(&pkt, data, len))
+        /*
+         * We reset the creation time so that we don't discard the session as
+         * too old.
+         */
+        if (!SSL_SESSION_set_time(sess, time(NULL))) {
+            printf("Unable to set creation time on SSL_SESSION\n");
             goto end;
-
-        /* Skip the record header */
-        if (!PACKET_forward(&pkt, SSL3_RT_HEADER_LENGTH))
+        }
+        if (!SSL_set_session(con, sess)) {
+            printf("Unable to set the session on the connection\n");
             goto end;
+        }
+    }
 
-        /* Skip the handshake message header */
-        if (!PACKET_forward(&pkt, SSL3_HM_HEADER_LENGTH))
-            goto end;
+    rbio = BIO_new(BIO_s_mem());
+    wbio = BIO_new(BIO_s_mem());
+    if (rbio == NULL || wbio == NULL) {
+        BIO_free(rbio);
+        BIO_free(wbio);
+        goto end;
+    }
 
-        /* Skip client version and random */
-        if (!PACKET_forward(&pkt, CLIENT_VERSION_LEN + SSL3_RANDOM_SIZE))
-            goto end;
+    SSL_set_bio(con, rbio, wbio);
+    SSL_set_connect_state(con);
 
-        /* Skip session id */
-        if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
+    if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
+        if (!SSL_set_session_ticket_ext(con, dummytick, strlen(dummytick)))
             goto end;
+    }
 
-        /* Skip ciphers */
-        if (!PACKET_get_length_prefixed_2(&pkt, &pkt2))
-            goto end;
+    if (SSL_connect(con) > 0) {
+        /* This shouldn't succeed because we don't have a server! */
+        goto end;
+    }
 
-        /* Skip compression */
-        if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
-            goto end;
+    len = BIO_get_mem_data(wbio, (char **)&data);
+    if (!PACKET_buf_init(&pkt, data, len))
+        goto end;
+
+    /* Skip the record header */
+    if (!PACKET_forward(&pkt, SSL3_RT_HEADER_LENGTH))
+        goto end;
+
+    msglen = PACKET_remaining(&pkt);
+
+    /* Skip the handshake message header */
+    if (!PACKET_forward(&pkt, SSL3_HM_HEADER_LENGTH))
+        goto end;
+
+    /* Skip client version and random */
+    if (!PACKET_forward(&pkt, CLIENT_VERSION_LEN + SSL3_RANDOM_SIZE))
+        goto end;
+
+    /* Skip session id */
+    if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
+        goto end;
+
+    /* Skip ciphers */
+    if (!PACKET_get_length_prefixed_2(&pkt, &pkt2))
+        goto end;
+
+    /* Skip compression */
+    if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
+        goto end;
 
-        /* Extensions len */
-        if (!PACKET_as_length_prefixed_2(&pkt, &pkt2))
+    /* Extensions len */
+    if (!PACKET_as_length_prefixed_2(&pkt, &pkt2))
+        goto end;
+
+    /* Loop through all extensions */
+    while (PACKET_remaining(&pkt2)) {
+
+        if (!PACKET_get_net_2(&pkt2, &type) ||
+            !PACKET_get_length_prefixed_2(&pkt2, &pkt3))
             goto end;
 
-        /* Loop through all extensions */
-        while (PACKET_remaining(&pkt2)) {
-
-            if (!PACKET_get_net_2(&pkt2, &type) ||
-                !PACKET_get_length_prefixed_2(&pkt2, &pkt3))
-                goto end;
-
-            if (type == TLSEXT_TYPE_session_ticket) {
-                if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
-                    if (PACKET_equal(&pkt3, dummytick, strlen(dummytick))) {
-                        /* Ticket data is as we expected */
-                        testresult = 1;
-                    } else {
-                        printf("Received session ticket is not as expected\n");
-                    }
-                    break;
+        if (type == TLSEXT_TYPE_session_ticket) {
+            if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
+                if (PACKET_equal(&pkt3, dummytick, strlen(dummytick))) {
+                    /* Ticket data is as we expected */
+                    testresult = 1;
+                } else {
+                    printf("Received session ticket is not as expected\n");
                 }
+                break;
             }
-
         }
-
- end:
-        SSL_free(con);
-        SSL_CTX_free(ctx);
-        if (!testresult) {
-            printf("ClientHello test: FAILED (Test %d)\n", currtest);
-            break;
+        if (type == TLSEXT_TYPE_padding) {
+            if (currtest == TEST_ADD_PADDING
+                    || currtest == TEST_ADD_PADDING_AND_PSK)
+                testresult = (msglen == F5_WORKAROUND_MAX_MSG_LEN);
+            else
+                testresult = 0;
         }
     }
 
-#ifndef OPENSSL_NO_CRYPTO_MDEBUG
-    if (CRYPTO_mem_leaks(err) <= 0)
-        testresult = 0;
-#endif
-    BIO_free(err);
+end:
+    SSL_free(con);
+    SSL_CTX_free(ctx);
+    SSL_SESSION_free(sess);
+    BIO_free(sessbio);
+    if (!testresult)
+        printf("ClientHello test: FAILED (Test %d)\n", currtest);
+
+    return testresult;
+}
+
+int test_main(int argc, char *argv[])
+{
+    if (argc != 2)
+        return 0;
+
+    sessionfile = argv[1];
+
+    ADD_ALL_TESTS(test_client_hello, TOTAL_NUM_TESTS);
 
-    return testresult?0:1;
+    return run_tests(argv[0]);
 }

test/recipes/70-test_clienthello.t

@@ -7,7 +7,7 @@
 # https://www.openssl.org/source/license.html
 
 
-use OpenSSL::Test;
+use OpenSSL::Test qw/:DEFAULT srctop_file/;
 use OpenSSL::Test::Utils;
 
 setup("test_clienthello");
@@ -17,4 +17,5 @@ plan skip_all => "No TLS/SSL protocols are supported by this OpenSSL build"
 
 plan tests => 1;
 
-ok(run(test(["clienthellotest"])), "running clienthellotest");
+ok(run(test(["clienthellotest", srctop_file("test", "session.pem")])),
+   "running clienthellotest");

test/session.pem

+-----BEGIN SSL SESSION PARAMETERS-----
+MIIFMAIBAQICAwQEAhMCBCAuhyL8Neo+jOicuNiWOzIDX/HXQRGGkgru3aX+p7+6
+CgQwXZWvZnbuON/qITvDWC7KoECPjyThlAd3fRe7ZxD/6C+vqf+SpSUMcxS7P24t
+RyXYoQYCBFjKfImiBAICHCCjggPrMIID5zCCAs+gAwIBAgIJALnu1NlVpZ6zMA0G
+CSqGSIb3DQEBBQUAMHAxCzAJBgNVBAYTAlVLMRYwFAYDVQQKDA1PcGVuU1NMIEdy
+b3VwMSIwIAYDVQQLDBlGT1IgVEVTVElORyBQVVJQT1NFUyBPTkxZMSUwIwYDVQQD
+DBxPcGVuU1NMIFRlc3QgSW50ZXJtZWRpYXRlIENBMB4XDTExMTIwODE0MDE0OFoX
+DTIxMTAxNjE0MDE0OFowZDELMAkGA1UEBhMCVUsxFjAUBgNVBAoMDU9wZW5TU0wg
+R3JvdXAxIjAgBgNVBAsMGUZPUiBURVNUSU5HIFBVUlBPU0VTIE9OTFkxGTAXBgNV
+BAMMEFRlc3QgU2VydmVyIENlcnQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK
+AoIBAQDzhPOSNtyyRspmeuUpxfNJKCLTuf7g3uQ4zu4iHOmRO5TQci+HhVlLZrHF
+9XqFXcIP0y4pWDbMSGuiorUmzmfiR7bfSdI/+qIQt8KXRH6HNG1t8ou0VSvWId5T
+S5Dq/er5ODUr9OaaDva7EquHIcMvvPQGuI+OEAcnleVCy9HVEIySrO4P3CNIicnG
+kwwiAud05yUAq/gPXBC1hTtmlPD7TVcGVSEiJdvzqqlgv02qedGrkki6GY4S7GjZ
+xrrf7Foc2EP+51LJzwLQx3/JfrCU41NEWAsu/Sl0tQabXESN+zJ1pDqoZ3uHMgpQ
+jeGiE0olr+YcsSW/tJmiU9OiAr8RAgMBAAGjgY8wgYwwDAYDVR0TAQH/BAIwADAO
+BgNVHQ8BAf8EBAMCBeAwLAYJYIZIAYb4QgENBB8WHU9wZW5TU0wgR2VuZXJhdGVk
+IENlcnRpZmljYXRlMB0GA1UdDgQWBBSCvM8AABPR9zklmifnr9LvIBturDAfBgNV
+HSMEGDAWgBQ2w2yI55X+sL3szj49hqshgYfa2jANBgkqhkiG9w0BAQUFAAOCAQEA
+qb1NV0B0/pbpK9Z4/bNjzPQLTRLKWnSNm/Jh5v0GEUOE/Beg7GNjNrmeNmqxAlpq
+Wz9qoeoFZax+QBpIZYjROU3TS3fpyLsrnlr0CDQ5R7kCCDGa8dkXxemmpZZLbUCp
+W2Uoy8sAA4JjN9OtsZY7dvUXFgJ7vVNTRnI01ghknbtD+2SxSQd3CWF6QhcRMAzZ
+J1z1cbbwGDDzfvGFPzJ+Sq+zEPdsxoVLLSetCiBc+40ZcDS5dV98h9XD7JMTQfxz
+A7mNGv73JoZJA6nFgj+ADSlJsY/tJBv+z1iQRueoh9Qeee+ZbRifPouCB8FDx+Al
+tvHTANdAq0t/K3o+pplMVKQCBAClAwIBFakEAgIcIKqBswSBsKXqWrhXS9CdUYkn
+yj8+BRslsixGMMFyWSHsivOMmAf3dX5z/iDaY8cqytsRkNRKzlSPjblplzcGo9pz
+sUazmp39cWRsWrKJs2izBxqVRcp4rpzzDCSTZK3UiY2uhKgGmC2WPwIMyxuEya00
+rmMgKGee7AQPG8qQGQgDEd/6Vh1ZPbpsh+XQW42ZgMhc4iDsRETH/DTlRkm527lH
+IA1ez17Zk5vMIa65o82opA4KCVRqrgcCBQDXFjTErwQCAkAA
+-----END SSL SESSION PARAMETERS-----