Move AEAD handling into the new TLSv1.2 record layer.

ok tb@

4 files changed, 102 insertions, 134 deletions

diff --git a/src/lib/libssl/ssl_lib.c b/src/lib/libssl/ssl_lib.c
index 5cf4be74aa..b67f8569cc 100644
--- a/src/lib/libssl/ssl_lib.c
+++ b/src/lib/libssl/ssl_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.243 2021/01/26 18:45:32 tb Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.244 2021/01/28 17:00:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2618,12 +2618,6 @@ ssl_clear_cipher_read_state(SSL *s)
         s->read_hash = NULL;
 
         tls12_record_layer_clear_read_state(s->internal->rl);
-
-        if (s->internal->aead_read_ctx != NULL) {
-                EVP_AEAD_CTX_cleanup(&s->internal->aead_read_ctx->ctx);
-                free(s->internal->aead_read_ctx);
-                s->internal->aead_read_ctx = NULL;
-        }
 }
 
 void
@@ -2635,12 +2629,6 @@ ssl_clear_cipher_write_state(SSL *s)
         s->internal->write_hash = NULL;
 
         tls12_record_layer_clear_write_state(s->internal->rl);
-
-        if (s->internal->aead_write_ctx != NULL) {
-                EVP_AEAD_CTX_cleanup(&s->internal->aead_write_ctx->ctx);
-                free(s->internal->aead_write_ctx);
-                s->internal->aead_write_ctx = NULL;
-        }
 }
 
 /* Fix this function so that it takes an optional type parameter */
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 4390361c66..d5298d7af1 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.317 2021/01/26 14:22:20 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.318 2021/01/28 17:00:39 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -481,6 +481,8 @@ int tls12_record_layer_write_overhead(struct tls12_record_layer *rl,
     size_t *overhead);
 int tls12_record_layer_read_protected(struct tls12_record_layer *rl);
 int tls12_record_layer_write_protected(struct tls12_record_layer *rl);
+void tls12_record_layer_set_aead(struct tls12_record_layer *rl,
+    const EVP_AEAD *aead);
 void tls12_record_layer_set_version(struct tls12_record_layer *rl,
     uint16_t version);
 void tls12_record_layer_set_write_epoch(struct tls12_record_layer *rl,
@@ -758,14 +760,6 @@ typedef struct ssl_internal_st {
 
         STACK_OF(SSL_CIPHER) *cipher_list_tls13;
 
-        SSL_AEAD_CTX *aead_read_ctx;    /* AEAD context. If non-NULL, then
-                                           enc_read_ctx and read_hash are
-                                           ignored. */
-
-        SSL_AEAD_CTX *aead_write_ctx;   /* AEAD context. If non-NULL, then
-                                           enc_write_ctx and write_hash are
-                                           ignored. */
-
         EVP_CIPHER_CTX *enc_write_ctx;          /* cryptographic state */
         EVP_MD_CTX *write_hash;                 /* used for mac generation */
 
diff --git a/src/lib/libssl/t1_enc.c b/src/lib/libssl/t1_enc.c
index a0b377389b..b84a5347f1 100644
--- a/src/lib/libssl/t1_enc.c
+++ b/src/lib/libssl/t1_enc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_enc.c,v 1.130 2021/01/26 14:22:20 jsing Exp $ */
+/* $OpenBSD: t1_enc.c,v 1.131 2021/01/28 17:00:39 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -311,90 +311,6 @@ tls1_generate_key_block(SSL *s, uint8_t *key_block, size_t key_block_len)
 }
 
 /*
- * tls1_aead_ctx_init allocates aead_ctx, if needed. It returns 1 on success
- * and 0 on failure.
- */
-static int
-tls1_aead_ctx_init(SSL_AEAD_CTX **aead_ctx)
-{
-        if (*aead_ctx != NULL) {
-                EVP_AEAD_CTX_cleanup(&(*aead_ctx)->ctx);
-                return (1);
-        }
-
-        *aead_ctx = malloc(sizeof(SSL_AEAD_CTX));
-        if (*aead_ctx == NULL) {
-                SSLerrorx(ERR_R_MALLOC_FAILURE);
-                return (0);
-        }
-
-        return (1);
-}
-
-static int
-tls1_change_cipher_state_aead(SSL *s, char is_read, const unsigned char *key,
-    unsigned int key_len, const unsigned char *iv, unsigned int iv_len)
-{
-        const EVP_AEAD *aead = S3I(s)->tmp.new_aead;
-        SSL_AEAD_CTX *aead_ctx;
-
-        /* XXX - Need to avoid clearing write state for DTLS. */
-        if (SSL_is_dtls(s))
-                return 0;
-
-        if (is_read) {
-                ssl_clear_cipher_read_state(s);
-                if (!tls1_aead_ctx_init(&s->internal->aead_read_ctx))
-                        return 0;
-                aead_ctx = s->internal->aead_read_ctx;
-
-                if (!tls12_record_layer_set_read_aead(s->internal->rl, aead_ctx))
-                        return 0;
-        } else {
-                ssl_clear_cipher_write_state(s);
-                if (!tls1_aead_ctx_init(&s->internal->aead_write_ctx))
-                        return 0;
-                aead_ctx = s->internal->aead_write_ctx;
-
-                if (!tls12_record_layer_set_write_aead(s->internal->rl, aead_ctx))
-                        return 0;
-        }
-
-        if (!EVP_AEAD_CTX_init(&aead_ctx->ctx, aead, key, key_len,
-            EVP_AEAD_DEFAULT_TAG_LENGTH, NULL))
-                return (0);
-        if (iv_len > sizeof(aead_ctx->fixed_nonce)) {
-                SSLerrorx(ERR_R_INTERNAL_ERROR);
-                return (0);
-        }
-        memcpy(aead_ctx->fixed_nonce, iv, iv_len);
-        aead_ctx->fixed_nonce_len = iv_len;
-        aead_ctx->variable_nonce_len = 8;  /* always the case, currently. */
-        aead_ctx->variable_nonce_in_record =
-            (S3I(s)->hs.new_cipher->algorithm2 &
-            SSL_CIPHER_ALGORITHM2_VARIABLE_NONCE_IN_RECORD) != 0;
-        aead_ctx->xor_fixed_nonce =
-            S3I(s)->hs.new_cipher->algorithm_enc == SSL_CHACHA20POLY1305;
-        aead_ctx->tag_len = EVP_AEAD_max_overhead(aead);
-
-        if (aead_ctx->xor_fixed_nonce) {
-                if (aead_ctx->fixed_nonce_len != EVP_AEAD_nonce_length(aead) ||
-                    aead_ctx->variable_nonce_len > EVP_AEAD_nonce_length(aead)) {
-                        SSLerrorx(ERR_R_INTERNAL_ERROR);
-                        return (0);
-                }
-        } else {
-                if (aead_ctx->variable_nonce_len + aead_ctx->fixed_nonce_len !=
-                    EVP_AEAD_nonce_length(aead)) {
-                        SSLerrorx(ERR_R_INTERNAL_ERROR);
-                        return (0);
-                }
-        }
-
-        return (1);
-}
-
-/*
  * tls1_change_cipher_state_cipher performs the work needed to switch cipher
  * states when using EVP_CIPHER. The argument is_read is true iff this function
  * is being called due to reading, as opposed to writing, a ChangeCipherSpec
@@ -566,10 +482,8 @@ tls1_change_cipher_state(SSL *s, int which)
                         goto err;
         }
 
-        if (aead != NULL) {
-                return tls1_change_cipher_state_aead(s, is_read, key, key_len,
-                    iv, iv_len);
-        }
+        if (aead != NULL)
+                return 1;
 
         return tls1_change_cipher_state_cipher(s, is_read,
             mac_secret, mac_secret_size, key, key_len, iv, iv_len);
@@ -617,6 +531,8 @@ tls1_setup_key_block(SSL *s)
         S3I(s)->tmp.new_mac_pkey_type = mac_type;
         S3I(s)->tmp.new_mac_secret_size = mac_secret_size;
 
+        tls12_record_layer_set_aead(s->internal->rl, aead);
+
         tls1_cleanup_key_block(s);
 
         if ((key_block = reallocarray(NULL, mac_secret_size + key_len + iv_len,
diff --git a/src/lib/libssl/tls12_record_layer.c b/src/lib/libssl/tls12_record_layer.c
index 0104443286..b74a6588ef 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.15 2021/01/26 14:22:20 jsing Exp $ */
+/* $OpenBSD: tls12_record_layer.c,v 1.16 2021/01/28 17:00:39 jsing Exp $ */
 /*
  * Copyright (c) 2020 Joel Sing <jsing@openbsd.org>
  *
@@ -15,6 +15,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <limits.h>
 #include <stdlib.h>
 
 #include <openssl/evp.h>
@@ -25,6 +26,8 @@ struct tls12_record_protection {
         uint16_t epoch;
         uint8_t seq_num[SSL3_SEQUENCE_SIZE];
 
+        SSL_AEAD_CTX *aead_ctx;
+
         int stream_mac;
 
         uint8_t *mac_key;
@@ -34,8 +37,6 @@ struct tls12_record_protection {
          * XXX - for now these are just pointers to externally managed
          * structs/memory. These should eventually be owned by the record layer.
          */
-        SSL_AEAD_CTX *aead_ctx;
-
         EVP_CIPHER_CTX *cipher_ctx;
         EVP_MD_CTX *hash_ctx;
 };
@@ -51,6 +52,12 @@ tls12_record_protection_clear(struct tls12_record_protection *rp)
 {
         memset(rp->seq_num, 0, sizeof(rp->seq_num));
 
+        if (rp->aead_ctx != NULL) {
+                EVP_AEAD_CTX_cleanup(&rp->aead_ctx->ctx);
+                freezero(rp->aead_ctx, sizeof(*rp->aead_ctx));
+                rp->aead_ctx = NULL;
+        }
+
         freezero(rp->mac_key, rp->mac_key_len);
         rp->mac_key = NULL;
         rp->mac_key_len = 0;
@@ -141,6 +148,8 @@ struct tls12_record_layer {
 
         uint8_t alert_desc;
 
+        const EVP_AEAD *aead;
+
         /* Pointers to active record protection (memory is not owned). */
         struct tls12_record_protection *read;
         struct tls12_record_protection *write;
@@ -232,6 +241,12 @@ tls12_record_layer_write_protected(struct tls12_record_layer *rl)
 }
 
 void
+tls12_record_layer_set_aead(struct tls12_record_layer *rl, const EVP_AEAD *aead)
+{
+        rl->aead = aead;
+}
+
+void
 tls12_record_layer_set_version(struct tls12_record_layer *rl, uint16_t version)
 {
         rl->version = version;
@@ -324,24 +339,6 @@ tls12_record_layer_reflect_seq_num(struct tls12_record_layer *rl)
 }
 
 int
-tls12_record_layer_set_read_aead(struct tls12_record_layer *rl,
-    SSL_AEAD_CTX *aead_ctx)
-{
-        tls12_record_layer_set_read_state(rl, aead_ctx, NULL, NULL, 0);
-
-        return 1;
-}
-
-int
-tls12_record_layer_set_write_aead(struct tls12_record_layer *rl,
-    SSL_AEAD_CTX *aead_ctx)
-{
-        tls12_record_layer_set_write_state(rl, aead_ctx, NULL, NULL, 0);
-
-        return 1;
-}
-
-int
 tls12_record_layer_set_read_cipher_hash(struct tls12_record_layer *rl,
     EVP_CIPHER_CTX *cipher_ctx, EVP_MD_CTX *hash_ctx, int stream_mac)
 {
@@ -381,6 +378,75 @@ tls12_record_layer_set_read_mac_key(struct tls12_record_layer *rl,
         return 1;
 }
 
+static int
+tls12_record_layer_ccs_aead(struct tls12_record_layer *rl,
+    struct tls12_record_protection *rp, int is_write, const uint8_t *mac_key,
+    size_t mac_key_len, const uint8_t *key, size_t key_len, const uint8_t *iv,
+    size_t iv_len)
+{
+        size_t aead_nonce_len = EVP_AEAD_nonce_length(rl->aead);
+
+        if ((rp->aead_ctx = calloc(1, sizeof(*rp->aead_ctx))) == NULL)
+                return 0;
+
+        /* AES GCM cipher suites use variable nonce in record. */
+        if (rl->aead == EVP_aead_aes_128_gcm() ||
+            rl->aead == EVP_aead_aes_256_gcm())
+                rp->aead_ctx->variable_nonce_in_record = 1;
+
+        /* ChaCha20 Poly1305 XORs the fixed and variable nonces. */
+        if (rl->aead == EVP_aead_chacha20_poly1305())
+                rp->aead_ctx->xor_fixed_nonce = 1;
+
+        if (iv_len > sizeof(rp->aead_ctx->fixed_nonce))
+                return 0;
+
+        memcpy(rp->aead_ctx->fixed_nonce, iv, iv_len);
+        rp->aead_ctx->fixed_nonce_len = iv_len;
+        rp->aead_ctx->tag_len = EVP_AEAD_max_overhead(rl->aead);
+        rp->aead_ctx->variable_nonce_len = 8;
+
+        if (rp->aead_ctx->xor_fixed_nonce) {
+                /* Fixed nonce length must match, variable must not exceed. */
+                if (rp->aead_ctx->fixed_nonce_len != aead_nonce_len)
+                        return 0;
+                if (rp->aead_ctx->variable_nonce_len > aead_nonce_len)
+                        return 0;
+        } else {
+                /* Concatenated nonce length must equal AEAD nonce length. */
+                if (rp->aead_ctx->fixed_nonce_len +
+                    rp->aead_ctx->variable_nonce_len != aead_nonce_len)
+                        return 0;
+        }
+
+        if (!EVP_AEAD_CTX_init(&rp->aead_ctx->ctx, rl->aead, key, key_len,
+            EVP_AEAD_DEFAULT_TAG_LENGTH, NULL))
+                return 0;
+
+        return 1;
+}
+
+static int
+tls12_record_layer_change_cipher_state(struct tls12_record_layer *rl,
+    struct tls12_record_protection *rp, int is_write, const uint8_t *mac_key,
+    size_t mac_key_len, const uint8_t *key, size_t key_len, const uint8_t *iv,
+    size_t iv_len)
+{
+        /* Require unused record protection. */
+        if (rp->cipher_ctx != NULL || rp->aead_ctx != NULL)
+                return 0;
+
+        if (mac_key_len > INT_MAX || key_len > INT_MAX || iv_len > INT_MAX)
+                return 0;
+
+        /* XXX - only aead for now. */
+        if (rl->aead == NULL)
+                return 1;
+
+        return tls12_record_layer_ccs_aead(rl, rp, is_write, mac_key,
+            mac_key_len, key, key_len, iv, iv_len);
+}
+
 int
 tls12_record_layer_change_read_cipher_state(struct tls12_record_layer *rl,
     const uint8_t *mac_key, size_t mac_key_len, const uint8_t *key,
@@ -394,7 +460,9 @@ tls12_record_layer_change_read_cipher_state(struct tls12_record_layer *rl,
 
         /* Read sequence number gets reset to zero. */
 
-        /* XXX - change cipher state. */
+        if (!tls12_record_layer_change_cipher_state(rl, read_new, 0,
+            mac_key, mac_key_len, key, key_len, iv, iv_len))
+                goto err;
 
         tls12_record_protection_free(rl->read_current);
         rl->read = rl->read_current = read_new;
@@ -421,7 +489,9 @@ tls12_record_layer_change_write_cipher_state(struct tls12_record_layer *rl,
 
         /* Write sequence number gets reset to zero. */
 
-        /* XXX - change cipher state. */
+        if (!tls12_record_layer_change_cipher_state(rl, write_new, 1,
+            mac_key, mac_key_len, key, key_len, iv, iv_len))
+                goto err;
 
         if (rl->dtls) {
                 tls12_record_protection_free(rl->write_previous);