From: Emilia Kasper Date: Thu, 9 Jun 2016 22:39:22 +0000 (+0200) Subject: Clean up following new SNI tests X-Git-Tag: OpenSSL_1_1_0-pre6~478 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=81fc33c951b645f31727dc811e02200bde4a0982;p=oweals%2Fopenssl.git Clean up following new SNI tests - Only send SNI in SNI tests. This allows us to test handshakes without the SNI extension as well. - Move all handshake-specific machinery to handshake_helper.c - Use enum types to represent the enum everywhere (Resorting to plain ints can end in sign mismatch when the enum is represented by an unsigned type.) Reviewed-by: Rich Salz --- diff --git a/test/README.ssltest.md b/test/README.ssltest.md index 9cbfbc4f3b..03b9f93979 100644 --- a/test/README.ssltest.md +++ b/test/README.ssltest.md @@ -64,8 +64,9 @@ The test section supports the following options: - AcceptAll - accepts all certificates. - RejectAll - rejects all certificates. -* ServerName - the server the client is expected to successfully connect to - - server1 - the initial context (default) +* ServerName - the server the client should attempt to connect to. One of + - None - do not use SNI (default) + - server1 - the initial context - server2 - the secondary context * SessionTicketExpected - whether or not a session ticket is expected diff --git a/test/handshake_helper.c b/test/handshake_helper.c index f7ab841f57..22ba1ce0f9 100644 --- a/test/handshake_helper.c +++ b/test/handshake_helper.c @@ -41,6 +41,23 @@ static void info_callback(const SSL *s, int where, int ret) } } +static int servername_callback(SSL *s, int *ad, void *arg) +{ + const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name); + if (servername != NULL && !strcmp(servername, "server2")) { + SSL_CTX *new_ctx = (SSL_CTX*)arg; + SSL_set_SSL_CTX(s, new_ctx); + /* + * Copy over all the SSL_CTX options - reasonable behavior + * allows testing of cases where the options between two + * contexts differ/conflict + */ + SSL_clear_options(s, 0xFFFFFFFFL); + SSL_set_options(s, SSL_CTX_get_options(new_ctx)); + } + return SSL_TLSEXT_ERR_OK; +} + static int verify_reject_callback(X509_STORE_CTX *ctx, void *arg) { X509_STORE_CTX_set_error(ctx, X509_V_ERR_APPLICATION_VERIFICATION); return 0; @@ -56,8 +73,10 @@ static int broken_session_ticket_callback(SSL* s, unsigned char* key_name, unsig return 0; } -int do_not_call_session_ticket_callback(SSL* s, unsigned char* key_name, unsigned char *iv, - EVP_CIPHER_CTX *ctx, HMAC_CTX *hctx, int enc) +static int do_not_call_session_ticket_callback(SSL* s, unsigned char* key_name, + unsigned char *iv, + EVP_CIPHER_CTX *ctx, + HMAC_CTX *hctx, int enc) { HANDSHAKE_EX_DATA *ex_data = (HANDSHAKE_EX_DATA*)(SSL_get_ex_data(s, ex_data_idx)); @@ -69,7 +88,8 @@ int do_not_call_session_ticket_callback(SSL* s, unsigned char* key_name, unsigne * Configure callbacks and other properties that can't be set directly * in the server/client CONF. */ -static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *client_ctx, +static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *server2_ctx, + SSL_CTX *client_ctx, const SSL_TEST_CTX *test_ctx) { switch (test_ctx->client_verify_callback) { @@ -84,6 +104,17 @@ static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *client_ctx, default: break; } + + /* link the two contexts for SNI purposes */ + SSL_CTX_set_tlsext_servername_callback(server_ctx, servername_callback); + SSL_CTX_set_tlsext_servername_arg(server_ctx, server2_ctx); + /* + * The initial_ctx/session_ctx always handles the encrypt/decrypt of the + * session ticket. This ticket_key callback is assigned to the second + * session (assigned via SNI), and should never be invoked + */ + SSL_CTX_set_tlsext_ticket_key_cb(server2_ctx, do_not_call_session_ticket_callback); + if (test_ctx->session_ticket_expected == SSL_TEST_SESSION_TICKET_BROKEN) { SSL_CTX_set_tlsext_ticket_key_cb(server_ctx, broken_session_ticket_callback); } @@ -96,7 +127,9 @@ static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *client_ctx, static void configure_handshake_ssl(SSL *server, SSL *client, const SSL_TEST_CTX *test_ctx) { - SSL_set_tlsext_host_name(client, ssl_servername_name(test_ctx->servername)); + if (test_ctx->servername != SSL_TEST_SERVERNAME_NONE) + SSL_set_tlsext_host_name(client, + ssl_servername_name(test_ctx->servername)); } @@ -199,8 +232,8 @@ static handshake_status_t handshake_status(peer_status_t last_status, return INTERNAL_ERROR; } -HANDSHAKE_RESULT do_handshake(SSL_CTX *server_ctx, SSL_CTX *client_ctx, - const SSL_TEST_CTX *test_ctx) +HANDSHAKE_RESULT do_handshake(SSL_CTX *server_ctx, SSL_CTX *server2_ctx, + SSL_CTX *client_ctx, const SSL_TEST_CTX *test_ctx) { SSL *server, *client; BIO *client_to_server, *server_to_client; @@ -213,7 +246,7 @@ HANDSHAKE_RESULT do_handshake(SSL_CTX *server_ctx, SSL_CTX *client_ctx, size_t len = 0; SSL_SESSION* sess = NULL; - configure_handshake_ctx(server_ctx, client_ctx, test_ctx); + configure_handshake_ctx(server_ctx, server2_ctx, client_ctx, test_ctx); server = SSL_new(server_ctx); client = SSL_new(client_ctx); diff --git a/test/handshake_helper.h b/test/handshake_helper.h index d04655a9ed..4a51ad4a6e 100644 --- a/test/handshake_helper.h +++ b/test/handshake_helper.h @@ -27,18 +27,15 @@ typedef struct handshake_result { int server_protocol; int client_protocol; /* Server connection */ - int servername; + ssl_servername_t servername; /* Session ticket status */ - int session_ticket; + ssl_session_ticket_t session_ticket; /* Was this called on the second context? */ int session_ticket_do_not_call; } HANDSHAKE_RESULT; /* Do a handshake and report some information about the result. */ -HANDSHAKE_RESULT do_handshake(SSL_CTX *server_ctx, SSL_CTX *client_ctx, - const SSL_TEST_CTX *test_ctx); - -int do_not_call_session_ticket_callback(SSL* s, unsigned char* key_name, unsigned char *iv, - EVP_CIPHER_CTX *ctx, HMAC_CTX *hctx, int enc); +HANDSHAKE_RESULT do_handshake(SSL_CTX *server_ctx, SSL_CTX *server2_ctx, + SSL_CTX *client_ctx, const SSL_TEST_CTX *test_ctx); #endif /* HEADER_HANDSHAKE_HELPER_H */ diff --git a/test/ssl_test.c b/test/ssl_test.c index 56dcef5510..0062c51a4d 100644 --- a/test/ssl_test.c +++ b/test/ssl_test.c @@ -125,7 +125,8 @@ static int check_protocol(HANDSHAKE_RESULT result, SSL_TEST_CTX *test_ctx) static int check_servername(HANDSHAKE_RESULT result, SSL_TEST_CTX *test_ctx) { - if (result.servername != test_ctx->servername) { + if (test_ctx->servername != SSL_TEST_SERVERNAME_NONE + && result.servername != test_ctx->servername) { fprintf(stderr, "Client ServerName mismatch, expected %s, got %s\n.", ssl_servername_name(test_ctx->servername), ssl_servername_name(result.servername)); @@ -134,7 +135,7 @@ static int check_servername(HANDSHAKE_RESULT result, SSL_TEST_CTX *test_ctx) return 1; } -static int check_session_ticket_expected(HANDSHAKE_RESULT result, SSL_TEST_CTX *test_ctx) +static int check_session_ticket(HANDSHAKE_RESULT result, SSL_TEST_CTX *test_ctx) { if (test_ctx->session_ticket_expected == SSL_TEST_SESSION_TICKET_IGNORE) return 1; @@ -143,8 +144,8 @@ static int check_session_ticket_expected(HANDSHAKE_RESULT result, SSL_TEST_CTX * return 1; if (result.session_ticket != test_ctx->session_ticket_expected) { fprintf(stderr, "Client SessionTicketExpected mismatch, expected %s, got %s\n.", - ssl_session_ticket_expected_name(test_ctx->session_ticket_expected), - ssl_session_ticket_expected_name(result.session_ticket)); + ssl_session_ticket_name(test_ctx->session_ticket_expected), + ssl_session_ticket_name(result.session_ticket)); return 0; } return 1; @@ -163,29 +164,12 @@ static int check_test(HANDSHAKE_RESULT result, SSL_TEST_CTX *test_ctx) if (result.result == SSL_TEST_SUCCESS) { ret &= check_protocol(result, test_ctx); ret &= check_servername(result, test_ctx); - ret &= check_session_ticket_expected(result, test_ctx); + ret &= check_session_ticket(result, test_ctx); ret &= (result.session_ticket_do_not_call == 0); } return ret; } -static int servername_callback(SSL *s, int *ad, void *arg) -{ - const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name); - if (servername != NULL && !strcmp(servername, "server2")) { - SSL_CTX *new_ctx = (SSL_CTX*)arg; - SSL_set_SSL_CTX(s, new_ctx); - /* - * Copy over all the SSL_CTX options - reasonable behavior - * allows testing of cases where the options between two - * contexts differ/conflict - */ - SSL_clear_options(s, 0xFFFFFFFFL); - SSL_set_options(s, SSL_CTX_get_options(new_ctx)); - } - return SSL_TLSEXT_ERR_OK; -} - static int execute_test(SSL_TEST_FIXTURE fixture) { int ret = 0; @@ -206,21 +190,11 @@ static int execute_test(SSL_TEST_FIXTURE fixture) goto err; } - /* link the two contexts for SNI purposes */ - SSL_CTX_set_tlsext_servername_callback(server_ctx, servername_callback); - SSL_CTX_set_tlsext_servername_arg(server_ctx, server2_ctx); - /* - * The initial_ctx/session_ctx always handles the encrypt/decrypt of the - * session ticket. This ticket_key callback is assigned to the second - * session (assigned via SNI), and should never be invoked - */ - SSL_CTX_set_tlsext_ticket_key_cb(server2_ctx, do_not_call_session_ticket_callback); - test_ctx = SSL_TEST_CTX_create(conf, fixture.test_app); if (test_ctx == NULL) goto err; - result = do_handshake(server_ctx, client_ctx, test_ctx); + result = do_handshake(server_ctx, server2_ctx, client_ctx, test_ctx); ret = check_test(result, test_ctx); diff --git a/test/ssl_test_ctx.c b/test/ssl_test_ctx.c index 598c89996c..87060f3799 100644 --- a/test/ssl_test_ctx.c +++ b/test/ssl_test_ctx.c @@ -159,6 +159,7 @@ const char *ssl_verify_callback_name(ssl_verify_callback_t callback) /**************/ static const test_enum ssl_servername[] = { + {"None", SSL_TEST_SERVERNAME_NONE}, {"server1", SSL_TEST_SERVERNAME_SERVER1}, {"server2", SSL_TEST_SERVERNAME_SERVER2}, }; @@ -185,18 +186,17 @@ const char *ssl_servername_name(ssl_servername_t server) /* SessionTicketExpected */ /*************************/ -static const test_enum ssl_session_ticket_expected[] = { +static const test_enum ssl_session_ticket[] = { {"Ignore", SSL_TEST_SESSION_TICKET_IGNORE}, {"Yes", SSL_TEST_SESSION_TICKET_YES}, {"No", SSL_TEST_SESSION_TICKET_NO}, {"Broken", SSL_TEST_SESSION_TICKET_BROKEN}, }; -__owur static int parse_session_ticket_expected(SSL_TEST_CTX *test_ctx, - const char *value) +__owur static int parse_session_ticket(SSL_TEST_CTX *test_ctx, const char *value) { int ret_value; - if (!parse_enum(ssl_session_ticket_expected, OSSL_NELEM(ssl_session_ticket_expected), + if (!parse_enum(ssl_session_ticket, OSSL_NELEM(ssl_session_ticket), &ret_value, value)) { return 0; } @@ -204,10 +204,10 @@ __owur static int parse_session_ticket_expected(SSL_TEST_CTX *test_ctx, return 1; } -const char *ssl_session_ticket_expected_name(ssl_session_ticket_expected_t server) +const char *ssl_session_ticket_name(ssl_session_ticket_t server) { - return enum_name(ssl_session_ticket_expected, - OSSL_NELEM(ssl_session_ticket_expected), + return enum_name(ssl_session_ticket, + OSSL_NELEM(ssl_session_ticket), server); } @@ -227,7 +227,7 @@ static const ssl_test_ctx_option ssl_test_ctx_options[] = { { "Protocol", &parse_protocol }, { "ClientVerifyCallback", &parse_client_verify_callback }, { "ServerName", &parse_servername }, - { "SessionTicketExpected", &parse_session_ticket_expected }, + { "SessionTicketExpected", &parse_session_ticket }, }; diff --git a/test/ssl_test_ctx.h b/test/ssl_test_ctx.h index e7570856b1..e1541c0c98 100644 --- a/test/ssl_test_ctx.h +++ b/test/ssl_test_ctx.h @@ -27,7 +27,8 @@ typedef enum { } ssl_verify_callback_t; typedef enum { - SSL_TEST_SERVERNAME_SERVER1 = 0, /* Default */ + SSL_TEST_SERVERNAME_NONE = 0, /* Default */ + SSL_TEST_SERVERNAME_SERVER1, SSL_TEST_SERVERNAME_SERVER2 } ssl_servername_t; @@ -36,7 +37,7 @@ typedef enum { SSL_TEST_SESSION_TICKET_YES, SSL_TEST_SESSION_TICKET_NO, SSL_TEST_SESSION_TICKET_BROKEN, /* Special test */ -} ssl_session_ticket_expected_t; +} ssl_session_ticket_t; typedef struct ssl_test_ctx { /* Test expectations. */ @@ -55,7 +56,7 @@ typedef struct ssl_test_ctx { ssl_verify_callback_t client_verify_callback; /* One of a number of predefined server names use by the client */ ssl_servername_t servername; - ssl_session_ticket_expected_t session_ticket_expected; + ssl_session_ticket_t session_ticket_expected; } SSL_TEST_CTX; const char *ssl_test_result_name(ssl_test_result_t result); @@ -63,7 +64,7 @@ const char *ssl_alert_name(int alert); const char *ssl_protocol_name(int protocol); const char *ssl_verify_callback_name(ssl_verify_callback_t verify_callback); const char *ssl_servername_name(ssl_servername_t server); -const char *ssl_session_ticket_expected_name(ssl_session_ticket_expected_t server); +const char *ssl_session_ticket_name(ssl_session_ticket_t server); /* * Load the test case context from |conf|. diff --git a/test/ssl_test_ctx_test.c b/test/ssl_test_ctx_test.c index 6b202ef3e5..3f0101f836 100644 --- a/test/ssl_test_ctx_test.c +++ b/test/ssl_test_ctx_test.c @@ -72,8 +72,8 @@ static int SSL_TEST_CTX_equal(SSL_TEST_CTX *ctx, SSL_TEST_CTX *ctx2) } if (ctx->session_ticket_expected != ctx2->session_ticket_expected) { fprintf(stderr, "SessionTicketExpected mismatch: %s vs %s.\n", - ssl_session_ticket_expected_name(ctx->session_ticket_expected), - ssl_session_ticket_expected_name(ctx2->session_ticket_expected)); + ssl_session_ticket_name(ctx->session_ticket_expected), + ssl_session_ticket_name(ctx2->session_ticket_expected)); return 0; }