[Openswan dev] Assertion failed at packet.c:1672

[D. Hugh Redelmeier]

| From: Albert Veli <albert.veli at gmail.com>

|  ASSERTION FAILED at .../openswan-2.6.24/lib/libpluto/packet.c:1672: pl[NOFFSETOF_isa_np] == ISAKMP_NEXT_NONE
| 
| 
| The code around the assert looks like this:
| 
|     if (left == NSIZEOF_isakmp_hdr) {
| 	/* no payloads, just the isakmp_hdr: insert np here */
| 	passert(pl[NOFFSETOF_isa_np] == ISAKMP_NEXT_NONE);
| 	pl[NOFFSETOF_isa_np] = np;
|     }
| 


| Then I found two problems. First one in packet.h, it looks like this:
| 
| #define NOFFSETOF_isa_np        8       /* on-the-wire offset of
| isa_np (one octet) */
| 
| struct isakmp_hdr
| {
|     u_int8_t    isa_icookie[COOKIE_SIZE];
|     u_int8_t    isa_rcookie[COOKIE_SIZE];
|     u_int8_t    isa_np;                 /* Next payload */
| ...
| };
| 
| Well COOKIE_SIZE is 8 (found it in ietf_constants.h) so
| NOFFSETOF_isa_np should be 16.

Right.  I just assumed that would be fixed at the same time that may
buggy NSIZEOF_isakmp_hdr was fixed.  (Thanks for that fix too.)

| So now I changed the assert to:
| 
|  passert(pl[NOFFSETOF_isa_np] == ISAKMP_NEXT_NONE ||
| pl[NOFFSETOF_isa_np] == ISAKMP_NEXT_HASH);
| 
| That seems to work. Well, at least it dodges the assert. If it is
| correct is another question ;-)

We backed off the version of that assertion in the loop.  You have
demonstrated it needs fixing here too.

| Summary. In packet.h at line 170, change to:
| 
| #define NOFFSETOF_isa_np       16       /* on-the-wire offset of
| isa_np (two octets) */
| 
| And in packet.c at line 1672, allow for pl[NOFFSETOF_isa_np] to be
| ISAKMP_NEXT_HASH.
| 
| The change in packet.h seems bullet proof to me. But the change in
| packet.c needs review by somebody who are familiar with ipsec. Why is
| isa_np == ISAKMP_NEXT_HASH? Is that normal?

Looks good to me.

Thanks!