Overall the code looks good and works well in my tests. A few remarks
(but ACK otherwise):
> AC_ARG_ENABLE(
> [aead-modes],
> [AS_HELP_STRING([--disable-aead-modes], [disable AEAD crypto modes
> @<:@default=yes@:>@])],
> ,
> [enable_aead_modes="yes"]
> )
I would not make this user configurable. If it wasn't for RHEL5 I even
would throw out 0.9.8 support and always compile it in.
{
+ /* Get the key we will use to encrypt the packet. */
tls_pre_encrypt (c->c2.tls_multi, &c->c2.buf, &co);
+ /* If using P_DATA_V2, prepend the 1-byte opcode and 3-byte peer-id to
the
+ * packet before openvpn_encrypt(), so we can authenticate the opcode
too.
+ */
+ if (c->c2.buf.len > 0 && !c->c2.tls_multi->opt.server &&
c->c2.tls_multi->use_peer_id)
The latter part of that if might be turned into a small inline fuction
to make it clearer. Or alternatively a local boolean variable to be used
in both if places.
> /* Prepare IV */
> {
> struct buffer iv_buffer;
> struct packet_id_net pin;
> const int iv_len = cipher_ctx_iv_length (ctx->cipher);
>
> ASSERT (iv_len >= 12 && iv_len <= OPENVPN_MAX_IV_LENGTH);
A comment about the 12 would be nice here. (=> 96 bits ....)
> /* set_tag() call required for older OpenSSL versions only */
What does older mean in this context? Add an idef or atleast specify the
version in the file.
> const int ad_size = BPTR (buf) - ad_start - tag_size;
ad_size can be 0. Did you check what hte PolarSSL/OpenSSL function do on
a 0 byte call?
Also add for the manpage:
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 6b97fc9..30faff3 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -3991,6 +3991,10 @@ Set
.B alg=none
to disable authentication.
+If a AEAD
+.B \-\-cipher
+is selected like AES-128-GCM is used the authentication of that cipher
is used for the data channel packets.
+
For more information on HMAC see
.I http://www.cs.ucsd.edu/users/mihir/papers/hmac.html
.\"*********************************************************
Arne
