From 7dc7837889af010b174d8e0b2c64d1d8d41c7749 Mon Sep 17 00:00:00 2001 From: jsing <> Date: Tue, 12 Jan 2021 17:47:20 +0000 Subject: Split the record protection from the TLSv1.2 record layer. When changing cipher state, DTLS requires that the previous write protection state remain available so that messages can be retransmitted. Currently, this is done by DTLS saving and restoring various pointers, along with special casing to not free the cipher and hash where it would normally be freed for TLS (and requiring DTLS to free things at the appropriate times). This can be handled in a much cleaner manner by splitting the record protection from the record layer. This allows for the previous write state to be retained and restored by swapping a single pointer. Additionally, it also results in more readable and manageable code. This diff simply splits the record protection from the record layer - future changes will add support for maintaining and switching between write states. ok inoguchi@ tb@ --- src/lib/libssl/tls12_record_layer.c | 176 +++++++++++++++++++++--------------- 1 file changed, 101 insertions(+), 75 deletions(-) (limited to 'src/lib') diff --git a/src/lib/libssl/tls12_record_layer.c b/src/lib/libssl/tls12_record_layer.c index 600b73987e..2b331355be 100644 --- a/src/lib/libssl/tls12_record_layer.c +++ b/src/lib/libssl/tls12_record_layer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls12_record_layer.c,v 1.7 2021/01/07 15:37:19 jsing Exp $ */ +/* $OpenBSD: tls12_record_layer.c,v 1.8 2021/01/12 17:47:20 jsing Exp $ */ /* * Copyright (c) 2020 Joel Sing * @@ -21,35 +21,51 @@ #include "ssl_locl.h" -struct tls12_record_layer { - uint16_t version; - int dtls; - - uint8_t alert_desc; - - uint16_t read_epoch; - uint16_t write_epoch; +struct tls12_record_protection { + uint16_t epoch; - int read_stream_mac; - int write_stream_mac; + int stream_mac; - uint8_t *read_mac_key; - size_t read_mac_key_len; + uint8_t *mac_key; + size_t mac_key_len; /* * XXX - for now these are just pointers to externally managed * structs/memory. These should eventually be owned by the record layer. */ - SSL_AEAD_CTX *read_aead_ctx; - SSL_AEAD_CTX *write_aead_ctx; + SSL_AEAD_CTX *aead_ctx; + + EVP_CIPHER_CTX *cipher_ctx; + EVP_MD_CTX *hash_ctx; + + uint8_t *seq_num; +}; + +static struct tls12_record_protection * +tls12_record_protection_new(void) +{ + return calloc(1, sizeof(struct tls12_record_protection)); +} + +static void +tls12_record_protection_free(struct tls12_record_protection *rp) +{ + if (rp == NULL) + return; + + freezero(rp->mac_key, rp->mac_key_len); + + freezero(rp, sizeof(struct tls12_record_protection)); +} - EVP_CIPHER_CTX *read_cipher_ctx; - EVP_MD_CTX *read_hash_ctx; - EVP_CIPHER_CTX *write_cipher_ctx; - EVP_MD_CTX *write_hash_ctx; +struct tls12_record_layer { + uint16_t version; + int dtls; - uint8_t *read_seq_num; - uint8_t *write_seq_num; + uint8_t alert_desc; + + struct tls12_record_protection *read; + struct tls12_record_protection *write; }; struct tls12_record_layer * @@ -58,9 +74,18 @@ tls12_record_layer_new(void) struct tls12_record_layer *rl; if ((rl = calloc(1, sizeof(struct tls12_record_layer))) == NULL) - return NULL; + goto err; + if ((rl->read = tls12_record_protection_new()) == NULL) + goto err; + if ((rl->write = tls12_record_protection_new()) == NULL) + goto err; return rl; + + err: + tls12_record_layer_free(rl); + + return NULL; } void @@ -69,7 +94,8 @@ tls12_record_layer_free(struct tls12_record_layer *rl) if (rl == NULL) return; - freezero(rl->read_mac_key, rl->read_mac_key_len); + tls12_record_protection_free(rl->read); + tls12_record_protection_free(rl->write); freezero(rl, sizeof(struct tls12_record_layer)); } @@ -90,13 +116,13 @@ tls12_record_layer_set_version(struct tls12_record_layer *rl, uint16_t version) void tls12_record_layer_set_read_epoch(struct tls12_record_layer *rl, uint16_t epoch) { - rl->read_epoch = epoch; + rl->read->epoch = epoch; } void tls12_record_layer_set_write_epoch(struct tls12_record_layer *rl, uint16_t epoch) { - rl->write_epoch = epoch; + rl->write->epoch = epoch; } static void @@ -104,11 +130,11 @@ tls12_record_layer_set_read_state(struct tls12_record_layer *rl, SSL_AEAD_CTX *aead_ctx, EVP_CIPHER_CTX *cipher_ctx, EVP_MD_CTX *hash_ctx, int stream_mac) { - rl->read_aead_ctx = aead_ctx; + rl->read->aead_ctx = aead_ctx; - rl->read_cipher_ctx = cipher_ctx; - rl->read_hash_ctx = hash_ctx; - rl->read_stream_mac = stream_mac; + rl->read->cipher_ctx = cipher_ctx; + rl->read->hash_ctx = hash_ctx; + rl->read->stream_mac = stream_mac; } static void @@ -116,11 +142,11 @@ tls12_record_layer_set_write_state(struct tls12_record_layer *rl, SSL_AEAD_CTX *aead_ctx, EVP_CIPHER_CTX *cipher_ctx, EVP_MD_CTX *hash_ctx, int stream_mac) { - rl->write_aead_ctx = aead_ctx; + rl->write->aead_ctx = aead_ctx; - rl->write_cipher_ctx = cipher_ctx; - rl->write_hash_ctx = hash_ctx; - rl->write_stream_mac = stream_mac; + rl->write->cipher_ctx = cipher_ctx; + rl->write->hash_ctx = hash_ctx; + rl->write->stream_mac = stream_mac; } void @@ -128,28 +154,28 @@ tls12_record_layer_clear_read_state(struct tls12_record_layer *rl) { tls12_record_layer_set_read_state(rl, NULL, NULL, NULL, 0); tls12_record_layer_set_read_mac_key(rl, NULL, 0); - rl->read_seq_num = NULL; + rl->read->seq_num = NULL; } void tls12_record_layer_clear_write_state(struct tls12_record_layer *rl) { tls12_record_layer_set_write_state(rl, NULL, NULL, NULL, 0); - rl->write_seq_num = NULL; + rl->write->seq_num = NULL; } void tls12_record_layer_set_read_seq_num(struct tls12_record_layer *rl, uint8_t *seq_num) { - rl->read_seq_num = seq_num; + rl->read->seq_num = seq_num; } void tls12_record_layer_set_write_seq_num(struct tls12_record_layer *rl, uint8_t *seq_num) { - rl->write_seq_num = seq_num; + rl->write->seq_num = seq_num; } int @@ -194,18 +220,18 @@ int tls12_record_layer_set_read_mac_key(struct tls12_record_layer *rl, const uint8_t *mac_key, size_t mac_key_len) { - freezero(rl->read_mac_key, rl->read_mac_key_len); - rl->read_mac_key = NULL; - rl->read_mac_key_len = 0; + freezero(rl->read->mac_key, rl->read->mac_key_len); + rl->read->mac_key = NULL; + rl->read->mac_key_len = 0; if (mac_key == NULL || mac_key_len == 0) return 1; - if ((rl->read_mac_key = calloc(1, mac_key_len)) == NULL) + if ((rl->read->mac_key = calloc(1, mac_key_len)) == NULL) return 0; - memcpy(rl->read_mac_key, mac_key, mac_key_len); - rl->read_mac_key_len = mac_key_len; + memcpy(rl->read->mac_key, mac_key, mac_key_len); + rl->read->mac_key_len = mac_key_len; return 1; } @@ -328,19 +354,19 @@ tls12_record_layer_read_mac_cbc(struct tls12_record_layer *rl, CBB *cbb, * Must be constant time to avoid leaking details about CBC padding. */ - if (!ssl3_cbc_record_digest_supported(rl->read_hash_ctx)) + if (!ssl3_cbc_record_digest_supported(rl->read->hash_ctx)) goto err; if (!tls12_record_layer_pseudo_header(rl, content_type, content_len, - rl->read_epoch, rl->read_seq_num, SSL3_SEQUENCE_SIZE, + rl->read->epoch, rl->read->seq_num, SSL3_SEQUENCE_SIZE, &header, &header_len)) goto err; if (!CBB_add_space(cbb, &mac, mac_len)) goto err; - if (!ssl3_cbc_digest_record(rl->read_hash_ctx, mac, &out_mac_len, header, + if (!ssl3_cbc_digest_record(rl->read->hash_ctx, mac, &out_mac_len, header, content, content_len + mac_len, content_len + mac_len + padding_len, - rl->read_mac_key, rl->read_mac_key_len)) + rl->read->mac_key, rl->read->mac_key_len)) goto err; if (mac_len != out_mac_len) goto err; @@ -357,14 +383,14 @@ static int tls12_record_layer_read_mac(struct tls12_record_layer *rl, CBB *cbb, uint8_t content_type, const uint8_t *content, size_t content_len) { - EVP_CIPHER_CTX *enc = rl->read_cipher_ctx; + EVP_CIPHER_CTX *enc = rl->read->cipher_ctx; size_t out_len; if (EVP_CIPHER_CTX_mode(enc) == EVP_CIPH_CBC_MODE) return 0; - return tls12_record_layer_mac(rl, cbb, rl->read_hash_ctx, - rl->read_stream_mac, rl->read_epoch, rl->read_seq_num, + return tls12_record_layer_mac(rl, cbb, rl->read->hash_ctx, + rl->read->stream_mac, rl->read->epoch, rl->read->seq_num, SSL3_SEQUENCE_SIZE, content_type, content, content_len, &out_len); } @@ -373,8 +399,8 @@ tls12_record_layer_write_mac(struct tls12_record_layer *rl, CBB *cbb, uint8_t content_type, const uint8_t *content, size_t content_len, size_t *out_len) { - return tls12_record_layer_mac(rl, cbb, rl->write_hash_ctx, - rl->write_stream_mac, rl->write_epoch, rl->write_seq_num, + return tls12_record_layer_mac(rl, cbb, rl->write->hash_ctx, + rl->write->stream_mac, rl->write->epoch, rl->write->seq_num, SSL3_SEQUENCE_SIZE, content_type, content, content_len, out_len); } @@ -456,7 +482,7 @@ static int tls12_record_layer_open_record_plaintext(struct tls12_record_layer *rl, uint8_t content_type, CBS *fragment, uint8_t **out, size_t *out_len) { - if (rl->read_aead_ctx != NULL || rl->read_cipher_ctx != NULL) + if (rl->read->aead_ctx != NULL || rl->read->cipher_ctx != NULL) return 0; /* XXX - decrypt/process in place for now. */ @@ -470,7 +496,7 @@ static int tls12_record_layer_open_record_protected_aead(struct tls12_record_layer *rl, uint8_t content_type, CBS *fragment, uint8_t **out, size_t *out_len) { - const SSL_AEAD_CTX *aead = rl->read_aead_ctx; + const SSL_AEAD_CTX *aead = rl->read->aead_ctx; uint8_t *header = NULL, *nonce = NULL; size_t header_len = 0, nonce_len = 0; uint8_t *plain; @@ -482,7 +508,7 @@ tls12_record_layer_open_record_protected_aead(struct tls12_record_layer *rl, /* XXX - move to nonce allocated in record layer, matching TLSv1.3 */ if (aead->xor_fixed_nonce) { if (!tls12_record_layer_aead_xored_nonce(rl, aead, - rl->read_seq_num, &nonce, &nonce_len)) + rl->read->seq_num, &nonce, &nonce_len)) goto err; } else if (aead->variable_nonce_in_record) { if (!CBS_get_bytes(fragment, &var_nonce, @@ -493,7 +519,7 @@ tls12_record_layer_open_record_protected_aead(struct tls12_record_layer *rl, goto err; } else { if (!tls12_record_layer_aead_concat_nonce(rl, aead, - rl->read_seq_num, &nonce, &nonce_len)) + rl->read->seq_num, &nonce, &nonce_len)) goto err; } @@ -512,7 +538,7 @@ tls12_record_layer_open_record_protected_aead(struct tls12_record_layer *rl, plain_len = CBS_len(fragment) - aead->tag_len; if (!tls12_record_layer_pseudo_header(rl, content_type, plain_len, - epoch, rl->read_seq_num, SSL3_SEQUENCE_SIZE, &header, &header_len)) + epoch, rl->read->seq_num, SSL3_SEQUENCE_SIZE, &header, &header_len)) goto err; if (!EVP_AEAD_CTX_open(&aead->ctx, plain, out_len, plain_len, @@ -545,7 +571,7 @@ static int tls12_record_layer_open_record_protected_cipher(struct tls12_record_layer *rl, uint8_t content_type, CBS *fragment, uint8_t **out, size_t *out_len) { - EVP_CIPHER_CTX *enc = rl->read_cipher_ctx; + EVP_CIPHER_CTX *enc = rl->read->cipher_ctx; SSL3_RECORD_INTERNAL rrec; int block_size, eiv_len; uint8_t *mac = NULL; @@ -573,8 +599,8 @@ tls12_record_layer_open_record_protected_cipher(struct tls12_record_layer *rl, goto err; mac_len = 0; - if (rl->read_hash_ctx != NULL) { - mac_len = EVP_MD_CTX_size(rl->read_hash_ctx); + if (rl->read->hash_ctx != NULL) { + mac_len = EVP_MD_CTX_size(rl->read->hash_ctx); if (mac_len <= 0 || mac_len > EVP_MAX_MD_SIZE) goto err; } @@ -689,11 +715,11 @@ tls12_record_layer_open_record(struct tls12_record_layer *rl, uint8_t *buf, if (!CBS_get_u16_length_prefixed(&cbs, &fragment)) return 0; - if (rl->read_aead_ctx != NULL) { + if (rl->read->aead_ctx != NULL) { if (!tls12_record_layer_open_record_protected_aead(rl, content_type, &fragment, out, out_len)) return 0; - } else if (rl->read_cipher_ctx != NULL) { + } else if (rl->read->cipher_ctx != NULL) { if (!tls12_record_layer_open_record_protected_cipher(rl, content_type, &fragment, out, out_len)) return 0; @@ -704,7 +730,7 @@ tls12_record_layer_open_record(struct tls12_record_layer *rl, uint8_t *buf, } if (!rl->dtls) - tls1_record_sequence_increment(rl->read_seq_num); + tls1_record_sequence_increment(rl->read->seq_num); return 1; } @@ -713,7 +739,7 @@ static int tls12_record_layer_seal_record_plaintext(struct tls12_record_layer *rl, uint8_t content_type, const uint8_t *content, size_t content_len, CBB *out) { - if (rl->write_aead_ctx != NULL || rl->write_cipher_ctx != NULL) + if (rl->write->aead_ctx != NULL || rl->write->cipher_ctx != NULL) return 0; return CBB_add_bytes(out, content, content_len); @@ -723,7 +749,7 @@ static int tls12_record_layer_seal_record_protected_aead(struct tls12_record_layer *rl, uint8_t content_type, const uint8_t *content, size_t content_len, CBB *out) { - const SSL_AEAD_CTX *aead = rl->write_aead_ctx; + const SSL_AEAD_CTX *aead = rl->write->aead_ctx; uint8_t *header = NULL, *nonce = NULL; size_t header_len = 0, nonce_len = 0; size_t enc_record_len, out_len; @@ -734,22 +760,22 @@ tls12_record_layer_seal_record_protected_aead(struct tls12_record_layer *rl, /* XXX - move to nonce allocated in record layer, matching TLSv1.3 */ if (aead->xor_fixed_nonce) { if (!tls12_record_layer_aead_xored_nonce(rl, aead, - rl->write_seq_num, &nonce, &nonce_len)) + rl->write->seq_num, &nonce, &nonce_len)) goto err; } else { if (!tls12_record_layer_aead_concat_nonce(rl, aead, - rl->write_seq_num, &nonce, &nonce_len)) + rl->write->seq_num, &nonce, &nonce_len)) goto err; } if (aead->variable_nonce_in_record) { /* XXX - length check? */ - if (!CBB_add_bytes(out, rl->write_seq_num, aead->variable_nonce_len)) + if (!CBB_add_bytes(out, rl->write->seq_num, aead->variable_nonce_len)) goto err; } if (!tls12_record_layer_pseudo_header(rl, content_type, content_len, - epoch, rl->write_seq_num, SSL3_SEQUENCE_SIZE, &header, &header_len)) + epoch, rl->write->seq_num, SSL3_SEQUENCE_SIZE, &header, &header_len)) goto err; /* XXX EVP_AEAD_max_tag_len vs EVP_AEAD_CTX_tag_len. */ @@ -779,7 +805,7 @@ static int tls12_record_layer_seal_record_protected_cipher(struct tls12_record_layer *rl, uint8_t content_type, const uint8_t *content, size_t content_len, CBB *out) { - EVP_CIPHER_CTX *enc = rl->write_cipher_ctx; + EVP_CIPHER_CTX *enc = rl->write->cipher_ctx; size_t mac_len, pad_len; int block_size, eiv_len; uint8_t *enc_data, *eiv, *pad, pad_val; @@ -808,7 +834,7 @@ tls12_record_layer_seal_record_protected_cipher(struct tls12_record_layer *rl, goto err; mac_len = 0; - if (rl->write_hash_ctx != NULL) { + if (rl->write->hash_ctx != NULL) { if (!tls12_record_layer_write_mac(rl, &cbb, content_type, content, content_len, &mac_len)) goto err; @@ -865,18 +891,18 @@ tls12_record_layer_seal_record(struct tls12_record_layer *rl, return 0; if (rl->dtls) { if (!tls12_record_layer_build_seq_num(rl, cbb, - rl->write_epoch, rl->write_seq_num, + rl->write->epoch, rl->write->seq_num, SSL3_SEQUENCE_SIZE)) return 0; } if (!CBB_add_u16_length_prefixed(cbb, &fragment)) return 0; - if (rl->write_aead_ctx != NULL) { + if (rl->write->aead_ctx != NULL) { if (!tls12_record_layer_seal_record_protected_aead(rl, content_type, content, content_len, &fragment)) return 0; - } else if (rl->write_cipher_ctx != NULL) { + } else if (rl->write->cipher_ctx != NULL) { if (!tls12_record_layer_seal_record_protected_cipher(rl, content_type, content, content_len, &fragment)) return 0; @@ -889,7 +915,7 @@ tls12_record_layer_seal_record(struct tls12_record_layer *rl, if (!CBB_flush(cbb)) return 0; - tls1_record_sequence_increment(rl->write_seq_num); + tls1_record_sequence_increment(rl->write->seq_num); return 1; } -- cgit v1.2.3-55-g6feb