> -----Original Message----- > From: Kenny Root [mailto:ke...@the-b.org] > Sent: dinsdag 4 juni 2013 2:15 > To: openvpn-devel@lists.sourceforge.net > Subject: [Openvpn-devel] Adding support for AEAD cipher modes > (AES-GCM, et al.) > > I wrote a patch to add AEAD cipher modes to OpenVPN. This is one of > NIST's recommended modes and newer Intel chips supporting the AES-NI > (with PCLMULQDQ) have excellent support for acceleration in this mode. > I posted the initial version at > https://community.openvpn.net/openvpn/ticket/301 and that ticket has > some of the performance numbers on an Ivy Bridge class chip. > > It also doesn't appear PolarSSL has added AES-GCM to their main crypto > API yet. > > (You can also comment at Github at > https://github.com/kruton/openvpn/commit/a8c6701c8b47429c32da5c01f4398 > f > 9e0b293c01)
Cool to see support for GCM in the data channel!
PolarSSL does support GCM (since 1.2, see include/polarssl/gcm.h), and
indeed OpenVPN-NL exclusively uses a GCM SSL ciphersuite in the control
channel. However, it would be very neat to also have it in the data channel,
so I looked at your patch with great interest. It's really neat, but I do
have some comments.
I don't really see the benefit of adding CCM support; I'm not aware of any
new protocol using it, since GCM is generally faster/nicer/more common, and
- on recent Intel chips - benefits from hardware support (GCLMULQDQ). That
said, it's not hard, so...
XTS seems rather unsuited for this application; it's an unauthenticated
tweakable block cipher used for disk encryption. The fact that it uses the
same API as GCM/CCM is yet another instance of OpenSSL perfectly aligning a
gun with your foot.
The prng_bytes() on src/openvpn/crypto.c:144 seems odd in various ways. I'll
continue reviewing tonight and tomorrow morning, and I'll take a good look
at this chunk of code then. (Sorry for the slow review; life and spotty
internet in the train got in the way.)
I don't understand src/openvpn/crypto.c:194 either, but I'll take another
look tonight.
Some further minor comments:
src/openvpn/crypto.c:311: MAX_HMAC_KEY_LENGTH is a rather strange name for
the length of a variable holding the authentication tag.
src/openvpn/crypto.c 322: whitespace error
src/openvpn/crypto.c 430: #ifndef ALLOW_NON_CBC_CIPHERS, we still allow
AEAD? That seems odd.
Src/openvpn/crypto_backend.h:271: "message authentication code". In general,
neither the function name nor the doxygen makes it obvious that this
function is valid only for AEAD modes.
Again, thanks for the patch! I'm no contributor, but I do think it's really
cool.
Joachim
smime.p7s
Description: S/MIME cryptographic signature
