Generate IVs directly in esp output.

[jhb]

Summary

This is the only place that uses CRYPTO_F_IV_GENERATE. All crypto
drivers currently duplicate the same boilerplate code to handle this
case. Doing the generation directly removes complexity from drivers.
It also simplifies support for separate input and output buffers.

Test Plan

• pinged across a connection using AES-CBC with a SHA HMAC (using a static setkey config)

[cem]

sys/netipsec/xform_esp.c

822–825 ↗

(On Diff #70657)

This comment seems inaccurate (the second half of ivp seems to be a counter), which is usually a good way to implement a nonce, if you can be sure the counter does not reset.

I know, it predates this change.

836–837 ↗

(On Diff #70657)

If we swapped these lines, the arithmetic in the m_copyback() offset parameter could be replaced with crp->crp_iv_start.

[jhb]

sys/netipsec/xform_esp.c

822–825 ↗

(On Diff #70657)

What it means is that the salt/nonce is started as the last 4 bytes of the encryption key (sav->key_enc->key_data) provided via setkey(8) or an IKE daemon rather than being stored as the last four bytes in crp_iv. It does seem like the cntr variable here isn't used?

[cem]

sys/netipsec/xform_esp.c

822–825 ↗

(On Diff #70657)

That's the "Salt is ..." comment and memcpy().

I'm talking about the "Nonce is ..." comment and be64enc(). Although now that I think about it, be64enc seems very wrong for a 4-byte value?

829 ↗

(On Diff #70657)

Here we (sometimes?) overwrite it with a 4-byte be32enc "1" value. The XXX comment is, uh, "very strong yes." Nonce reuse with other parameters fixed absolutely breaks CTR algorithms. Unless I'm really misunderstanding how this code is used (definite possibility, I have not investigated), this is a SA-type finding.

[jhb]

sys/netipsec/xform_esp.c

822–825 ↗

(On Diff #70657)

Both comments are about the same thing (the memcpy). The "salt" comment is for GCM (RFC 4106), the "nonce" comment is for AES-CTR (RFC 3686).

829 ↗

(On Diff #70657)

This is the non-GCM case when using AES-CTR. Note that AES-GCM always uses a counter value of '2' here for the start of each packet, and I read the cntr bit wrong. The way this works is that both AES-GCM and AES-CTR are using a 16-byte IV split into 3 fields:

--------------------------------------------------------------
| 4 byte salt | 8 byte packet counter | 4 byte block counter |
--------------------------------------------------------------

This 4 byte salt/nonce is the last 4 bytes of the encryption "key" copied as the first for bytes via the memcpy(). The 8 byte packet counter is cntr that is incremented for each packet (and the IKE daemon is supposed to generate a new key + salt before it rolls over). The last 4 bytes is a per-packet block counter. For AES-GCM this always starts at '2' as part of how AES-GCM works when using a 12-byte IV and a 4 byte counter. This comment is just about using '1' instead of '2' for that block counter for AES-CTR. Using the value '1' is mandated by section 4 of RFC 3686 as the first comment suggests, and it is per-packet, so the XXXAE comment really just needs to be removed. It has to be standardized and not random since this value isn't passed on the wire (only the 8 byte counter is on the wire) and both sides have to use the same 16-byte IV value.

[cem]

sys/netipsec/xform_esp.c

822–825 ↗

(On Diff #70657)

Ah, ok.

829 ↗

(On Diff #70657)

Ok, the design seems fine (the packet counter isn't reused without bumping the salt, I take it?) and the comment should be dropped (not necessarily in this revision, but if you wouldn't mind doing it as a driveby, I would appreciate it — you've already written the commit log ;-)).

[jhb]

• Reuse csp_digest_start.