Convert serverinfo in SSL_CTX_use_serverinfo() to v2.

Fixes openssl#18183.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18614)

2 files changed, 179 insertions, 76 deletions

diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index 4b9d005fa8..1e698ce960 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -737,16 +737,68 @@ static int serverinfo_process_buffer(unsigned int version,
     return 1;
 }
 
+static size_t extension_contextoff(unsigned int version)
+{
+    return version == SSL_SERVERINFOV1 ? 4 : 0;
+}
+
+static size_t extension_append_length(unsigned int version, size_t extension_length)
+{
+    return extension_length + extension_contextoff(version);
+}
+
+static void extension_append(unsigned int version,
+                             const unsigned char *extension,
+                             const size_t extension_length,
+                             unsigned char *serverinfo)
+{
+    const size_t contextoff = extension_contextoff(version);
+
+    if (contextoff > 0) {
+        /* We know this only uses the last 2 bytes */
+        serverinfo[0] = 0;
+        serverinfo[1] = 0;
+        serverinfo[2] = (SYNTHV1CONTEXT >> 8) & 0xff;
+        serverinfo[3] = SYNTHV1CONTEXT & 0xff;
+    }
+
+    memcpy(serverinfo + contextoff, extension, extension_length);
+}
+
 int SSL_CTX_use_serverinfo_ex(SSL_CTX *ctx, unsigned int version,
                               const unsigned char *serverinfo,
                               size_t serverinfo_length)
 {
-    unsigned char *new_serverinfo;
+    unsigned char *new_serverinfo = NULL;
 
     if (ctx == NULL || serverinfo == NULL || serverinfo_length == 0) {
         ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
+    if (version == SSL_SERVERINFOV1) {
+        /*
+         * Convert serverinfo version v1 to v2 and call yourself recursively
+         * over the converted serverinfo.
+         */
+        const size_t sinfo_length = extension_append_length(SSL_SERVERINFOV1,
+                                                            serverinfo_length);
+        unsigned char *sinfo;
+        int ret;
+
+        sinfo = OPENSSL_malloc(sinfo_length);
+        if (sinfo == NULL) {
+            ERR_raise(ERR_LIB_SSL, ERR_R_MALLOC_FAILURE);
+            return 0;
+        }
+
+        extension_append(SSL_SERVERINFOV1, serverinfo, serverinfo_length, sinfo);
+
+        ret = SSL_CTX_use_serverinfo_ex(ctx, SSL_SERVERINFOV2, sinfo,
+                                        sinfo_length);
+
+        OPENSSL_free(sinfo);
+        return ret;
+    }
     if (!serverinfo_process_buffer(version, serverinfo, serverinfo_length,
                                    NULL)) {
         ERR_raise(ERR_LIB_SSL, SSL_R_INVALID_SERVERINFO_DATA);
@@ -797,7 +849,7 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
     unsigned int name_len;
     int ret = 0;
     BIO *bin = NULL;
-    size_t num_extensions = 0, contextoff = 0;
+    size_t num_extensions = 0;
 
     if (ctx == NULL || file == NULL) {
         ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_NULL_PARAMETER);
@@ -816,6 +868,7 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
 
     for (num_extensions = 0;; num_extensions++) {
         unsigned int version;
+        size_t append_length;
 
         if (PEM_read_bio(bin, &name, &header, &extension, &extension_length)
             == 0) {
@@ -858,11 +911,6 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
                 ERR_raise(ERR_LIB_SSL, SSL_R_BAD_DATA);
                 goto end;
             }
-            /*
-             * File does not have a context value so we must take account of
-             * this later.
-             */
-            contextoff = 4;
         } else {
             /* 8 byte header: 4 bytes context, 2 bytes type, 2 bytes len */
             if (extension_length < 8
@@ -873,25 +921,16 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
             }
         }
         /* Append the decoded extension to the serverinfo buffer */
-        tmp = OPENSSL_realloc(serverinfo, serverinfo_length + extension_length
-                                          + contextoff);
+        append_length = extension_append_length(version, extension_length);
+        tmp = OPENSSL_realloc(serverinfo, serverinfo_length + append_length);
         if (tmp == NULL) {
             ERR_raise(ERR_LIB_SSL, ERR_R_MALLOC_FAILURE);
             goto end;
         }
         serverinfo = tmp;
-        if (contextoff > 0) {
-            unsigned char *sinfo = serverinfo + serverinfo_length;
-
-            /* We know this only uses the last 2 bytes */
-            sinfo[0] = 0;
-            sinfo[1] = 0;
-            sinfo[2] = (SYNTHV1CONTEXT >> 8) & 0xff;
-            sinfo[3] = SYNTHV1CONTEXT & 0xff;
-        }
-        memcpy(serverinfo + serverinfo_length + contextoff,
-               extension, extension_length);
-        serverinfo_length += extension_length + contextoff;
+        extension_append(version, extension, extension_length,
+                         serverinfo + serverinfo_length);
+        serverinfo_length += append_length;
 
         OPENSSL_free(name);
         name = NULL;
diff --git a/test/sslapitest.c b/test/sslapitest.c
index a3aecb6477..ca68c23b05 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -135,20 +135,6 @@ struct sslapitest_log_counts {
 };
 
 
-static unsigned char serverinfov1[] = {
-    0xff, 0xff, /* Dummy extension type */
-    0x00, 0x01, /* Extension length is 1 byte */
-    0xff        /* Dummy extension data */
-};
-
-static unsigned char serverinfov2[] = {
-    0x00, 0x00, 0x00,
-    (unsigned char)(SSL_EXT_CLIENT_HELLO & 0xff), /* Dummy context - 4 bytes */
-    0xff, 0xff, /* Dummy extension type */
-    0x00, 0x01, /* Extension length is 1 byte */
-    0xff        /* Dummy extension data */
-};
-
 static int hostname_cb(SSL *s, int *al, void *arg)
 {
     const char *hostname = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name);
@@ -5866,62 +5852,138 @@ end:
     return testresult;
 }
 
-/*
- * Test loading of serverinfo data in various formats. test_sslmessages actually
- * tests to make sure the extensions appear in the handshake
- */
-static int test_serverinfo(int tst)
-{
-    unsigned int version;
-    unsigned char *sibuf;
-    size_t sibuflen;
-    int ret, expected, testresult = 0;
-    SSL_CTX *ctx;
+#if !defined(OPENSSL_NO_TLS1_2) && !defined(OSSL_NO_USABLE_TLS1_3)
 
-    ctx = SSL_CTX_new_ex(libctx, NULL, TLS_method());
-    if (!TEST_ptr(ctx))
-        goto end;
+#define  SYNTHV1CONTEXT     (SSL_EXT_TLS1_2_AND_BELOW_ONLY \
+                             | SSL_EXT_CLIENT_HELLO \
+                             | SSL_EXT_TLS1_2_SERVER_HELLO \
+                             | SSL_EXT_IGNORE_ON_RESUMPTION)
 
-    if ((tst & 0x01) == 0x01)
-        version = SSL_SERVERINFOV2;
-    else
-        version = SSL_SERVERINFOV1;
+#define TLS13CONTEXT (SSL_EXT_TLS1_3_CERTIFICATE \
+                      | SSL_EXT_TLS1_2_SERVER_HELLO \
+                      | SSL_EXT_CLIENT_HELLO)
 
-    if ((tst & 0x02) == 0x02) {
-        sibuf = serverinfov2;
-        sibuflen = sizeof(serverinfov2);
-        expected = (version == SSL_SERVERINFOV2);
-    } else {
-        sibuf = serverinfov1;
-        sibuflen = sizeof(serverinfov1);
-        expected = (version == SSL_SERVERINFOV1);
+#define SERVERINFO_CUSTOM                                 \
+    0x00, (char)TLSEXT_TYPE_signed_certificate_timestamp, \
+    0x00, 0x03,                                           \
+    0x04, 0x05, 0x06                                      \
+
+static const unsigned char serverinfo_custom_tls13[] = {
+    0x00, 0x00, (TLS13CONTEXT >> 8) & 0xff, TLS13CONTEXT & 0xff,
+    SERVERINFO_CUSTOM
+};
+static const unsigned char serverinfo_custom_v2[] = {
+    0x00, 0x00, (SYNTHV1CONTEXT >> 8) & 0xff,  SYNTHV1CONTEXT & 0xff,
+    SERVERINFO_CUSTOM
+};
+static const unsigned char serverinfo_custom_v1[] = {
+    SERVERINFO_CUSTOM
+};
+static const size_t serverinfo_custom_tls13_len = sizeof(serverinfo_custom_tls13);
+static const size_t serverinfo_custom_v2_len = sizeof(serverinfo_custom_v2);
+static const size_t serverinfo_custom_v1_len = sizeof(serverinfo_custom_v1);
+
+static int serverinfo_custom_parse_cb(SSL *s, unsigned int ext_type,
+                                      unsigned int context,
+                                      const unsigned char *in,
+                                      size_t inlen, X509 *x,
+                                      size_t chainidx, int *al,
+                                      void *parse_arg)
+{
+    const size_t len = serverinfo_custom_v1_len;
+    const unsigned char *si = &serverinfo_custom_v1[len - 3];
+    int *p_cb_result = (int*)parse_arg;
+    *p_cb_result = TEST_mem_eq(in, inlen, si, 3);
+    return 1;
+}
+
+static int test_serverinfo_custom(const int idx)
+{
+    SSL_CTX *sctx = NULL, *cctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    int testresult = 0;
+    int cb_result = 0;
+
+    /*
+     * Following variables are set in the switch statement
+     *  according to the test iteration.
+     * Default values do not make much sense: test would fail with them.
+     */
+    int serverinfo_version = 0;
+    int protocol_version = 0;
+    unsigned int extension_context = 0;
+    const unsigned char *si = NULL;
+    size_t si_len = 0;
+
+    const int call_use_serverinfo_ex = idx > 0;
+    switch (idx) {
+    case 0: /* FALLTHROUGH */
+    case 1:
+        serverinfo_version = SSL_SERVERINFOV1;
+        protocol_version = TLS1_2_VERSION;
+        extension_context = SYNTHV1CONTEXT;
+        si = serverinfo_custom_v1;
+        si_len = serverinfo_custom_v1_len;
+        break;
+    case 2:
+        serverinfo_version = SSL_SERVERINFOV2;
+        protocol_version = TLS1_2_VERSION;
+        extension_context = SYNTHV1CONTEXT;
+        si = serverinfo_custom_v2;
+        si_len = serverinfo_custom_v2_len;
+        break;
+    case 3:
+        serverinfo_version = SSL_SERVERINFOV2;
+        protocol_version = TLS1_3_VERSION;
+        extension_context = TLS13CONTEXT;
+        si = serverinfo_custom_tls13;
+        si_len = serverinfo_custom_tls13_len;
+        break;
     }
 
-    if ((tst & 0x04) == 0x04) {
-        ret = SSL_CTX_use_serverinfo_ex(ctx, version, sibuf, sibuflen);
-    } else {
-        ret = SSL_CTX_use_serverinfo(ctx, sibuf, sibuflen);
+    if (!TEST_true(create_ssl_ctx_pair(libctx,
+                                       TLS_method(),
+                                       TLS_method(),
+                                       protocol_version,
+                                       protocol_version,
+                                       &sctx, &cctx, cert, privkey)))
+        goto end;
 
-        /*
-         * The version variable is irrelevant in this case - it's what is in the
-         * buffer that matters
-         */
-        if ((tst & 0x02) == 0x02)
-            expected = 0;
-        else
-            expected = 1;
+    if (call_use_serverinfo_ex) {
+        if (!TEST_true(SSL_CTX_use_serverinfo_ex(sctx, serverinfo_version,
+                                                 si, si_len)))
+            goto end;
+    } else {
+        if (!TEST_true(SSL_CTX_use_serverinfo(sctx, si, si_len)))
+            goto end;
     }
 
-    if (!TEST_true(ret == expected))
+    if (!TEST_true(SSL_CTX_add_custom_ext(cctx, TLSEXT_TYPE_signed_certificate_timestamp,
+                                          extension_context,
+                                          NULL, NULL, NULL,
+                                          serverinfo_custom_parse_cb,
+                                          &cb_result))
+        || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+                                         NULL, NULL))
+        || !TEST_true(create_ssl_connection(serverssl, clientssl,
+                                            SSL_ERROR_NONE))
+        || !TEST_int_eq(SSL_do_handshake(clientssl), 1))
+        goto end;
+
+    if (!TEST_true(cb_result))
         goto end;
 
     testresult = 1;
 
  end:
-    SSL_CTX_free(ctx);
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
 
     return testresult;
 }
+#endif
 
 /*
  * Test that SSL_export_keying_material() produces expected results. There are
@@ -10112,7 +10174,6 @@ int setup_tests(void)
 #else
     ADD_ALL_TESTS(test_custom_exts, 3);
 #endif
-    ADD_ALL_TESTS(test_serverinfo, 8);
     ADD_ALL_TESTS(test_export_key_mat, 6);
 #ifndef OSSL_NO_USABLE_TLS1_3
     ADD_ALL_TESTS(test_export_key_mat_early, 3);
@@ -10167,6 +10228,9 @@ int setup_tests(void)
 #ifndef OSSL_NO_USABLE_TLS1_3
     ADD_TEST(test_read_ahead_key_change);
 #endif
+#if !defined(OPENSSL_NO_TLS1_2) && !defined(OSSL_NO_USABLE_TLS1_3)
+    ADD_ALL_TESTS(test_serverinfo_custom, 4);
+#endif
     return 1;
 
  err: