diff options
author | Todd Short <tshort@akamai.com> | 2021-03-22 12:56:36 -0400 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2021-04-13 12:31:02 +0200 |
commit | 86a90dc749af91f8a7b8da6628c9ffca2bae3009 (patch) | |
tree | 19dcc6dd6c5960ab0e55a920d75517cb716e0f0a | |
parent | f82f5392f39797c1cf3a5d114c0125f121b0f769 (diff) |
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.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14679)
-rw-r--r-- | ssl/ssl_lib.c | 49 | ||||
-rw-r--r-- | test/clienthellotest.c | 12 | ||||
-rw-r--r-- | test/sslapitest.c | 72 |
3 files changed, 124 insertions, 9 deletions
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 58f8f3c14c..5501ecdb58 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2829,6 +2829,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 @@ -2837,13 +2850,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; @@ -2857,13 +2882,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 8ae1e4d9c6..8106591213 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 665aa13c23..b866135065 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -6715,6 +6715,77 @@ end: return testresult; } #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; +} int setup_tests(void) { @@ -6842,6 +6913,7 @@ int setup_tests(void) #ifndef OPENSSL_NO_TLS1_3 ADD_TEST(test_sni_tls13); #endif + ADD_TEST(test_set_alpn); return 1; } |