From 555dd9390ba56f1c400d3f067a2dfe7b00fbf7d3 Mon Sep 17 00:00:00 2001 From: Daniel Fiala Date: Tue, 24 May 2022 15:11:58 +0200 Subject: Convert serverinfo in SSL_CTX_use_serverinfo() to v2. Fixes openssl#18183. Reviewed-by: Matt Caswell Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/18614) --- ssl/ssl_rsa.c | 81 ++++++++++++++++++------- test/sslapitest.c | 174 +++++++++++++++++++++++++++++++++++++----------------- 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); @@ -10166,6 +10227,9 @@ int setup_tests(void) ADD_TEST(test_load_dhfile); #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; -- cgit v1.2.3