[Swan-dev] Camellia trransform representation in IKEv1

[D. Hugh Redelmeier]

I'm looking at ce89ea7feed076044110da204456b8ba68a0986a, but not carefully 
enough.  My justification: this code should be clear enough that one 
doesn't have to look at it carefully.

As I understand it, the annoying fundamental problem is that IKEv1 and 
IKEv2 differ on how to represent Camellia, and worse, the representation 
used in v1 already has a different meaning in v2.

I think that the v2 representation has no meaning in v1.

We should always know whether we're dealing with a v1 or a v2 number.

But since they are almost the same, we have used the same functions
for both v1 and v2.

To keep the code sane, if it is shared, I would think that we should
be using a superset coding.  Perhaps that is the v2 code.  It cannot
be the v1 code since we've just seen that v1 assigns Camellia the same
number as something else uses in v2.

Anyway, the functions seems to be going back and forth in ways that
are not clear.  I think we need to systematize this, simplify it, and
document it.  With luck, we only have to change representations very
locally and the resulting value should be ephemeral.

One way to help is to use a macro or function to do each
transformation, not if-statements.

/* NOTE: for illustration only; all names are likely wrong */

static inline enum alg_v2 alg_univ_to_v2(enum alg_univ i)
{
	return i; /* I think that this is the identity function */
}

static inline enum alg_univ alg_v2_to_univ(enum alg_v2 i)
{
	return i; /* I think that this is the identity function */
}

static inline enum alg_v1 alg_univ_to_v1(enum alg_univ i)
{
	return i == ESP_CAMELLIA ? ESP_CAMELLIAv1 : i;
}

static inline enum alg_univ alg_v1_to_univ(enum alg_v1 i)
{
	return i == ESP_CAMELLIAv1 ? ESP_CAMELLIA : i;
}

(Each of these functions could check that the argument value is
proper.  That might make "inline" less appropriate.)

At a minimum, each variable or field that holds one of these values
should have a comment to say which of the three varaints it contains:
universal, v1, or v2.  Even better is to use a type to signify this.

The whole sordid story should be documented clearly in the code (perhaps 
it is).