[Swan-dev] [Swan-commit] Changes to ref refs/heads/master

[D. Hugh Redelmeier]

| From: Paul Wouters <paul at nohats.ca>

| On Fri, 17 Jul 2015, D. Hugh Redelmeier wrote:
| 

| -       if (ipprotoid == IPPROTO_ESP || ipprotoid == IPPROTO_AH)
| +       if (proto_info != NULL) {
| 
| So I had changed it because I think the first line is much more clear.
| If you do ESP or AH, get a SPI number.
| 
| The NULL check just obfuscates that......

I don't think so.  Look at the next line:
		proto_info->our_spi = get_ipsec_spi(0, /* avoid this # */

Clearly we need to know that proto_info is not NULL.  So the test
addresses this ("guards" the dereference).

The immediately preceding switch statement establishes which cases are
handled, and pretty explicitly.

Now you've forced me to read that function again :-)

The logic does not handle rekeying.  I've made this clearer.

I don't understand why the code goes to such trouble to skip an
encryption transform within AH (see skip_encr).  Why the heck does it
not just get mad when it see encryption in AH?  (I probably used to
know -- I think that wrote the current code as a simplification of the
previous code.)

In 523ee509f7110862dd497027a58dd327c40ad804 you added code in this
routine to handle ikev2_out_attr failure.  That's good.  But it
included a message that is not useful to a user:
	libreswan_log("ikev2_out_attr() failed");
Is this message redundant (i.e. the error has already been reported
reasonably)?  If so, it should be removed.  If not, in should be
improved.

I've checked in some changes to reflect these issues.  Untested but
they appear safe.