[Openswan dev] ARM unaligned bug

Albert Veli albert.veli at gmail.com
Wed Jan 20 06:57:45 EST 2010


---------- Forwarded message ----------
From: Albert Veli <albert.veli at gmail.com>
Date: Wed, Jan 20, 2010 at 12:51 PM
Subject: Re: [Openswan dev] ARM unaligned bug
To: "D. Hugh Redelmeier" <hugh at mimosa.com>


> Here's a version that attempts to do this right.  I have not even
> compiled the code let alone tested it so it is surely wrong.
>
> /* Find last complete top-level payload and change its np (or fail) */
> bool
> out_modify_previous_np(u_int8_t np, pb_stream *outs)
> {
>    size_t left = (outs->cur - outs->start);
>    size_t offset = 0;
>
>    for (;;) {
>                if (left < sizeof(struct isakmp_hdr)) {
>                        return FALSE;   /* fail: no room for another payload */
>                } else {
>                        size_t pllen = (outs->start[offset + offsetof(struct isakmp_generic, isag_length)] << 8)
>                                       | outs->start[offset + offsetof(struct isakmp_generic, isag_length) + 1]
>
>                        if (left < pllen) {
>                                return FALSE;   /* fail: not enough room for this payload */
>                        } else if (left == pllen) {
>                                /* found last top-level payload */
>                                outs->start[offset + offsetof(struct isakmp_generic, isag_np)] = np;
>                                return TRUE;
>                        } else {
>                                /* this payload is not the last: scan forward */
>                                offset += pllen;
>                                left -= pllen;
>                        }
>                }
>        }
> }
>

There seems to be some problem with this code too. On the big-endian
machine it repeatedly comes to the "method 4" line:

pluto[563]: "ipsec1"[1] 88.88.88.88 #2: enabling possible
NAT-traversal with method 4
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[Dead Peer Detection]
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[RFC 3947] method set to=109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-03] meth=108, but already using method 109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-02_n] meth=106, but already using method
109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-02] meth=107, but already using method 109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-00]
pluto[563]: "ipsec1"[1] 88.88.88.88 #3: Aggressive mode peer ID is
ID_FQDN: '@init'
pluto[563]: "ipsec1"[1] 88.88.88.88 #3: responding to Aggressive Mode,
state #3, connection "ipsec1" from 88.88.88.88
pluto[563]: "ipsec1"[1] 88.88.88.88 #3: enabling possible
NAT-traversal with method 4
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[Dead Peer Detection]
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[RFC 3947] method set to=109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-03] meth=108, but already using method 109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-02_n] meth=106, but already using method
109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-02] meth=107, but already using method 109
pluto[563]: packet from 88.88.88.88:500: received Vendor ID payload
[draft-ietf-ipsec-nat-t-ike-00]
pluto[563]: "ipsec1"[1] 88.88.88.88 #4: Aggressive mode peer ID is
ID_FQDN: '@init'
pluto[563]: "ipsec1"[1] 88.88.88.88 #4: responding to Aggressive Mode,
state #4, connection "ipsec1" from 88.88.88.88
pluto[563]: "ipsec1"[1] 88.88.88.88 #4: enabling possible
NAT-traversal with method 4

I tried to debug and it always returns FALSE after the condition if
(left < pllen) because pllen gets a really big value.

On the little-endian machine pluto exits after the same line. I didn't
debug it yet.

But I managed to do a similar approach to the old code and it seems to work:

bool
out_modify_previous_np(u_int8_t np, pb_stream *outs)
{
       size_t len = (outs->cur - outs->start), offset;
       if (len < sizeof(struct isakmp_hdr)) {
               return FALSE;
       }
       else if (len == sizeof(struct isakmp_hdr)) {
               struct isakmp_hdr *hdr = (struct isakmp_hdr *)outs->start;
               hdr->isa_np = np;
               return TRUE;
       }
       else {
               struct isakmp_generic *hdr;
               u_int16_t isaglen;

               offset = sizeof(struct isakmp_hdr);

               while (offset < len) {
                       if ((len - offset) < sizeof(struct isakmp_generic))
                               return FALSE;
                       hdr = (struct isakmp_generic *)(outs->start+offset);
                       isaglen = (*((char *)hdr + 2) << 8) + *((char
*)hdr + 2 + 1);
                       if ((len - offset) < isaglen)
                               return FALSE;
                       if ((len - offset) == isaglen) {
                               hdr->isag_np = np;
                               return TRUE;
                       }
                       offset += isaglen;
               }
       }
       return FALSE;
}



The line that replaces the "dangerous" -> operation is: isaglen =
(*((char *)hdr + 2) << 8) + *((char *)hdr + 2 + 1);

I haven't digged into what the rest of the code actually does, but
according to the log files it seems to work on both machines.


Best regards,

Albert


More information about the Dev mailing list