Attention is currently required from: flichtenheld, plaisthos.
Hello flichtenheld,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/801?usp=email
to look at the new patch set (#9).
The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld
Change subject: Change API of init_key_ctx to use struct key_parameters
......................................................................
Change API of init_key_ctx to use struct key_parameters
This introduces a new structure key_parameters. The reason is that the
current struct serves both as an internal struct as well as an
on-wire/in-file format. Separate these two different usages to allow
extending the struct.
Change-Id: I4a981c5a70717e2276d89bf83a06c7fdbe6712d7
Signed-off-by: Arne Schwabe <[email protected]>
---
M src/openvpn/auth_token.c
M src/openvpn/crypto.c
M src/openvpn/crypto.h
M src/openvpn/tls_crypt.c
M tests/unit_tests/openvpn/test_auth_token.c
M tests/unit_tests/openvpn/test_tls_crypt.c
6 files changed, 94 insertions(+), 41 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/01/801/9
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index 192c7c2..3cf55e8 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -152,7 +152,10 @@
{
msg(M_FATAL, "ERROR: not enough data in auth-token secret");
}
- init_key_ctx(key_ctx, &key, &kt, false, "auth-token secret");
+
+ struct key_parameters key_params;
+ key_parameters_from_key(&key_params, &key);
+ init_key_ctx(key_ctx, &key_params, &kt, false, "auth-token secret");
free_buf(&server_secret_key);
}
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index da503b3..df38cdd 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -892,17 +892,18 @@
}
/**
- * Update the implicit IV for a key_ctx_bi based on TLS session ids and cipher
+ * Update the implicit IV for a key_ctx 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.
+ * Note that the implicit IV is based on the HMAC key of the \c key parameter,
+ * 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
+ * @param key key parameters holding the key and hmac/
+ * implicit iv used to calculate implicit IV
*/
static void
-key_ctx_update_implicit_iv(struct key_ctx *ctx, const struct key *key)
+key_ctx_update_implicit_iv(struct key_ctx *ctx, const struct key_parameters
*key)
{
/* Only use implicit IV in AEAD cipher mode, where HMAC key is not used */
if (cipher_ctx_mode_aead(ctx->cipher))
@@ -912,6 +913,7 @@
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);
+ ASSERT(impl_iv_len <= key->hmac_size);
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);
@@ -920,7 +922,7 @@
/* given a key and key_type, build a key_ctx */
void
-init_key_ctx(struct key_ctx *ctx, const struct key *key,
+init_key_ctx(struct key_ctx *ctx, const struct key_parameters *key,
const struct key_type *kt, int enc,
const char *prefix)
{
@@ -928,7 +930,7 @@
CLEAR(*ctx);
if (cipher_defined(kt->cipher))
{
-
+ ASSERT(key->cipher_size >= cipher_kt_key_size(kt->cipher));
ctx->cipher = cipher_ctx_new();
cipher_ctx_init(ctx->cipher, key->cipher, kt->cipher, enc);
@@ -943,8 +945,10 @@
cipher_kt_iv_size(kt->cipher));
warn_insecure_key_type(ciphername);
}
+
if (md_defined(kt->digest))
{
+ ASSERT(key->hmac_size >= md_kt_size(kt->digest));
ctx->hmac = hmac_ctx_new();
hmac_ctx_init(ctx->hmac, key->hmac, kt->digest);
@@ -965,41 +969,43 @@
}
void
-init_key_bi_ctx_send(struct key_ctx *ctx, const struct key2 *key2,
- int key_direction, const struct key_type *kt, const char
*name)
+init_key_bi_ctx_send(struct key_ctx *ctx, const struct key_parameters
*key_params,
+ const struct key_type *kt, const char *name)
{
char log_prefix[128] = { 0 };
- struct key_direction_state kds;
-
- key_direction_state_init(&kds, key_direction);
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]);
+ init_key_ctx(ctx, key_params, kt, OPENVPN_OP_ENCRYPT, log_prefix);
+ key_ctx_update_implicit_iv(ctx, key_params);
}
void
-init_key_bi_ctx_recv(struct key_ctx *ctx, const struct key2 *key2,
- int key_direction, const struct key_type *kt, const char
*name)
+init_key_bi_ctx_recv(struct key_ctx *ctx, const struct key_parameters
*key_params,
+ const struct key_type *kt, const char *name)
{
char log_prefix[128] = { 0 };
- struct key_direction_state kds;
-
- key_direction_state_init(&kds, key_direction);
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]);
+ init_key_ctx(ctx, key_params, kt, OPENVPN_OP_DECRYPT, log_prefix);
+ key_ctx_update_implicit_iv(ctx, key_params);
}
void
init_key_ctx_bi(struct key_ctx_bi *ctx, const struct key2 *key2,
int key_direction, const struct key_type *kt, const char *name)
{
- init_key_bi_ctx_send(&ctx->encrypt, key2, key_direction, kt, name);
- init_key_bi_ctx_recv(&ctx->decrypt, key2, key_direction, kt, name);
+ struct key_direction_state kds;
+
+ key_direction_state_init(&kds, key_direction);
+
+ struct key_parameters send_key;
+ struct key_parameters recv_key;
+
+ key_parameters_from_key(&send_key, &key2->keys[kds.out_key]);
+ key_parameters_from_key(&recv_key, &key2->keys[kds.in_key]);
+
+ init_key_bi_ctx_send(&ctx->encrypt, &send_key, kt, name);
+ init_key_bi_ctx_recv(&ctx->decrypt, &recv_key, kt, name);
ctx->initialized = true;
}
@@ -1116,6 +1122,16 @@
}
void
+key_parameters_from_key(struct key_parameters *key_params, const struct key
*key)
+{
+ CLEAR(*key_params);
+ memcpy(key_params->cipher, key->cipher, MAX_CIPHER_KEY_LENGTH);
+ key_params->cipher_size = MAX_CIPHER_KEY_LENGTH;
+ memcpy(key_params->hmac, key->hmac, MAX_HMAC_KEY_LENGTH);
+ key_params->hmac_size = MAX_HMAC_KEY_LENGTH;
+}
+
+void
test_crypto(struct crypto_options *co, struct frame *frame)
{
int i, j;
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index b3612fb..d765737 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -144,7 +144,8 @@
/**
* Container for unidirectional cipher and HMAC %key material.
- * @ingroup control_processor
+ * @ingroup control_processor. This is used as a wire format/file format
+ * key, so it cannot be changed to add fields or change the length of fields
*/
struct key
{
@@ -154,6 +155,32 @@
/**< %Key material for HMAC operations. */
};
+/** internal structure similar to struct key that holds key information
+ * but is not represented on wire and can be changed/extended
+ */
+struct key_parameters {
+ /** %Key material for cipher operations. */
+ uint8_t cipher[MAX_CIPHER_KEY_LENGTH];
+
+ /** Number of bytes set in the cipher key material */
+ int cipher_size;
+
+ /** %Key material for HMAC operations. */
+ uint8_t hmac[MAX_HMAC_KEY_LENGTH];
+
+ /** Number of bytes set in the HMac key material */
+ int hmac_size;
+};
+
+/**
+ * Converts a struct key representation into a struct key_parameters
+ * representation.
+ *
+ * @param key_params destination for the converted struct
+ * @param key source of the conversion
+ */
+void
+key_parameters_from_key(struct key_parameters *key_params, const struct key
*key);
/**
* Container for one set of cipher and/or HMAC contexts.
@@ -347,19 +374,17 @@
* Key context functions
*/
-void init_key_ctx(struct key_ctx *ctx, const struct key *key,
+void init_key_ctx(struct key_ctx *ctx, const struct key_parameters *key,
const struct key_type *kt, int enc,
const char *prefix);
void
-init_key_bi_ctx_send(struct key_ctx *ctx, const struct key2 *key2,
- int key_direction, const struct key_type *kt,
- const char *name);
+init_key_bi_ctx_send(struct key_ctx *ctx, const struct key_parameters *key,
+ const struct key_type *kt, const char *name);
void
-init_key_bi_ctx_recv(struct key_ctx *ctx, const struct key2 *key2,
- int key_direction, const struct key_type *kt,
- const char *name);
+init_key_bi_ctx_recv(struct key_ctx *ctx, const struct key_parameters *key,
+ const struct key_type *kt, const char *name);
void free_key_ctx(struct key_ctx *ctx);
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index b8894db..76f06bc 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -377,7 +377,11 @@
{
msg(M_FATAL, "ERROR: --tls-crypt-v2 not supported");
}
- init_key_ctx(key_ctx, &srv_key, &kt, encrypt, "tls-crypt-v2 server key");
+ struct key_parameters srv_key_params;
+
+ key_parameters_from_key(&srv_key_params, &srv_key);
+
+ init_key_ctx(key_ctx, &srv_key_params, &kt, encrypt, "tls-crypt-v2 server
key");
secure_memzero(&srv_key, sizeof(srv_key));
}
diff --git a/tests/unit_tests/openvpn/test_auth_token.c
b/tests/unit_tests/openvpn/test_auth_token.c
index 8a0c16a..be3b2b7 100644
--- a/tests/unit_tests/openvpn/test_auth_token.c
+++ b/tests/unit_tests/openvpn/test_auth_token.c
@@ -87,7 +87,8 @@
struct test_context *ctx = calloc(1, sizeof(*ctx));
*state = ctx;
- struct key key = { 0 };
+ struct key_parameters key = { 0 };
+ key.hmac_size = MAX_HMAC_KEY_LENGTH; /* 64 byte of 0 */
ctx->kt = auth_token_kt();
if (!ctx->kt.digest)
@@ -148,15 +149,17 @@
AUTH_TOKEN_HMAC_OK);
/* Change auth-token key */
- struct key key;
- memset(&key, '1', sizeof(key));
+ struct key_parameters key;
+ memset(key.hmac, '1', sizeof(key.hmac));
+ key.hmac_size = MAX_HMAC_KEY_LENGTH;
+
free_key_ctx(&ctx->multi.opt.auth_token_key);
init_key_ctx(&ctx->multi.opt.auth_token_key, &key, &ctx->kt, false,
"TEST");
assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
0);
/* Load original test key again */
- memset(&key, 0, sizeof(key));
+ memset(&key.hmac, 0, sizeof(key.hmac));
free_key_ctx(&ctx->multi.opt.auth_token_key);
init_key_ctx(&ctx->multi.opt.auth_token_key, &key, &ctx->kt, false,
"TEST");
assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c
b/tests/unit_tests/openvpn/test_tls_crypt.c
index 4f12f88..94cd0ee 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -157,7 +157,8 @@
struct test_tls_crypt_context *ctx = calloc(1, sizeof(*ctx));
*state = ctx;
- struct key key = { 0 };
+ struct key_parameters key = { .cipher = { 0 }, .hmac = { 0 },
+ .hmac_size = MAX_HMAC_KEY_LENGTH,
.cipher_size = MAX_CIPHER_KEY_LENGTH };
ctx->kt = tls_crypt_kt();
if (!ctx->kt.cipher || !ctx->kt.digest)
@@ -367,7 +368,8 @@
skip_if_tls_crypt_not_supported(ctx);
/* Change decrypt key */
- struct key key = { { 1 } };
+ struct key_parameters key = { .cipher = { 1 }, .hmac = { 1 },
+ .cipher_size = MAX_CIPHER_KEY_LENGTH,
.hmac_size = MAX_HMAC_KEY_LENGTH };
free_key_ctx(&ctx->co.key_ctx_bi.decrypt);
init_key_ctx(&ctx->co.key_ctx_bi.decrypt, &key, &ctx->kt, false, "TEST");
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/801?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4a981c5a70717e2276d89bf83a06c7fdbe6712d7
Gerrit-Change-Number: 801
Gerrit-PatchSet: 9
Gerrit-Owner: plaisthos <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
