[Swan-dev] COOKIE_SIZE is IKEv1!

[Andrew Cagney]

On Mon, 16 Jul 2018 at 13:52, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
>
> | From: Andrew Cagney <andrew.cagney at gmail.com>
>
> | But why use 'cookie' for something that isn't even a cookie!  Here's
> | the relevant section of the RFC:
> |
> | https://tools.ietf.org/html/rfc7296#section-2.6
>
> There are two relevant RFCs.  The other one calls them cookies.
>
> In v1, the IKE SPI is a bit confusing.  Look at RFC 2408 2.4.  But
> these "cookies" are not, each, IKEv1 SPIs.
>
> And 2408's use of the word cookie is questionable since it was
> copied (badly) from Photurus.

This is how KEv2 describes the situation:

   A note on terminology: the term "cookies" originates with Karn and
   Simpson [PHOTURIS] in Photuris, an early proposal for key management
   with IPsec, and it has persisted.  The Internet Security Association
   and Key Management Protocol (ISAKMP) [ISAKMP] fixed message header
   includes two eight-octet fields called "cookies", and that syntax is
   used by both IKEv1 and IKEv2, although in IKEv2 they are referred to
   as the "IKE SPI" and there is a new separate field in a Notify
   payload holding the cookie.

so, again, there should be no reason to refer to IKEv1.

> But that is the term of the standard to which the code was written.
>
> As Paul suggests, we could transition to the new terminology.
> But I still wonder when v1 code gets deleted.  If it is soon, do the
> renaming then.  If it isn't soon, why rename and disrupt the code.

Because it makes for needlessly confusing code.

Heck, the IKEv2 code, outside of ikev2_spdb_struct.c, refers to all of
icookie, rcookie and dcookie.   From my POV it was hard enough getting
those initial SPI exchanges and hashes right, and that's before I had
to fight the clearly bogus variable names.

> I have proposed that all IKEv2_ names become v2_ names and IKEv1-only
> names (currently with no prefix) become v1_names.  Nobody seemed
> interested.
>
> | So what we're sizing here isn't an IKEv2 cookie.
>
> Of it's not about v2 cookies.  Currently names with no qualification
> are V1 names (even if they apply only to v1).  So COOKIE_SIZE, by
> convention, is in v1 terms.  But it is relevant to v2 (as previously
> discussed).

This is important.  Anytime I encounter code using an esoteric IKEv1
name I try to first change things so that either an IKEv2 or neutral
term is used and then push IKEv1-ism into a corner.  So here,
minimally, an IKEv2 name, and preferably a rename of COOKIE_SIZE to
IKE_SA_SPI_SIZE.

Does that sound reasonable?

BTW:

$ grep IPSEC_DOI_SPI_SIZE programs/pluto/*
programs/pluto/ikev1_spdb_struct.c:
IPSEC_DOI_SPI_SIZE,
programs/pluto/ikev1_spdb_struct.c:
IPSEC_DOI_SPI_SIZE -
programs/pluto/ikev1_spdb_struct.c:
IPSEC_DOI_SPI_SIZE,
programs/pluto/ikev1_spdb_struct.c:                IPSEC_DOI_SPI_SIZE
- IPCOMP_CPI_SIZE,
programs/pluto/ikev1_spdb_struct.c:        passert(out_raw((u_char *)
&pi->our_spi, IPSEC_DOI_SPI_SIZE,
programs/pluto/ikev1_spdb_struct.c:                    IPSEC_DOI_SPI_SIZE) {
programs/pluto/ikev1_spdb_struct.c:                    u_int8_t
filler[IPSEC_DOI_SPI_SIZE -
programs/pluto/ikev1_spdb_struct.c:                        IPSEC_DOI_SPI_SIZE -
programs/pluto/ikev1_spdb_struct.c:                    IPSEC_DOI_SPI_SIZE) {
programs/pluto/ikev2_spdb_struct.c:    uint8_t
bytes[COOKIE_SIZE>IPSEC_DOI_SPI_SIZE ? COOKIE_SIZE :
IPSEC_DOI_SPI_SIZE];    /* space for largest SPI */
programs/pluto/ikev2_spdb_struct.c:        return IPSEC_DOI_SPI_SIZE;

$ grep 'sizeof(ipsec_spi_t)' programs/pluto/*.c
programs/pluto/ikev1_main.c:            isad.isad_spisize = sizeof(ipsec_spi_t);
programs/pluto/ikev1_main.c:            passert(out_raw(&ns->spi,
sizeof(ipsec_spi_t),
programs/pluto/ikev1_main.c:        sizespi = sizeof(ipsec_spi_t);
programs/pluto/ikev2_parent.c:    if (ntfy.isan_spisize !=
sizeof(ipsec_spi_t)) {
programs/pluto/ikev2_parent.c:            if (v2del->isad_spisize !=
sizeof(ipsec_spi_t)) {
programs/pluto/ikev2_send.c:            v2del_tmp.isad_spisize =
sizeof(ipsec_spi_t);
programs/pluto/ikev2_send.c:                     sizeof(ipsec_spi_t), &del_pbs,

if we're going to argue consistency then IKEv2 should use the totally
obscure sizeof(ipsec_spi_t).