Attention is currently required from: plaisthos.
Hello flichtenheld,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/506?usp=email
to look at the new patch set (#4).
The change is no longer submittable: checks~ChecksSubmitRule is unsatisfied now.
Change subject: Implement support for AEAD tag at the end
......................................................................
Implement support for AEAD tag at the end
Using the AEAD tag at the end is the standard way of doing AEAD. Several
APIs even only support the tag at the end (e.g. mbed TLS). Having the tag at
the front or end makes no difference for security but allows streaming HW
implementations like NICs to be much more efficient as they do not need to
buffer a whole packet content and encrypt it to finally write the tag but
instead just add the calculated tag at the end of processing.
Change-Id: I00821d75342daf3f813b829812d648fe298bea81
Signed-off-by: Arne Schwabe <a...@rfc2549.org>
---
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 tests/unit_tests/openvpn/test_ssl.c
6 files changed, 80 insertions(+), 26 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/06/506/4
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 5d05cc4..9758c8c 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 85a0bfe..61184bc 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 02205e7..c56ce48 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2312,6 +2312,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 768332d..cb8ff88 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8683,6 +8683,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 1b406b9..10806ac 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/tests/unit_tests/openvpn/test_ssl.c
b/tests/unit_tests/openvpn/test_ssl.c
index a9a3137..8d73858 100644
--- a/tests/unit_tests/openvpn/test_ssl.c
+++ b/tests/unit_tests/openvpn/test_ssl.c
@@ -265,6 +265,17 @@
}
+/* This adds a few more methods than 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 +285,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 +333,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: 4
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
