Joachim Schipper wrote [I'm continuing my own message here]:
> > -----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.
> >
> > It also doesn't appear PolarSSL has added AES-GCM to their main crypto
> > API yet.
> >
>
> 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.

> 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.)

Continuing this train of thought, both GCM and CCM need non-repeating (but
possibly predictable) IVs. As long as net_time and net_id are unique to the
current packet, your code works fine; and you should still have 32 bits of
randomness left to guard against Bad Things happening to the clock. So this
code is just fine.

> I don't understand src/openvpn/crypto.c:194 either, but I'll take
> another look tonight.

After another reading, I understand just fine. Consider adding "else
ASSERT(outlen == 0)", but ok.

> Again, thanks for the patch! I'm no contributor, but I do think it's
> really cool.

Let me reiterate: thanks! I've just taken a quick look, but it looks quite
nice.

                Joachim

Reply via email to