[Swan-dev] More confusion of options to clean up regarding phase1 and phase2 options

[Paul Wouters]

On Sat, 19 Apr 2014, Matt Rogers wrote:

>> 	esp=/ah= or phase2alg= ?
>>
>> 	same of different parser for phase2alg= versus ike= (phase1alg) ?
>>
> These should be able to at least appear cohesive, for example
> phase1alg=
> phase2alg=
>
> or:
> ike=
> esp/ah=

While I agree, in IKEv2 we don't really say "phase 1" or "phase 2"
anymore. Partially because the "phase 1" kinda comes with a free "phase
2" (and then can receive CREATE_CHILD_SA's for more "phase 2s".

I personally don't mind the ike= and esp=/ah=. I don't remember why
Michael went on to try and migrate peopleto phase2alg=. I assume mostly
to get rid of esp= vs ah= keywords?

>> 	default proposal for ikev1 and ikev2, not the v1tov2 mess we have now.
>>
>> 	allowing v2 style: esp=aes,3des,sha1,md5 ? Use that to build v1 style?
>> 	perhaps using a new keyword default-proposal or something ?
>>
>> 	(v1 proposals are sets, v2 proposals are items)
>>
> If the parser can choose a default for an item left out of the set (or ignore an unneeded option), then it would be flexible enough to work for both.

My thoughts was that each item seperated by a "," should be identifiable
as either an ENCR or INTEG algorithm (or an AEAD like aes_gcm)

strongswan uses: aes128-aes256-sha1-modp1536-modp2048,3des-sha1-md5-modp1024.
While it looks silly, it is so allow for support of IKEv2 proposal sets.
The "," seperates the sets. This way, one could propose "only md5 with
3des, not with aes" but using two proposals. Alternatives to that could
be:  esp=3des,aes,sha1,md5;aes_gcm

which would mean in IKEv2 that you can have either 3des or aes with md5
or sha1 OR aes_gcm (but not aes_gcm with sha1 - which would violate the spec)
For ikev1, that esp= line would need to generate a proposal set of
"3des-md5,3des-sha1,aes-md5,aes-sha1,aes_gcm". But we need to deal with
the ";" for modp then.

Another issue:

ike=3des-sha1-modp1024	works
ike=3des-sha1;modp1024  works
esp=3des-sha1-modp1024  FAILS
esp=3des-sha1;modp1024  works


>> 	rip out the crappy parser hacks scattered everywhere for these
>> options.
>>
>> 	deal with GROUP names. some are modp, some are dh, but the entire list
>> has no common name. Perhaps use group= ?
>>
> Aliasing options right now is a hack, so that is something that should be built in and easy for us to add new aliases.

I'm not sure how easy that is to do. I'm fine with an easier way of
adding them. Are you thinking a "preparsing" that rewrites aliases into
the real options, than calls our current code?

>> 	pfsgroup=
>>
>> 	confusion of esp=3des-sha1-modp1536 bs esp=3des-sha1;modp2048
>>
>> 	deal with rsasigkey= versus ecsigkey or start using pubsigkey= ?
>>
> I like pubsigkey= better. While there is a technical difference between the two, the intent of the option is still the same.

As long as that works for identifying EC keys too. I assume X.509 is no
problem with the friendly_name on import, but I'm not sure how we would
identify a (raw) EC key yet - then again, technically I don't think the
RFCs support raw EC keys yet because the draft did not get enough
traction at the working group.

>> 	fix up AES CCM/GCM keywords, do not require silly "null" for auth
>>
>> To me this is a higher priority than fixing "_". But the "_" fix is
>> something that can be done in a day, and I guess should just be done
>> now. The above is not :(
>
> There is a wide range of choices when it comes to algorithms so I think it should be easy for a user to choose the options that they mean to, and accommodate for small mistakes. For example if someone does ah=aes-sha1, it continues with choosing sha1 for AH and ignores the aes. In turn, is this being helpful or just misleading?

That's actually really bad, and I put a test case into
testing/lib/libpluto/algparse.c for that. It MUST fail because the user
might think they are getting AES encryption when they are not getting
any encryption! We seem to fail it. (although our failures are failing
with a passert rather than just rejecting the string)

> I'm happy to help out with these efforts. Perhaps we can use check or CUnit for beefing up the unit tests and expand to other areas of the code.

See algparse.c, eg:

cd testing/lib/libswan
make check
./algparse

Paul