From a9d752cff710268706b6ca58fe75f675427e2baf Mon Sep 17 00:00:00 2001
From: huitema <huitema@huitema.net>
Date: Mon, 8 Oct 2018 21:36:37 -0700
Subject: [PATCH] Compute new rotated keys and test
---
UnitTest1/unittest1.cpp | 7 +++
picoquic/picoquic_internal.h | 2 +
picoquic/quicctx.c | 4 ++
picoquic/tls_api.c | 100 ++++++++++++++++++++++++++++++++---
picoquic/tls_api.h | 7 +++
picoquic_t/picoquic_t.c | 1 +
picoquictest/picoquictest.h | 1 +
picoquictest/tls_api_test.c | 70 ++++++++++++++++++++++++
8 files changed, 186 insertions(+), 6 deletions(-)
diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp
index 3bc169ac..336afbfd 100644
--- a/UnitTest1/unittest1.cpp
+++ b/UnitTest1/unittest1.cpp
@@ -686,6 +686,13 @@ namespace UnitTest1
Assert::AreEqual(ret, 0);
}
+
+ TEST_METHOD(new_rotated_key)
+ {
+ int ret = new_rotated_key_test();
+
+ Assert::AreEqual(ret, 0);
+ }
TEST_METHOD(stress)
{
diff --git a/picoquic/picoquic_internal.h b/picoquic/picoquic_internal.h
index 40b73237..d096d566 100644
--- a/picoquic/picoquic_internal.h
+++ b/picoquic/picoquic_internal.h
@@ -582,6 +582,8 @@ typedef struct st_picoquic_cnx_t {
picoquic_stream_head tls_stream[PICOQUIC_NUMBER_OF_EPOCHS]; /* Separate input/output from each epoch */
picoquic_crypto_context_t crypto_context[PICOQUIC_NUMBER_OF_EPOCHS]; /* Encryption and decryption objects */
+ picoquic_crypto_context_t crypto_context_old; /* Old encryption and decryption context after key rotation */
+ picoquic_crypto_context_t crypto_context_new; /* New encryption and decryption context just before key rotation */
/* Liveness detection */
uint64_t latest_progress_time; /* last local time at which the connection progressed */
diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c
index 1f317e8f..e2b2dd9c 100644
--- a/picoquic/quicctx.c
+++ b/picoquic/quicctx.c
@@ -1724,6 +1724,8 @@ int picoquic_reset_cnx(picoquic_cnx_t* cnx, uint64_t current_time)
picoquic_crypto_context_free(&cnx->crypto_context[k]);
}
+ picoquic_crypto_context_free(&cnx->crypto_context_new);
+
if (ret == 0) {
ret = picoquic_setup_initial_traffic_keys(cnx);
}
@@ -1840,6 +1842,8 @@ void picoquic_delete_cnx(picoquic_cnx_t* cnx)
picoquic_crypto_context_free(&cnx->crypto_context[i]);
}
+ picoquic_crypto_context_free(&cnx->crypto_context_new);
+
for (picoquic_packet_context_enum pc = 0;
pc < picoquic_nb_packet_context; pc++) {
picoquic_reset_packet_context(cnx, pc);
diff --git a/picoquic/tls_api.c b/picoquic/tls_api.c
index 114fd791..f2c29a2f 100644
--- a/picoquic/tls_api.c
+++ b/picoquic/tls_api.c
@@ -56,6 +56,8 @@ typedef struct st_picoquic_tls_ctx_t {
uint8_t ext_received[128];
size_t ext_received_length;
int ext_received_return;
+ uint8_t app_secret_enc[PTLS_MAX_SECRET_SIZE];
+ uint8_t app_secret_dec[PTLS_MAX_SECRET_SIZE];
} picoquic_tls_ctx_t;
int picoquic_receive_transport_extensions(picoquic_cnx_t* cnx, int extension_mode,
@@ -110,7 +112,7 @@ void picoquic_provide_received_transport_extensions(picoquic_cnx_t* cnx,
int* ext_received_return,
int* client_mode)
{
- picoquic_tls_ctx_t* ctx = cnx->tls_ctx;
+ picoquic_tls_ctx_t* ctx = (picoquic_tls_ctx_t*)cnx->tls_ctx;
*ext_received = ctx->ext_received;
*ext_received_length = ctx->ext_received_length;
@@ -634,10 +636,9 @@ static int picoquic_set_pn_enc_from_secret(void ** v_pn_enc, ptls_cipher_suite_t
}
-static int picoquic_set_key_from_secret(picoquic_cnx_t* cnx, ptls_cipher_suite_t * cipher, int is_enc, size_t epoch, const void *secret)
+static int picoquic_set_key_from_secret(picoquic_cnx_t* cnx, ptls_cipher_suite_t * cipher, int is_enc, picoquic_crypto_context_t * ctx, const void *secret)
{
int ret = 0;
- picoquic_crypto_context_t * ctx = &cnx->crypto_context[epoch];
if (is_enc != 0) {
ret = picoquic_set_aead_from_secret(&ctx->aead_encrypt, cipher, is_enc, secret);
@@ -686,6 +687,7 @@ typedef struct st_picoquic_update_traffic_key_t {
static int picoquic_update_traffic_key_callback(ptls_update_traffic_key_t * self, ptls_t *tls, int is_enc, size_t epoch, const void *secret)
{
picoquic_cnx_t* cnx = (picoquic_cnx_t*)*ptls_get_data_ptr(tls);
+ picoquic_tls_ctx_t * tls_ctx = (picoquic_tls_ctx_t *)cnx->tls_ctx;
ptls_context_t* ctx = (ptls_context_t*)cnx->quic->tls_master_ctx;
ptls_cipher_suite_t * cipher = ptls_get_cipher(tls);
UNREFERENCED_PARAMETER(self);
@@ -694,7 +696,11 @@ static int picoquic_update_traffic_key_callback(ptls_update_traffic_key_t * self
debug_dump(secret, (int)cipher->hash->digest_size);
#endif
- int ret = picoquic_set_key_from_secret(cnx, cipher, is_enc, epoch, secret);
+ int ret = picoquic_set_key_from_secret(cnx, cipher, is_enc, &cnx->crypto_context[epoch], secret);
+
+ if (ret == 0 && epoch == 3) {
+ memcpy((is_enc) ? tls_ctx->app_secret_enc : tls_ctx->app_secret_dec, secret, cipher->aead->key_size);
+ }
if (ctx->log_secret) {
static const char *log_labels[2][4] = {
@@ -798,17 +804,99 @@ int picoquic_setup_initial_traffic_keys(picoquic_cnx_t* cnx)
}
if (ret == 0) {
- ret = picoquic_set_key_from_secret(cnx, &cipher, 1, 0, secret1);
+ ret = picoquic_set_key_from_secret(cnx, &cipher, 1, &cnx->crypto_context[0], secret1);
}
if (ret == 0) {
- ret = picoquic_set_key_from_secret(cnx, &cipher, 0, 0, secret2);
+ ret = picoquic_set_key_from_secret(cnx, &cipher, 0, &cnx->crypto_context[0], secret2);
}
}
return ret;
}
+/*
+ * Key rotation.
+ *
+ * The old keys get moved to the old crypto context.
+ * The secrets are rotated.
+ * The new context gets informed.
+ *
+ * The key update is defined in RFC 8446 section 7.2 as:
+ * application_traffic_secret_N+1 =
+ * HKDF-Expand-Label(application_traffic_secret_N,
+ * "traffic upd", "", Hash.length)
+ * Label: PICOQUIC_LABEL_TRAFFIC_UPDATE
+ */
+static int picoquic_rotate_app_secret(ptls_cipher_suite_t * cipher, uint8_t * secret)
+{
+ int ret = 0;
+ uint8_t new_secret[PTLS_MAX_SECRET_SIZE];
+
+ ret = ptls_hkdf_expand_label(cipher->hash, new_secret,
+ cipher->aead->ctr_cipher->key_size, ptls_iovec_init(secret, cipher->aead->ctr_cipher->key_size),
+ PICOQUIC_LABEL_TRAFFIC_UPDATE, ptls_iovec_init(NULL, 0), PICOQUIC_LABEL_QUIC_BASE);
+ if (ret == 0) {
+ memcpy(secret, new_secret, cipher->aead->ctr_cipher->key_size);
+ }
+
+ return ret;
+}
+
+
+uint8_t * picoquic_get_app_secret(picoquic_cnx_t* cnx, int is_enc)
+{
+ picoquic_tls_ctx_t * tls_ctx = (picoquic_tls_ctx_t *)cnx->tls_ctx;
+
+ return (is_enc) ?tls_ctx->app_secret_enc:tls_ctx->app_secret_dec;
+}
+
+size_t picoquic_get_app_secret_size(picoquic_cnx_t* cnx)
+{
+ picoquic_tls_ctx_t * tls_ctx = (picoquic_tls_ctx_t *)cnx->tls_ctx;
+
+ ptls_cipher_suite_t * cipher = ptls_get_cipher(tls_ctx->tls);
+
+ return (cipher->aead->ctr_cipher->key_size);
+}
+
+int picoquic_compute_new_rotated_keys(picoquic_cnx_t * cnx)
+{
+ int ret = 0;
+ picoquic_tls_ctx_t * tls_ctx = (picoquic_tls_ctx_t *)cnx->tls_ctx;
+ ptls_cipher_suite_t * cipher = ptls_get_cipher(tls_ctx->tls);
+
+ /* Verify that the previous transition is complete */
+ if (cnx->crypto_context_new.aead_decrypt != NULL ||
+ cnx->crypto_context_new.aead_encrypt != NULL ||
+ cnx->crypto_context_new.pn_dec != NULL ||
+ cnx->crypto_context_new.pn_enc != NULL) {
+ ret = -1;
+ }
+
+ /* Recompute the secrets */
+ if (ret == 0) {
+ ret = picoquic_rotate_app_secret(cipher, tls_ctx->app_secret_enc);
+ }
+
+ if (ret == 0) {
+ ret = picoquic_set_key_from_secret(cnx, cipher, 1, &cnx->crypto_context_new, tls_ctx->app_secret_enc);
+ }
+
+ if (ret == 0) {
+ ret = picoquic_rotate_app_secret(cipher, tls_ctx->app_secret_dec);
+ }
+ if (ret == 0) {
+ ret = picoquic_set_key_from_secret(cnx, cipher, 0, &cnx->crypto_context_new, tls_ctx->app_secret_dec);
+ }
+
+ return 0;
+}
+
+/*
+ * Release the crypto context, and the associated keys.
+ */
+
void picoquic_crypto_context_free(picoquic_crypto_context_t * ctx)
{
if (ctx->aead_encrypt != NULL) {
diff --git a/picoquic/tls_api.h b/picoquic/tls_api.h
index 665d171b..7a01f6e4 100644
--- a/picoquic/tls_api.h
+++ b/picoquic/tls_api.h
@@ -29,6 +29,8 @@
#define PICOQUIC_LABEL_INITIAL_CLIENT "client in"
#define PICOQUIC_LABEL_INITIAL_SERVER "server in"
+#define PICOQUIC_LABEL_TRAFFIC_UPDATE "traffic upd"
+
#define PICOQUIC_LABEL_KEY "key"
#define PICOQUIC_LABEL_IV "iv"
#define PICOQUIC_LABEL_PN "pn"
@@ -93,6 +95,10 @@ int picoquic_setup_initial_secrets(
int picoquic_setup_initial_traffic_keys(picoquic_cnx_t* cnx);
+uint8_t * picoquic_get_app_secret(picoquic_cnx_t* cnx, int is_enc);
+size_t picoquic_get_app_secret_size(picoquic_cnx_t* cnx);
+int picoquic_compute_new_rotated_keys(picoquic_cnx_t * cnx);
+
void picoquic_crypto_context_free(picoquic_crypto_context_t * ctx);
void * picoquic_setup_test_aead_context(int is_encrypt, const uint8_t * secret);
@@ -123,4 +129,5 @@ int picoquic_tls_client_authentication_activated(picoquic_quic_t* quic);
int picoquic_get_retry_token(picoquic_quic_t* quic, uint8_t * base, size_t len, uint8_t * cid, uint8_t cid_len,
uint8_t * token, uint8_t token_length);
+
#endif /* TLS_API_H */
diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c
index 5330e7f8..2d97918a 100644
--- a/picoquic_t/picoquic_t.c
+++ b/picoquic_t/picoquic_t.c
@@ -138,6 +138,7 @@ static const picoquic_test_def_t test_table[] = {
{ "retire_cnxid", retire_cnxid_test },
{ "server_busy", server_busy_test },
{ "initial_close", initial_close_test },
+ { "new_rotated_key", new_rotated_key_test},
{ "stress", stress_test },
{ "fuzz", fuzz_test },
{ "fuzz_initial", fuzz_initial_test}
diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h
index 0a64395d..c3cfd67a 100644
--- a/picoquictest/picoquictest.h
+++ b/picoquictest/picoquictest.h
@@ -136,6 +136,7 @@ int retire_cnxid_test();
int server_busy_test();
int initial_close_test();
int fuzz_initial_test();
+int new_rotated_key_test();
#ifdef __cplusplus
}
diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c
index 280b16d7..0972f41d 100644
--- a/picoquictest/tls_api_test.c
+++ b/picoquictest/tls_api_test.c
@@ -4287,5 +4287,75 @@ int initial_close_test()
test_ctx = NULL;
}
+ return ret;
+}
+
+/*
+ * Test that rotated keys are computed in a compatible way on client and server.
+ */
+
+int new_rotated_key_test()
+{
+ uint64_t loss_mask = 0;
+ uint64_t simulated_time = 0;
+ picoquic_test_tls_api_ctx_t* test_ctx = NULL;
+ int ret = tls_api_init_ctx(&test_ctx, 0, PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, 0, 0, 0);
+
+ if (ret == 0) {
+ ret = tls_api_connection_loop(test_ctx, &loss_mask, 0, &simulated_time);
+ }
+
+ if (ret == 0) {
+ ret = wait_application_pn_enc_ready(test_ctx, &simulated_time);
+ }
+
+
+ for (int i = 1; ret == 0 && i <= 3; i++) {
+ if (ret == 0)
+ {
+ /* Try to compute rotated keys on server */
+ ret = picoquic_compute_new_rotated_keys(test_ctx->cnx_server);
+ if (ret != 0) {
+ DBG_PRINTF("Could not rotate server key, ret: %x\n", ret);
+ }
+ }
+
+ if (ret == 0)
+ {
+ /* Try to compute rotated keys on client */
+ ret = picoquic_compute_new_rotated_keys(test_ctx->cnx_client);
+ if (ret != 0) {
+ DBG_PRINTF("Could not rotate server key, round %d, ret: %x\n", i, ret);
+ }
+ }
+
+ if (ret == 0)
+ {
+ /* Compare server encryption and client decryption */
+ size_t key_size = picoquic_get_app_secret_size(test_ctx->cnx_client);
+
+ if (key_size != picoquic_get_app_secret_size(test_ctx->cnx_server)) {
+ DBG_PRINTF("Round %d. Key sizes dont match, client: %d, server: %d\n", i, key_size, picoquic_get_app_secret_size(test_ctx->cnx_server));
+ ret = -1;
+ }
+ else if (memcmp(picoquic_get_app_secret(test_ctx->cnx_server, 1), picoquic_get_app_secret(test_ctx->cnx_client, 0), key_size) != 0) {
+ DBG_PRINTF("Round %d. Server encryption secret does not match client decryption secret\n", i);
+ ret = -1;
+ }
+ else if (memcmp(picoquic_get_app_secret(test_ctx->cnx_server, 0), picoquic_get_app_secret(test_ctx->cnx_client, 1), key_size) != 0) {
+ DBG_PRINTF("Round %d. Server decryption secret does not match client encryption secret\n", i);
+ ret = -1;
+ }
+ }
+
+ picoquic_crypto_context_free(&test_ctx->cnx_server->crypto_context_new);
+ picoquic_crypto_context_free(&test_ctx->cnx_client->crypto_context_new);
+ }
+
+ if (test_ctx != NULL) {
+ tls_api_delete_ctx(test_ctx);
+ test_ctx = NULL;
+ }
+
return ret;
}
\ No newline at end of file
--
GitLab