[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Thu Feb 2 09:43:00 UTC 2017
The branch master has been updated
via f1a5939f177becfaf465f9cf5a834ce6341276c4 (commit)
via 2c7bd6921172c6a63cb7a111e84578fc7dca5a6f (commit)
from 3f5616d734a92fdf99ab827f21e5b6cab85e7194 (commit)
- Log -----------------------------------------------------------------
commit f1a5939f177becfaf465f9cf5a834ce6341276c4
Author: Cory Benfield <lukasaoz at gmail.com>
Date: Tue Jan 31 14:56:31 2017 +0000
Test logging TLSv1.3 secrets.
Reviewed-by: Richard Levitte <levitte at openssl.org>
Reviewed-by: Matt Caswell <matt at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2287)
commit 2c7bd6921172c6a63cb7a111e84578fc7dca5a6f
Author: Cory Benfield <lukasaoz at gmail.com>
Date: Tue Jan 31 14:56:15 2017 +0000
Add support for logging out TLSv1.3 secrets
Reviewed-by: Richard Levitte <levitte at openssl.org>
Reviewed-by: Matt Caswell <matt at 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,
More information about the openssl-commits
mailing list