Page MenuHomeFreeBSD

Add support for AES algorithms to IPSec
AbandonedPublic

Authored by gnn on May 16 2015, 3:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 3 2024, 11:52 AM
Unknown Object (File)
Dec 22 2023, 4:17 PM
Unknown Object (File)
Dec 20 2023, 10:47 AM
Unknown Object (File)
Oct 14 2023, 3:40 PM
Unknown Object (File)
Sep 30 2023, 12:10 AM
Unknown Object (File)
Aug 23 2023, 8:16 PM
Unknown Object (File)
Jul 4 2023, 3:34 AM
Unknown Object (File)
Jul 2 2023, 6:29 PM
Subscribers

Details

Reviewers
ae
eri
jmg
glebius
bz
Group Reviewers
network
manpages
Summary

Add support for AES algorithms to IPSec. While here clean up many of the magic numbers that plague this code.

Test Plan

Tested using the netperf project (https://github.com/gvnn3/netperf) using systems
from the Sentex cluster (lynx1 and rabbit3) both of which support AESNI. Tested AES-GCM,
AES-CTR, Rijndael, Blowfish and NULL algorithms.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

gnn retitled this revision from to Add support for AES algorithms to IPSec.
gnn updated this object.
gnn edited the test plan for this revision. (Show Details)
jmg requested changes to this revision.May 16 2015, 6:30 PM
jmg edited edge metadata.

The nonce change and some of the define changes I mentioned are required to be fixed before I'd approve this change.

sys/netipsec/xform_esp.c
252

This shouldn't include the + 4... I though we forced the klen of GMAC to be the same, but it looks like we don't generate an error.

The + 4 is probably more vestiges of OpenBSD's layering violations.

262

This does not look correct. AESGCM must not be using a random IV. Per 3.1 of RFC4106:
For a given key, the IV MUST NOT repeat. The most natural way to implement this is with a counter,

As there are only 64bits here, a random IV is really bad, because after a 600 million packets, there is a 1% chance that two of them collided, and this will allow an attacker to get the XOR difference between them. At 3.3 billion, it increases to 25% chance.

345

Can we do esph != NULL here?

357

can !espx be espx == NULL?

Is this check for GCM even necessary now that we've fixed tehe blocksize issue? or is the block size issue not fixed yet?

411

espx != NULL please

704

esph != NULL

sys/opencrypto/cryptodev.h
81

space should be a tab

107

more spaces after define should be tabs

117

I'd prefer to keep the RIJNDAEL128_BLOCK_LEN here, as there is no difference between the two.

118

This looks bogus. AES has a block length of 16 full stop. Only when AES is used in a different mode, such as CTR aka ICM can this be true.

126

This is also bogus. An IV length of 12 only applies to GCM. In almost all modes of AES, the IV length is 16.

Also, for GCM it is not an IV, but a nonce.

148

This is bad. We should not promote easily crackable ciphers. This should be a minimum of 12 or 16. I plan on removing some of the more questionable ciphers.

sys/opencrypto/xform.c
260

This is incorrect. None of our XTS implementations support using a block length of 1. They currently require a block size of 16.

This revision now requires changes to proceed.May 16 2015, 6:30 PM

I'll update the patch again this evening or tomorrow, but I'd be interested in comments on my feedback as well so I can get this all in at once. Thanks!

sys/netipsec/xform_esp.c
262

This happens once per session, not once per packet, so getting to 600 million might take a while. If a counter is what's necessary would the current time in nanoseconds be sufficient?

sys/opencrypto/cryptodev.h
118

If you take a look in xform.c you will see that the old magic number was 1. That's the blocksize according to the structure definition. What's the right name for this?

148

Again this is all based on the original magic numbers in xform.c

wblock added inline comments.
sbin/setkey/setkey.8
32

.Dd also needs to be updated when the contents change.

Did you test functionability of this AES-GCM implementation with other systems that already have implemented AES-GCM in IPSec? AFAIK, OpenBSD and Linux have it.

In D2566#48206, @ae wrote:

Did you test functionability of this AES-GCM implementation with other systems that already have implemented AES-GCM in IPSec? AFAIK, OpenBSD and Linux have it.

Yes, this has been tested with interoperability with linux, OpenBSD and even ASA.

This revision is too far removed from the number of fixes required to make this code acceptable to the HEAD of the tree. A newer patch set will be proposed in the coming days.

bz added a subscriber: bz.

PB unfortunately shows me no diffs; so I am inclined to say "reject". From what I read in the description this should be at least two separate things. Someone please poke me when PB is working again and I am happy to review.