Attention is currently required from: flichtenheld.
Hello flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/506?usp=email
to review the following change.
Change subject: Implement support for AEAD tag at the end
......................................................................
Implement support for AEAD tag at the end
Change-Id: I00821d75342daf3f813b829812d648fe298bea81
---
M src/openvpn/crypto.c
M src/openvpn/crypto.h
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/push.c
M src/openvpn/ssl.h
M tests/unit_tests/openvpn/test_ssl.c
7 files changed, 85 insertions(+), 26 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/06/506/1
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 2fca131..9988ebe 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -104,14 +104,10 @@
ASSERT(cipher_ctx_reset(ctx->cipher, iv));
}
- /* Reserve space for authentication tag */
- mac_out = buf_write_alloc(&work, mac_len);
- ASSERT(mac_out);
-
dmsg(D_PACKET_CONTENT, "ENCRYPT FROM: %s", format_hex(BPTR(buf),
BLEN(buf), 80, &gc));
/* Buffer overflow check */
- if (!buf_safe(&work, buf->len + cipher_ctx_block_size(ctx->cipher)))
+ if (!buf_safe(&work, buf->len + mac_len +
cipher_ctx_block_size(ctx->cipher)))
{
msg(D_CRYPT_ERRORS,
"ENCRYPT: buffer size error, bc=%d bo=%d bl=%d wc=%d wo=%d wl=%d",
@@ -121,9 +117,16 @@
}
/* For AEAD ciphers, authenticate Additional Data, including opcode */
- ASSERT(cipher_ctx_update_ad(ctx->cipher, BPTR(&work), BLEN(&work) -
mac_len));
+ ASSERT(cipher_ctx_update_ad(ctx->cipher, BPTR(&work), BLEN(&work)));
dmsg(D_PACKET_CONTENT, "ENCRYPT AD: %s",
- format_hex(BPTR(&work), BLEN(&work) - mac_len, 0, &gc));
+ format_hex(BPTR(&work), BLEN(&work), 0, &gc));
+
+ if (!(opt->flags & CO_AEAD_TAG_AT_THE_END))
+ {
+ /* Reserve space for authentication tag */
+ mac_out = buf_write_alloc(&work, mac_len);
+ ASSERT(mac_out);
+ }
/* Encrypt packet ID, payload */
ASSERT(cipher_ctx_update(ctx->cipher, BEND(&work), &outlen, BPTR(buf),
BLEN(buf)));
@@ -133,6 +136,14 @@
ASSERT(cipher_ctx_final(ctx->cipher, BEND(&work), &outlen));
ASSERT(buf_inc_len(&work, outlen));
+ /* if the tag is at end the end, allocate it now */
+ if (opt->flags & CO_AEAD_TAG_AT_THE_END)
+ {
+ /* Reserve space for authentication tag */
+ mac_out = buf_write_alloc(&work, mac_len);
+ ASSERT(mac_out);
+ }
+
/* Write authentication tag */
ASSERT(cipher_ctx_get_tag(ctx->cipher, mac_out, mac_len));
@@ -353,7 +364,6 @@
static const char error_prefix[] = "AEAD Decrypt error";
struct packet_id_net pin = { 0 };
const struct key_ctx *ctx = &opt->key_ctx_bi.decrypt;
- uint8_t *tag_ptr = NULL;
int outlen;
struct gc_arena gc;
@@ -406,19 +416,29 @@
/* keep the tag value to feed in later */
const int tag_size = OPENVPN_AEAD_TAG_LENGTH;
- if (buf->len < tag_size)
+ if (buf->len < tag_size + 1)
{
- CRYPT_ERROR("missing tag");
+ CRYPT_ERROR("missing tag or no payload");
}
- tag_ptr = BPTR(buf);
- ASSERT(buf_advance(buf, tag_size));
+
+ const int ad_size = BPTR(buf) - ad_start;
+
+ uint8_t *tag_ptr = NULL;
+ int data_len = 0;
+
+ if (opt->flags & CO_AEAD_TAG_AT_THE_END)
+ {
+ data_len = BLEN(buf) - tag_size;
+ tag_ptr = BPTR(buf) + data_len;
+ }
+ else
+ {
+ tag_ptr = BPTR(buf);
+ ASSERT(buf_advance(buf, tag_size));
+ data_len = BLEN(buf);
+ }
+
dmsg(D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex(tag_ptr, tag_size, 0,
&gc));
-
- if (buf->len < 1)
- {
- CRYPT_ERROR("missing payload");
- }
-
dmsg(D_PACKET_CONTENT, "DECRYPT FROM: %s", format_hex(BPTR(buf),
BLEN(buf), 0, &gc));
/* Buffer overflow check (should never fail) */
@@ -427,20 +447,19 @@
CRYPT_ERROR("potential buffer overflow");
}
- {
- /* feed in tag and the authenticated data */
- const int ad_size = BPTR(buf) - ad_start - tag_size;
- ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, ad_size));
- dmsg(D_PACKET_CONTENT, "DECRYPT AD: %s",
- format_hex(BPTR(buf) - ad_size - tag_size, ad_size, 0, &gc));
- }
+
+ /* feed in tag and the authenticated data */
+ ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, ad_size));
+ dmsg(D_PACKET_CONTENT, "DECRYPT AD: %s",
+ format_hex(ad_start, ad_size, 0, &gc));
/* Decrypt and authenticate packet */
if (!cipher_ctx_update(ctx->cipher, BPTR(&work), &outlen, BPTR(buf),
- BLEN(buf)))
+ data_len))
{
CRYPT_ERROR("cipher update failed");
}
+
ASSERT(buf_inc_len(&work, outlen));
if (!cipher_ctx_final_check_tag(ctx->cipher, BPTR(&work) + outlen,
&outlen, tag_ptr, tag_size))
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 4201524..95a5b31 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -279,6 +279,10 @@
/**< Bit-flag indicating that renegotiations are using tls-crypt
* with a TLS-EKM derived key.
*/
+#define CO_AEAD_TAG_AT_THE_END (1<<8)
+ /**< Bit-flag indicating that the AEAD tag is at the end of the
+ * packet.
+ */
unsigned int flags; /**< Bit-flags determining behavior of
* security operation functions. */
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c5cc154..cd37b36 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2323,6 +2323,10 @@
{
buf_printf(&out, " dyn-tls-crypt");
}
+ if (o->imported_protocol_flags & CO_AEAD_TAG_AT_THE_END)
+ {
+ buf_printf(&out, " aead-tag-end");
+ }
}
if (buf_len(&out) > strlen(header))
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 2c79a1e..39f00c0 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8686,6 +8686,10 @@
options->imported_protocol_flags |= CO_USE_DYNAMIC_TLS_CRYPT;
}
#endif
+ else if (streq(p[j], "aead-tag-end"))
+ {
+ options->imported_protocol_flags |= CO_AEAD_TAG_AT_THE_END;
+ }
else
{
msg(msglevel, "Unknown protocol-flags flag: %s", p[j]);
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 2249434..e4c122c 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -29,6 +29,7 @@
#include "push.h"
#include "options.h"
+#include "crypto.h"
#include "ssl.h"
#include "ssl_verify.h"
#include "ssl_ncp.h"
@@ -686,6 +687,11 @@
buf_printf(&proto_flags, " dyn-tls-crypt");
}
+ if (o->imported_protocol_flags & CO_AEAD_TAG_AT_THE_END)
+ {
+ buf_printf(&proto_flags, " aead-tag-end");
+ }
+
if (buf_len(&proto_flags) > 0)
{
push_option_fmt(gc, push_list, M_USAGE, "protocol-flags%s",
buf_str(&proto_flags));
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 71b99db..a1c67a2 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -107,6 +107,9 @@
/** Support to dynamic tls-crypt (renegotiation with TLS-EKM derived tls-crypt
key) */
#define IV_PROTO_DYN_TLS_CRYPT (1<<9)
+/** Suport for the AEAD tag at the end a larger peer id and IV */
+#define IV_PROTO_DATA_V3 (1<<10)
+
/* Default field in X509 to be username */
#define X509_USERNAME_FIELD_DEFAULT "CN"
diff --git a/tests/unit_tests/openvpn/test_ssl.c
b/tests/unit_tests/openvpn/test_ssl.c
index 8c1fb5b..0ded052 100644
--- a/tests/unit_tests/openvpn/test_ssl.c
+++ b/tests/unit_tests/openvpn/test_ssl.c
@@ -266,6 +266,19 @@
}
+/* This adds a few more methods that strictly necessary but this allows
+ * us to see which exact test was run from the backtrace of the test
+ * when it fails */
+
+static void
+run_data_channel_with_cipher_end(const char *cipher)
+{
+ struct crypto_options co = init_crypto_options(cipher, "none");
+ co.flags |= CO_AEAD_TAG_AT_THE_END;
+ do_data_channel_round_trip(&co);
+ uninit_crypto_options(&co);
+}
+
static void
run_data_channel_with_cipher(const char *cipher, const char *auth)
{
@@ -274,21 +287,25 @@
uninit_crypto_options(&co);
}
+
static void
test_data_channel_roundtrip_aes_128_gcm(void **state)
{
+ run_data_channel_with_cipher_end("AES-128-GCM");
run_data_channel_with_cipher("AES-128-GCM", "none");
}
static void
test_data_channel_roundtrip_aes_192_gcm(void **state)
{
+ run_data_channel_with_cipher_end("AES-192-GCM");
run_data_channel_with_cipher("AES-192-GCM", "none");
}
static void
test_data_channel_roundtrip_aes_256_gcm(void **state)
{
+ run_data_channel_with_cipher_end("AES-256-GCM");
run_data_channel_with_cipher("AES-256-GCM", "none");
}
@@ -318,6 +335,8 @@
skip();
return;
}
+
+ run_data_channel_with_cipher_end("ChaCha20-Poly1305");
run_data_channel_with_cipher("ChaCha20-Poly1305", "none");
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/506?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: I00821d75342daf3f813b829812d648fe298bea81
Gerrit-Change-Number: 506
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
