Skip to content

Commit

Permalink
Move initialisation of implicit IVs to init_key_ctx_bi methods
Browse files Browse the repository at this point in the history
This is really more a function of initialising the data cipher and key
context and putting it into the init_key_ctx_bi makes more sense.

It will allow calling init_key_ctx_bi to fully initialise a
data channel key without calling some extra functions after that
which will make the (upcoming) epoch key implementation cleaner.

Also ensure that free_ctx_bi actually also sets initialized to false.

Change-Id: Id223612c7bcab91d49c013fb775024bd64ab0836
Signed-off-by: Arne Schwabe <[email protected]>
Acked-by: Gert Doering <[email protected]>
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/[email protected]/msg30170.html
Signed-off-by: Gert Doering <[email protected]>
  • Loading branch information
schwabe authored and cron2 committed Dec 23, 2024
1 parent 435cc9a commit f0c26b0
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 59 deletions.
32 changes: 30 additions & 2 deletions src/openvpn/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,33 @@ init_key_type(struct key_type *kt, const char *ciphername,
}
}

/**
* Update the implicit IV for a key_ctx_bi based on TLS session ids and cipher
* used.
*
* Note that the implicit IV is based on the HMAC key, but only in AEAD modes
* where the HMAC key is not used for an actual HMAC.
*
* @param ctx Encrypt/decrypt key context
* @param key key, hmac part used to calculate implicit IV
*/
static void
key_ctx_update_implicit_iv(struct key_ctx *ctx, const struct key *key)
{
/* Only use implicit IV in AEAD cipher mode, where HMAC key is not used */
if (cipher_ctx_mode_aead(ctx->cipher))
{
size_t impl_iv_len = 0;
ASSERT(cipher_ctx_iv_length(ctx->cipher) >= OPENVPN_AEAD_MIN_IV_LEN);
impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - sizeof(packet_id_type);
ASSERT(impl_iv_len + sizeof(packet_id_type) <= OPENVPN_MAX_IV_LENGTH);
ASSERT(impl_iv_len <= MAX_HMAC_KEY_LENGTH);
CLEAR(ctx->implicit_iv);
/* The first bytes of the IV are filled with the packet id */
memcpy(ctx->implicit_iv + sizeof(packet_id_type), key->hmac, impl_iv_len);
}
}

/* given a key and key_type, build a key_ctx */
void
init_key_ctx(struct key_ctx *ctx, const struct key *key,
Expand Down Expand Up @@ -949,7 +976,7 @@ init_key_bi_ctx_send(struct key_ctx *ctx, const struct key2 *key2,
snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name);
init_key_ctx(ctx, &key2->keys[kds.out_key], kt,
OPENVPN_OP_ENCRYPT, log_prefix);

key_ctx_update_implicit_iv(ctx, &key2->keys[kds.out_key]);
}

void
Expand All @@ -964,7 +991,7 @@ init_key_bi_ctx_recv(struct key_ctx *ctx, const struct key2 *key2,
snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name);
init_key_ctx(ctx, &key2->keys[kds.in_key], kt,
OPENVPN_OP_DECRYPT, log_prefix);

key_ctx_update_implicit_iv(ctx, &key2->keys[kds.in_key]);
}

void
Expand Down Expand Up @@ -999,6 +1026,7 @@ free_key_ctx_bi(struct key_ctx_bi *ctx)
{
free_key_ctx(&ctx->encrypt);
free_key_ctx(&ctx->decrypt);
ctx->initialized = false;
}

static bool
Expand Down
38 changes: 0 additions & 38 deletions src/openvpn/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,21 +95,6 @@ show_tls_performance_stats(void)

#endif /* ifdef MEASURE_TLS_HANDSHAKE_STATS */

/**
* Update the implicit IV for a key_ctx_bi based on TLS session ids and cipher
* used.
*
* Note that the implicit IV is based on the HMAC key, but only in AEAD modes
* where the HMAC key is not used for an actual HMAC.
*
* @param ctx Encrypt/decrypt key context
* @param key HMAC key, used to calculate implicit IV
* @param key_len HMAC key length
*/
static void
key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len);


/**
* Limit the reneg_bytes value when using a small-block (<128 bytes) cipher.
*
Expand Down Expand Up @@ -1411,12 +1396,6 @@ init_key_contexts(struct key_state *ks,
else
{
init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel");
/* Initialize implicit IVs */
key_ctx_update_implicit_iv(&key->encrypt, key2->keys[(int)server].hmac,
MAX_HMAC_KEY_LENGTH);
key_ctx_update_implicit_iv(&key->decrypt,
key2->keys[1 - (int)server].hmac,
MAX_HMAC_KEY_LENGTH);
}
}

Expand Down Expand Up @@ -1553,23 +1532,6 @@ generate_key_expansion(struct tls_multi *multi, struct key_state *ks,
return ret;
}

static void
key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len)
{
/* Only use implicit IV in AEAD cipher mode, where HMAC key is not used */
if (cipher_ctx_mode_aead(ctx->cipher))
{
size_t impl_iv_len = 0;
ASSERT(cipher_ctx_iv_length(ctx->cipher) >= OPENVPN_AEAD_MIN_IV_LEN);
impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - sizeof(packet_id_type);
ASSERT(impl_iv_len + sizeof(packet_id_type) <= OPENVPN_MAX_IV_LENGTH);
ASSERT(impl_iv_len <= key_len);
CLEAR(ctx->implicit_iv);
/* The first bytes of the IV are filled with the packet id */
memcpy(ctx->implicit_iv + sizeof(packet_id_type), key, impl_iv_len);
}
}

/**
* Generate data channel keys for the supplied TLS session.
*
Expand Down
19 changes: 0 additions & 19 deletions tests/unit_tests/openvpn/test_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -277,24 +277,6 @@ test_load_certificate_and_key_uri(void **state)
#endif /* HAVE_OPENSSL_STORE */
}

static void
init_implicit_iv(struct crypto_options *co)
{
cipher_ctx_t *cipher = co->key_ctx_bi.encrypt.cipher;

if (cipher_ctx_mode_aead(cipher))
{
ASSERT(cipher_ctx_iv_length(cipher) <= OPENVPN_MAX_IV_LENGTH);
ASSERT(cipher_ctx_iv_length(cipher) >= OPENVPN_AEAD_MIN_IV_LEN);

/* Generate dummy implicit IV */
ASSERT(rand_bytes(co->key_ctx_bi.encrypt.implicit_iv,
OPENVPN_MAX_IV_LENGTH));

memcpy(co->key_ctx_bi.decrypt.implicit_iv,
co->key_ctx_bi.encrypt.implicit_iv, OPENVPN_MAX_IV_LENGTH);
}
}

static void
init_frame_parameters(struct frame *frame)
Expand Down Expand Up @@ -346,7 +328,6 @@ do_data_channel_round_trip(struct crypto_options *co)
/* init work */
ASSERT(buf_init(&work, frame.buf.headroom));

init_implicit_iv(co);
update_time();

/* Test encryption, decryption for all packet sizes */
Expand Down

0 comments on commit f0c26b0

Please sign in to comment.