The branch master has been updated via f1a5939f177becfaf465f9cf5a834ce6341276c4 (commit) via 2c7bd6921172c6a63cb7a111e84578fc7dca5a6f (commit) from 3f5616d734a92fdf99ab827f21e5b6cab85e7194 (commit)
- Log ----------------------------------------------------------------- commit f1a5939f177becfaf465f9cf5a834ce6341276c4 Author: Cory Benfield <lukas...@gmail.com> Date: Tue Jan 31 14:56:31 2017 +0000 Test logging TLSv1.3 secrets. Reviewed-by: Richard Levitte <levi...@openssl.org> Reviewed-by: Matt Caswell <m...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/2287) commit 2c7bd6921172c6a63cb7a111e84578fc7dca5a6f Author: Cory Benfield <lukas...@gmail.com> Date: Tue Jan 31 14:56:15 2017 +0000 Add support for logging out TLSv1.3 secrets Reviewed-by: Richard Levitte <levi...@openssl.org> Reviewed-by: Matt Caswell <m...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/2287) ----------------------------------------------------------------------- Summary of changes: ssl/ssl_lib.c | 34 +++-------- ssl/ssl_locl.h | 18 ++++-- ssl/statem/statem_lib.c | 11 ++-- ssl/tls13_enc.c | 10 +++ test/sslapitest.c | 157 +++++++++++++++++++++++++++++++++++++++--------- test/tls13secretstest.c | 8 +++ 6 files changed, 173 insertions(+), 65 deletions(-) diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index f027f1a..42d49d0 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -4441,32 +4441,16 @@ int ssl_log_rsa_client_key_exchange(SSL *ssl, premaster_len); } -int ssl_log_master_secret(SSL *ssl, - const uint8_t *client_random, - size_t client_random_len, - const uint8_t *master, - size_t master_len) +int ssl_log_secret(SSL *ssl, + const char *label, + const uint8_t *secret, + size_t secret_len) { - /* - * TLSv1.3 changes the derivation of the master secret compared to earlier - * TLS versions, meaning that logging it out is less useful. Instead we - * want to log out other secrets: specifically, the handshake and - * application traffic secrets. For this reason, if this function is called - * for TLSv1.3 we don't bother logging, and just return success - * immediately. - */ - if (SSL_IS_TLS13(ssl)) return 1; - - if (client_random_len != 32) { - SSLerr(SSL_F_SSL_LOG_MASTER_SECRET, ERR_R_INTERNAL_ERROR); - return 0; - } - - return nss_keylog_int("CLIENT_RANDOM", + return nss_keylog_int(label, ssl, - client_random, - client_random_len, - master, - master_len); + ssl->s3->client_random, + SSL3_RANDOM_SIZE, + secret, + secret_len); } diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 26580b0..53a33e9 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2287,13 +2287,19 @@ __owur int ssl_log_rsa_client_key_exchange(SSL *ssl, const uint8_t *premaster, size_t premaster_len); -/* ssl_log_master_secret logs |master| to the SSL_CTX associated with |ssl|, if - * logging is enabled. It returns one on success and zero on failure. The entry - * is identified by |client_random|. +/* + * ssl_log_secret logs |secret| to the SSL_CTX associated with |ssl|, if + * logging is available. It returns one on success and zero on failure. It tags + * the entry with |label|. */ -__owur int ssl_log_master_secret(SSL *ssl, const uint8_t *client_random, - size_t client_random_len, - const uint8_t *master, size_t master_len); +__owur int ssl_log_secret(SSL *ssl, const char *label, + const uint8_t *secret, size_t secret_len); + +#define MASTER_SECRET_LABEL "CLIENT_RANDOM" +#define CLIENT_HANDSHAKE_LABEL "CLIENT_HANDSHAKE_TRAFFIC_SECRET" +#define SERVER_HANDSHAKE_LABEL "SERVER_HANDSHAKE_TRAFFIC_SECRET" +#define CLIENT_APPLICATION_LABEL "CLIENT_TRAFFIC_SECRET_0" +#define SERVER_APPLICATION_LABEL "SERVER_TRAFFIC_SECRET_0" /* s3_cbc.c */ __owur char ssl3_cbc_record_digest_supported(const EVP_MD_CTX *ctx); diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index e21a102..4b021f9 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -470,10 +470,13 @@ int tls_construct_finished(SSL *s, WPACKET *pkt) goto err; } - /* Log the master secret, if logging is enabled. */ - if (!ssl_log_master_secret(s, s->s3->client_random, SSL3_RANDOM_SIZE, - s->session->master_key, - s->session->master_key_length)) + /* + * Log the master secret, if logging is enabled. We don't log it for + * TLSv1.3: there's a different key schedule for that. + */ + if (!SSL_IS_TLS13(s) && !ssl_log_secret(s, MASTER_SECRET_LABEL, + s->session->master_key, + s->session->master_key_length)) return 0; /* diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 7c217c1..0d29dae 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -261,6 +261,7 @@ int tls13_change_cipher_state(SSL *s, int which) unsigned char *hash = hashval; unsigned char *insecret; unsigned char *finsecret = NULL; + const char *log_label = NULL; EVP_CIPHER_CTX *ciph_ctx; const EVP_CIPHER *ciph = s->s3->tmp.new_sym_enc; size_t ivlen, keylen, finsecretlen = 0; @@ -306,10 +307,12 @@ int tls13_change_cipher_state(SSL *s, int which) finsecretlen = EVP_MD_size(ssl_handshake_md(s)); label = client_handshake_traffic; labellen = sizeof(client_handshake_traffic) - 1; + log_label = CLIENT_HANDSHAKE_LABEL; } else { insecret = s->master_secret; label = client_application_traffic; labellen = sizeof(client_application_traffic) - 1; + log_label = CLIENT_APPLICATION_LABEL; /* * For this we only use the handshake hashes up until the server * Finished hash. We do not include the client's Finished, which is @@ -325,10 +328,12 @@ int tls13_change_cipher_state(SSL *s, int which) finsecretlen = EVP_MD_size(ssl_handshake_md(s)); label = server_handshake_traffic; labellen = sizeof(server_handshake_traffic) - 1; + log_label = SERVER_HANDSHAKE_LABEL; } else { insecret = s->master_secret; label = server_application_traffic; labellen = sizeof(server_application_traffic) - 1; + log_label = SERVER_APPLICATION_LABEL; } } @@ -370,6 +375,11 @@ int tls13_change_cipher_state(SSL *s, int which) keylen = EVP_CIPHER_key_length(ciph); ivlen = EVP_CIPHER_iv_length(ciph); + if (!ssl_log_secret(s, log_label, secret, hashlen)) { + SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); + goto err; + } + if (!tls13_derive_key(s, secret, key, keylen) || !tls13_derive_iv(s, secret, iv, ivlen) || (finsecret != NULL && !tls13_derive_finishedkey(s, diff --git a/test/sslapitest.c b/test/sslapitest.c index d76357a..47f008a 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -41,6 +41,19 @@ static X509 *ocspcert = NULL; #define NUM_EXTRA_CERTS 40 +/* + * This structure is used to validate that the correct number of log messages + * of various types are emitted when emitting secret logs. + */ +struct sslapitest_log_counts { + unsigned int rsa_key_exchange_count; + unsigned int master_secret_count; + unsigned int client_handshake_secret_count; + unsigned int server_handshake_secret_count; + unsigned int client_application_secret_count; + unsigned int server_application_secret_count; +}; + static void client_keylog_callback(const SSL *ssl, const char *line) { int line_length = strlen(line); @@ -105,38 +118,51 @@ static int compare_hex_encoded_buffer(const char *hex_encoded, } static int test_keylog_output(char *buffer, const SSL *ssl, - const SSL_SESSION *session) { - int saw_client_random = 0; + const SSL_SESSION *session, + struct sslapitest_log_counts *expected) { char *token = NULL; unsigned char actual_client_random[SSL3_RANDOM_SIZE] = {0}; size_t client_random_size = SSL3_RANDOM_SIZE; unsigned char actual_master_key[SSL_MAX_MASTER_KEY_LENGTH] = {0}; size_t master_key_size = SSL_MAX_MASTER_KEY_LENGTH; + unsigned int rsa_key_exchange_count = 0; + unsigned int master_secret_count = 0; + unsigned int client_handshake_secret_count = 0; + unsigned int server_handshake_secret_count = 0; + unsigned int client_application_secret_count = 0; + unsigned int server_application_secret_count = 0; token = strtok(buffer, " \n"); while (token) { if (strcmp(token, "RSA") == 0) { - /* Premaster secret. Tokens should be: 16 ASCII bytes of + /* + * Premaster secret. Tokens should be: 16 ASCII bytes of * hex-encoded encrypted secret, then the hex-encoded pre-master * secret. */ token = strtok(NULL, " \n"); if (!token) { printf("Unexpectedly short premaster secret log.\n"); - return -1; + return 0; } if (strlen(token) != 16) { printf("Bad value for encrypted secret: %s\n", token); - return -1; + return 0; } token = strtok(NULL, " \n"); if (!token) { printf("Unexpectedly short premaster secret log.\n"); - return -1; + return 0; } - /* TODO: Can I check this sensibly? */ + /* + * We can't sensibly check the log because the premaster secret is + * transient, and OpenSSL doesn't keep hold of it once the master + * secret is generated. + */ + rsa_key_exchange_count++; } else if (strcmp(token, "CLIENT_RANDOM") == 0) { - /* Master secret. Tokens should be: 64 ASCII bytes of hex-encoded + /* + * Master secret. Tokens should be: 64 ASCII bytes of hex-encoded * client random, then the hex-encoded master secret. */ client_random_size = SSL_get_client_random(ssl, @@ -144,28 +170,28 @@ static int test_keylog_output(char *buffer, const SSL *ssl, SSL3_RANDOM_SIZE); if (client_random_size != SSL3_RANDOM_SIZE) { printf("Unexpected short client random.\n"); - return -1; + return 0; } token = strtok(NULL, " \n"); if (!token) { printf("Unexpected short master secret log.\n"); - return -1; + return 0; } if (strlen(token) != 64) { printf("Bad value for client random: %s\n", token); - return -1; + return 0; } if (compare_hex_encoded_buffer(token, 64, actual_client_random, client_random_size)) { printf("Bad value for client random: %s\n", token); - return -1; + return 0; } token = strtok(NULL, " \n"); if (!token) { printf("Unexpectedly short master secret log.\n"); - return -1; + return 0; } master_key_size = SSL_SESSION_get_master_key(session, @@ -173,25 +199,82 @@ static int test_keylog_output(char *buffer, const SSL *ssl, master_key_size); if (!master_key_size) { printf("Error getting master key to compare.\n"); - return -1; + return 0; } if (compare_hex_encoded_buffer(token, strlen(token), actual_master_key, master_key_size)) { printf("Bad value for master key: %s\n", token); - return -1; + return 0; } - saw_client_random = 1; + master_secret_count++; + } else if ((strcmp(token, "CLIENT_HANDSHAKE_TRAFFIC_SECRET") == 0) || + (strcmp(token, "SERVER_HANDSHAKE_TRAFFIC_SECRET") == 0) || + (strcmp(token, "CLIENT_TRAFFIC_SECRET_0") == 0) || + (strcmp(token, "SERVER_TRAFFIC_SECRET_0") == 0)) { + /* + * TLSv1.3 secret. Tokens should be: 64 ASCII bytes of hex-encoded + * client random, and then the hex-encoded secret. In this case, + * we treat all of these secrets identically and then just + * distinguish between them when counting what we saw. + */ + if (strcmp(token, "CLIENT_HANDSHAKE_TRAFFIC_SECRET") == 0) + client_handshake_secret_count++; + else if (strcmp(token, "SERVER_HANDSHAKE_TRAFFIC_SECRET") == 0) + server_handshake_secret_count++; + else if (strcmp(token, "CLIENT_TRAFFIC_SECRET_0") == 0) + client_application_secret_count++; + else if (strcmp(token, "SERVER_TRAFFIC_SECRET_0") == 0) + server_application_secret_count++; + + client_random_size = SSL_get_client_random(ssl, + actual_client_random, + SSL3_RANDOM_SIZE); + if (client_random_size != SSL3_RANDOM_SIZE) { + printf("Unexpected short client random.\n"); + return 0; + } + + token = strtok(NULL, " \n"); + if (!token) { + printf("Unexpected short client handshake secret log.\n"); + return 0; + } + if (strlen(token) != 64) { + printf("Bad value for client random: %s\n", token); + return 0; + } + if (compare_hex_encoded_buffer(token, 64, actual_client_random, + client_random_size)) { + printf("Bad value for client random: %s\n", token); + return 0; + } + + token = strtok(NULL, " \n"); + if (!token) { + printf("Unexpectedly short master secret log.\n"); + return 0; + } + + /* + * TODO(TLS1.3): test that application traffic secrets are what + * we expect */ } else { printf("Unexpected token in buffer: %s\n", token); - return -1; + return 0; } token = strtok(NULL, " \n"); } - return saw_client_random; + /* Return whether we got what we expected. */ + return ((rsa_key_exchange_count == expected->rsa_key_exchange_count) && + (master_secret_count == expected->master_secret_count) && + (client_handshake_secret_count == expected->client_handshake_secret_count) && + (server_handshake_secret_count == expected->server_handshake_secret_count) && + (client_application_secret_count == expected->client_application_secret_count) && + (server_application_secret_count == expected->server_application_secret_count)); } static int test_keylog(void) { @@ -199,6 +282,7 @@ static int test_keylog(void) { SSL *clientssl = NULL, *serverssl = NULL; int testresult = 0; int rc; + struct sslapitest_log_counts expected = {0}; /* Clean up logging space */ memset(client_log_buffer, 0, LOG_BUFFER_SIZE + 1); @@ -269,17 +353,23 @@ static int test_keylog(void) { goto end; } - /* Now we want to test that our output data was vaguely sensible. We + /* + * Now we want to test that our output data was vaguely sensible. We * do that by using strtok and confirming that we have more or less the - * data we expect. + * data we expect. For both client and server, we expect to see one master + * secret. The client should also see a RSA key exchange. */ - if (test_keylog_output(client_log_buffer, clientssl, - SSL_get_session(clientssl)) != 1) { + expected.rsa_key_exchange_count = 1; + expected.master_secret_count = 1; + if (!test_keylog_output(client_log_buffer, clientssl, + SSL_get_session(clientssl), &expected)) { printf("Error encountered in client log buffer\n"); goto end; } - if (test_keylog_output(server_log_buffer, serverssl, - SSL_get_session(serverssl)) != 1) { + + expected.rsa_key_exchange_count = 0; + if (!test_keylog_output(server_log_buffer, serverssl, + SSL_get_session(serverssl), &expected)) { printf("Error encountered in server log buffer\n"); goto end; } @@ -300,6 +390,7 @@ static int test_keylog_no_master_key(void) { SSL_CTX *cctx = NULL, *sctx = NULL; SSL *clientssl = NULL, *serverssl = NULL; int testresult = 0; + struct sslapitest_log_counts expected = {0}; /* Clean up logging space */ memset(client_log_buffer, 0, LOG_BUFFER_SIZE + 1); @@ -354,16 +445,22 @@ static int test_keylog_no_master_key(void) { goto end; } - /* Now we want to test that our output data was vaguely sensible. For this - * test, we expect no CLIENT_RANDOM entry. + /* + * Now we want to test that our output data was vaguely sensible. For this + * test, we expect no CLIENT_RANDOM entry becuase it doesn't make sense for + * TLSv1.3, but we do expect both client and server to emit keys. */ - if (test_keylog_output(client_log_buffer, clientssl, - SSL_get_session(clientssl)) != 0) { + expected.client_handshake_secret_count = 1; + expected.server_handshake_secret_count = 1; + expected.client_application_secret_count = 1; + expected.server_application_secret_count = 1; + if (!test_keylog_output(client_log_buffer, clientssl, + SSL_get_session(clientssl), &expected)) { printf("Error encountered in client log buffer\n"); goto end; } - if (test_keylog_output(server_log_buffer, serverssl, - SSL_get_session(serverssl)) != 0) { + if (!test_keylog_output(server_log_buffer, serverssl, + SSL_get_session(serverssl), &expected)) { printf("Error encountered in server log buffer\n"); goto end; } diff --git a/test/tls13secretstest.c b/test/tls13secretstest.c index 69b85b8..7f177bf 100644 --- a/test/tls13secretstest.c +++ b/test/tls13secretstest.c @@ -184,6 +184,14 @@ int tls1_alert_code(int code) return code; } +int ssl_log_secret(SSL *ssl, + const char *label, + const uint8_t *secret, + size_t secret_len) +{ + return 1; +} + /* End of mocked out code */ static int test_secret(SSL *s, unsigned char *prk, _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits