From f4e670f6836a1ebb652822d0bf6c5ef0fb52b5b7 Mon Sep 17 00:00:00 2001 From: K1 Date: Tue, 16 Jul 2024 17:23:13 +0800 Subject: [PATCH] Handle set_alpn_protos inputs better. It's possible to set an invalid protocol list that will be sent in a ClientHello. This validates the inputs to make sure this does not happen. (cherry picked from commit 86a90dc749af91f8a7b8da6628c9ffca2bae3009) --- ssl/ssl_lib.c | 49 ++++++++++++++++++++++++---- test/clienthellotest.c | 12 +++++-- test/sslapitest.c | 73 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 9 deletions(-) diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 394c3ab6a..1d961b05b 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2934,6 +2934,19 @@ void SSL_CTX_set_npn_select_cb(SSL_CTX *ctx, } #endif +static int alpn_value_ok(const unsigned char *protos, unsigned int protos_len) +{ + unsigned int idx; + + if (protos_len < 2 || protos == NULL) + return 0; + + for (idx = 0; idx < protos_len; idx += protos[idx] + 1) { + if (protos[idx] == 0) + return 0; + } + return idx == protos_len; +} /* * SSL_CTX_set_alpn_protos sets the ALPN protocol list on |ctx| to |protos|. * |protos| must be in wire-format (i.e. a series of non-empty, 8-bit @@ -2942,13 +2955,25 @@ void SSL_CTX_set_npn_select_cb(SSL_CTX *ctx, int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos, unsigned int protos_len) { - OPENSSL_free(ctx->ext.alpn); - ctx->ext.alpn = OPENSSL_memdup(protos, protos_len); - if (ctx->ext.alpn == NULL) { + unsigned char *alpn; + + if (protos_len == 0 || protos == NULL) { + OPENSSL_free(ctx->ext.alpn); + ctx->ext.alpn = NULL; ctx->ext.alpn_len = 0; + return 0; + } + /* Not valid per RFC */ + if (!alpn_value_ok(protos, protos_len)) + return 1; + + alpn = OPENSSL_memdup(protos, protos_len); + if (alpn == NULL) { SSLerr(SSL_F_SSL_CTX_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE); return 1; } + OPENSSL_free(ctx->ext.alpn); + ctx->ext.alpn = alpn; ctx->ext.alpn_len = protos_len; return 0; @@ -2962,13 +2987,25 @@ int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos, int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos, unsigned int protos_len) { - OPENSSL_free(ssl->ext.alpn); - ssl->ext.alpn = OPENSSL_memdup(protos, protos_len); - if (ssl->ext.alpn == NULL) { + unsigned char *alpn; + + if (protos_len == 0 || protos == NULL) { + OPENSSL_free(ssl->ext.alpn); + ssl->ext.alpn = NULL; ssl->ext.alpn_len = 0; + return 0; + } + /* Not valid per RFC */ + if (!alpn_value_ok(protos, protos_len)) + return 1; + + alpn = OPENSSL_memdup(protos, protos_len); + if (alpn == NULL) { SSLerr(SSL_F_SSL_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE); return 1; } + OPENSSL_free(ssl->ext.alpn); + ssl->ext.alpn = alpn; ssl->ext.alpn_len = protos_len; return 0; diff --git a/test/clienthellotest.c b/test/clienthellotest.c index 8ae1e4d9c..810659121 100644 --- a/test/clienthellotest.c +++ b/test/clienthellotest.c @@ -45,10 +45,16 @@ static const char *sessionfile = NULL; /* Dummy ALPN protocols used to pad out the size of the ClientHello */ +/* ASCII 'O' = 79 = 0x4F = EBCDIC '|'*/ +#ifdef CHARSET_EBCDIC static const char alpn_prots[] = - "0123456789012345678901234567890123456789012345678901234567890123456789" - "0123456789012345678901234567890123456789012345678901234567890123456789" - "01234567890123456789"; + "|1234567890123456789012345678901234567890123456789012345678901234567890123456789" + "|1234567890123456789012345678901234567890123456789012345678901234567890123456789"; +#else +static const char alpn_prots[] = + "O1234567890123456789012345678901234567890123456789012345678901234567890123456789" + "O1234567890123456789012345678901234567890123456789012345678901234567890123456789"; +#endif static int test_client_hello(int currtest) { diff --git a/test/sslapitest.c b/test/sslapitest.c index 07efafb62..56ed3aceb 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -6917,6 +6917,78 @@ static int test_quic_early_data(int tst) } #endif +/* + * Test that setting an ALPN does not violate RFC + */ +static int test_set_alpn(void) +{ + SSL_CTX *ctx = NULL; + SSL *ssl = NULL; + int testresult = 0; + + unsigned char bad0[] = { 0x00, 'b', 'a', 'd' }; + unsigned char good[] = { 0x04, 'g', 'o', 'o', 'd' }; + unsigned char bad1[] = { 0x01, 'b', 'a', 'd' }; + unsigned char bad2[] = { 0x03, 'b', 'a', 'd', 0x00}; + unsigned char bad3[] = { 0x03, 'b', 'a', 'd', 0x01, 'b', 'a', 'd'}; + unsigned char bad4[] = { 0x03, 'b', 'a', 'd', 0x06, 'b', 'a', 'd'}; + + /* Create an initial SSL_CTX with no certificate configured */ + ctx = SSL_CTX_new(TLS_server_method()); + if (!TEST_ptr(ctx)) + goto end; + + /* the set_alpn functions return 0 (false) on success, non-zero (true) on failure */ + if (!TEST_false(SSL_CTX_set_alpn_protos(ctx, NULL, 2))) + goto end; + if (!TEST_false(SSL_CTX_set_alpn_protos(ctx, good, 0))) + goto end; + if (!TEST_false(SSL_CTX_set_alpn_protos(ctx, good, sizeof(good)))) + goto end; + if (!TEST_true(SSL_CTX_set_alpn_protos(ctx, good, 1))) + goto end; + if (!TEST_true(SSL_CTX_set_alpn_protos(ctx, bad0, sizeof(bad0)))) + goto end; + if (!TEST_true(SSL_CTX_set_alpn_protos(ctx, bad1, sizeof(bad1)))) + goto end; + if (!TEST_true(SSL_CTX_set_alpn_protos(ctx, bad2, sizeof(bad2)))) + goto end; + if (!TEST_true(SSL_CTX_set_alpn_protos(ctx, bad3, sizeof(bad3)))) + goto end; + if (!TEST_true(SSL_CTX_set_alpn_protos(ctx, bad4, sizeof(bad4)))) + goto end; + + ssl = SSL_new(ctx); + if (!TEST_ptr(ssl)) + goto end; + + if (!TEST_false(SSL_set_alpn_protos(ssl, NULL, 2))) + goto end; + if (!TEST_false(SSL_set_alpn_protos(ssl, good, 0))) + goto end; + if (!TEST_false(SSL_set_alpn_protos(ssl, good, sizeof(good)))) + goto end; + if (!TEST_true(SSL_set_alpn_protos(ssl, good, 1))) + goto end; + if (!TEST_true(SSL_set_alpn_protos(ssl, bad0, sizeof(bad0)))) + goto end; + if (!TEST_true(SSL_set_alpn_protos(ssl, bad1, sizeof(bad1)))) + goto end; + if (!TEST_true(SSL_set_alpn_protos(ssl, bad2, sizeof(bad2)))) + goto end; + if (!TEST_true(SSL_set_alpn_protos(ssl, bad3, sizeof(bad3)))) + goto end; + if (!TEST_true(SSL_set_alpn_protos(ssl, bad4, sizeof(bad4)))) + goto end; + + testresult = 1; + +end: + SSL_free(ssl); + SSL_CTX_free(ctx); + return testresult; +} + #ifndef OPENSSL_NO_CERT_COMPRESSION #define CERT_COMPRESSION_XOR 16384 #define CERT_COMPRESSION_ROL 65535 @@ -8135,6 +8207,7 @@ int setup_tests(void) ADD_ALL_TESTS(test_client_cert_cb, 2); ADD_ALL_TESTS(test_ca_names, 3); ADD_ALL_TESTS(test_servername, 10); + ADD_TEST(test_set_alpn); #ifndef OPENSSL_NO_QUIC ADD_ALL_TESTS(test_quic_api, 9); ADD_ALL_TESTS(test_quic_early_data, 4);