Subject: [IPsec] On the alignment of IKEv2 payload fields, and a modest proposal

From: Dan McDonald <[email-masked]>
Date: Fri, 12 Sep 2008 12:55:42 -0400

I know at least in racoon2, there were reported bugs because some IKE
payloads (certificates in particular) would often push payloads that follow
it out of 32-bit or even 64-bit alignment.  This causes implementations to
perform some amount of unnatural acts due to not being able to assume
alignment.

I'm fully aware of "be liberal in what you recieve", but is there a way in
IKEv2bis we can encourage aligned payloads?  Actually, as I type it, I think
there might be...

A MODEST PROPOSAL... actually... a modest payload:

===================== (Cut up to and including here.) =====================

3.x    Alignment Payload

   The Alignment Payload SHOULD be inserted by a transmitter to re-establish
   alignment of subsequent payloads in an IKEv2 packet.  It MUST be ignored
   upon receipt, and it MUST have the critical bit (C) set to 0 to confirm
   that it must be ignored.

                        1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   | Next Payload  |0|  RESERVED   |         Payload Length        |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |   ignored bytes  (variable length).....			   |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   For example, if an implementation wishes to send 32-bit aligned payloads,
   it would insert an Alignment Payload if and only if the previous payload
   was of length inequal to 0, modulo 4.  This remainder/modulo 'X' (ranging
   between 1 and 3), would be used to compute a properly-sized Alignment
   Payload of length (8 - X) bytes to follow the previous payload.  The
   payload subsequent to the Alignment Payload would once again be on a
   32-bit boundary.

   In another example, if one desired a 16-byte aligned payload, extras 'X'
   would range between 1 and 15, and the Alignment Payload length would be
   computed as 4 + ((28 - X) % 16), because in the cases of 13 through
   16, the four-byte base header would push the payload into the next
   alignment boundary.

   The general formula for computing an Alignment Header for an N-byte
   boundary (for N >= 4 and N divisible by 4) given an
   over-boundary-byte-count X is:

	4 + ((2N - X - 4) % N)

   If powers-of-2 are the only allowable byte boundary, the above formula can
   be optimized to:

	4 + ((2N - X - 4) & (N - 1))

   using less-expensive logical-AND.

===================== (Cut up to and including here.) =====================

As noted above, I would recommend that implementations SHOULD use this
payload to insure 32-bit alignment, and MAY use it to insure 64-bit
alignment.

What do you think?

Dan
_______________________________________________
IPsec mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/ipsec

---

From: Dan McDonald <[email-masked]>
Date: Fri, 12 Sep 2008 13:19:22 -0400

> IKEv2bis we can encourage aligned payloads?  Actually, as I type it, I think
<mucho snippage deleted!>

Sorry for following up on my own post, but two other things:

	* My proposed payload shouldn't be needed/used a lot.  In fact,
          a smart implementation would:

		- Put all fixed-size and aligned payloads up front.

		- Put all variable-sized/unaligned payloads at the end.

		- Only use the Alignment Payload to separate the unaligned
		  payloads.

	  I could also talk about proper payload design and designed-in
	  alignment, but I'm too late for that.

	* Attached is some illustrative code for computing lengths.

Dan

#include <sys/types.h>
#include <stdlib.h>
#include <stdio.h>

void
usage(char *progname)
{
	printf("Usage:  %s <boundary> <hdr-len> {use-p2-calc?}\n", progname);
	exit(1);
}

uint_t
f(uint_t x, uint_t boundary, uint_t headerlen, boolean_t usep2)
{
	if (usep2)
		return (headerlen + (((boundary << 1) - x - headerlen) &
		    (boundary - 1)));
	else
		return (headerlen + ((boundary * 2 - x - headerlen) %
		    boundary));
}

int
main(int argc, char *argv[])
{
	uint_t boundary, headerlen, i;
	boolean_t usep2;

	if (argc < 3)
		usage(argv[0]);

	boundary = atoi(argv[1]);
	headerlen = atoi(argv[2]);
	/* If third argument is present, use the power-of-2 implementation. */
	usep2 = (argv[3] != NULL);

	printf("OVERAGE\t\tPAD-BYTES (incl. header of %d bytes)\n",
	    headerlen);
	printf("=======\t\t=======================================\n");
	for (i = 1; i < boundary ; i++) {
		printf("%d\t\t%d\n", i, f(i, boundary, headerlen, usep2));
	}

	return (0);
}

---

From: Nicolas Williams <[email-masked]>
Date: Fri, 12 Sep 2008 12:56:01 -0500

On Fri, Sep 12, 2008 at 12:55:42PM -0400, Dan McDonald wrote:
>    [...]

> payload to insure 32-bit alignment, and MAY use it to insure 64-bit

But how would the sender know what alignment the receiver prefers?

I think a notify payload for indicating one's alignment preference
becomes necessary.  Alternatively 64-bit alignment should suffice, so
say SHOULD 64-bit.

But also, by putting all alignment-breaking payloads at the end of the
message, as you also suggest, should be enough, no?

Nico
--

---

From: Dan McDonald <[email-masked]>
Date: Fri, 12 Sep 2008 14:16:38 -0400

> But how would the sender know what alignment the receiver prefers?

It doesn't.  This is an aid, not a mandate.  The inclusion of one ALIGNMENT
payload could implicitly express a desired alignment quite simply.  Also, the
IPv4 + UDP headers are not 64-bit aligned (20 + 8 == 28) unless you mandate
the zero-SPI, which makes things much worse.

> becomes necessary.  Alternatively 64-bit alignment should suffice, so

IMHO a notify is too much.  Then again, IMHO, I'm pushing it just suggesting
an alignment payload.

Also, the more I think about it, the more that any alignment beyond 32-bit is
annoyingly hard, because of the sizeof(IPv4) vs. sizeof(IPv6) problem.

> message, as you also suggest, should be enough, no?

Most of the time it should be, but I'm thinking about identities and such.
The example third packet in ikev2bis has:

   HDR, SK {IDi, [CERT,] [CERTREQ,]
       [IDr,] AUTH, SAi2,
       TSi, TSr}  -->

and there are potentially four unaligned fields (IDi, IDr, CERT, CERTREQ).
So you can put AUTH, SAi2, TSi and TSr up front, but after that, you're going
to have potential problems.  Though to be fair, your parsing code of those
extensions is going to be performing unnatural acts already, so perhaps it is
not so bad merely putting unaligned payloads at the end.

It's worth a thread here on the list, at any rate.  I'm particularly
interested hearing from those who've written header-parsing code.

Dan

---

From: Paul Koning <[email-masked]>
Date: Fri, 12 Sep 2008 14:25:39 -0400

>>>>> "Dan" == Dan McDonald <[email-masked]> writes:

 Dan> I know at least in racoon2, there were reported bugs because
 Dan> some IKE payloads (certificates in particular) would often push
 Dan> payloads that follow it out of 32-bit or even 64-bit alignment.
 Dan> This causes implementations to perform some amount of unnatural
 Dan> acts due to not being able to assume alignment.

 Dan> I'm fully aware of "be liberal in what you recieve", but is
 Dan> there a way in IKEv2bis we can encourage aligned payloads?

Yes, but what purpose would that serve?

If it's not mandatory, you still need the same code you have today at
the receive end.  So you've added complexity (at the transmit end) for
no benefit (no savings at the receive end).

Also, it seems to me that the "unnatural acts" amount to adding
"packed" to a variable declaration -- and the code the compiler
generates when you do this.  Since IKE isn't fastpath that isn't
enough to matter.

       paul

---

From: Paul Koning <[email-masked]>
Date: Fri, 12 Sep 2008 14:27:45 -0400

>>>>> "Dan" == Dan McDonald <[email-masked]> writes:

 Dan> It's worth a thread here on the list, at any rate.  I'm
 Dan> particularly interested hearing from those who've written
 Dan> header-parsing code.

I have.  I think the cost of the unaligned case is completely
irrelevant both in code complexity (tiny) and performance (ditto).
Now if we were talking fastpath, that might be different.

In particular, compared to the hassle of dealing with byte order,
alignment is a non-issue.

	  paul

---

From: Dan McDonald <[email-masked]>
Date: Fri, 12 Sep 2008 14:45:08 -0400

In my mind, "unnatural acts" include any load-one-byte-at-a-time ops, but as
you point out, it's not like IKE is in ANYONE'S fastpath, given dominance by
D-H operations.

> Now if we were talking fastpath, that might be different.

Agreed.

> alignment is a non-issue.

Says you!  ;) I have to deal with two different architectures, one with
strict alignment but proper byte-order (SPARC), and one with loose alignment
but improper byte-order (i386/amd64)

Dan

---

From: Paul Koning <[email-masked]>
Date: Fri, 12 Sep 2008 15:24:27 -0400

>>>>> "Dan" == Dan McDonald <[email-masked]> writes:

 Dan> <mucho snippage deleted!>

 Dan> In my mind, "unnatural acts" include any load-one-byte-at-a-time
 Dan> ops, ...

Sure, but the compiler can do that for you.

 >> Now if we were talking fastpath, that might be different.

 Dan> Agreed.

 >> alignment is a non-issue.

 Dan> Says you!  ;) I have to deal with two different architectures,
 Dan> one with strict alignment but proper byte-order (SPARC), and one
 Dan> with loose alignment but improper byte-order (i386/amd64)

At least you don't have to deal with strict alignment AND wrong byte
order (MIPS little endian) as we do...  :-)

      paul

---

From: "Dan Harkins" <[email-masked]>
Date: Sat, 13 Sep 2008 15:19:37 -0700 (PDT)

Hi Dan,

  Why not just use a 3 of the reserved bits in the IKE header to
indicate how much padding (included in the Payload Length) you've
added to the particular payload that would've otherwise pushed the
next payload out of alignment?

  regards,

  Dan.

---

From: Tero Kivinen <[email-masked]>
Date: Mon, 15 Sep 2008 13:59:44 +0300

>  >> Now if we were talking fastpath, that might be different.

I have also written code that must work on any byte order and in any
alignment requirements, and we do not even have any idea what those
requirements are, as our customers can use our OEM toolkit in various
platforms...

We use SSH_{GET,PUT}_{8,16,24,32,40,64}BIT macros to do all the
reading and writing data from actual packet. Those macros are then
either generic c-code using unsigned char pointers or optimzed code
for specific platforms (including assembler optimizations on some
platforms). It really does not affect coding once you get used to
that, and you normally first decode the packet completely to internal
c-structures and use those from that on. That also makes sure you have
already checked all payloads before you even start using the data, and
you do not need to care about things like payload orderings and so on,
as you decode full packet in the beginning.

I do not really think alignment issues matters that much in modern
processors, as the slow operation will be the fetching the data from
the main memory to the cache, and after it is in the cache processor
can quite quickly do whatever it likes with the data.

Also parsing and generating IKE payloads is so fast compared to other
things (Diffie-Hellman, RSA operations, even encryption and hashing)
that it is not really even noticeable.

I do not want to add any more complications to the IKE thus I do not
think we need any alignment payloads.

We did have some dicussion during the ikev2 protocol development phase
too:

http://www.ietf.org/mail-archive/web/ipsec/current/msg00348.html
http://www.ietf.org/mail-archive/web/ipsec/current/msg00349.html

(I think there was more discussion earlier, but cannot seem to find
those). 
-- 
[email-masked]
_______________________________________________
IPsec mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/ipsec

---

From: Dan McDonald <[email-masked]>
Date: Mon, 15 Sep 2008 10:04:43 -0400

On Mon, Sep 15, 2008 at 01:59:44PM +0300, Tero Kivinen wrote:
<mucho snippage deleted!>
> think we need any alignment payloads.

Fair enough.  I was more curious than anything else.

I would *recommend* to people, though, especially given that we've agreed
that payload order doesn't matter, that it would be nice if
known-to-be-aligned payloads go first.

Thanks,
Dan

---

From: Paul Koning <[email-masked]>
Date: Mon, 15 Sep 2008 10:48:03 -0400

>>>>> "Dan" == Dan McDonald <[email-masked]> writes:

 Dan> On Mon, Sep 15, 2008 at 01:59:44PM +0300, Tero Kivinen wrote:
 Dan> <mucho snippage deleted!>
 >> I do not want to add any more complications to the IKE thus I do
 >> not think we need any alignment payloads.

 Dan> Fair enough.  I was more curious than anything else.

 Dan> I would *recommend* to people, though, especially given that
 Dan> we've agreed that payload order doesn't matter, that it would be
 Dan> nice if known-to-be-aligned payloads go first.

I disagree.  Such recommendations make no difference to conforming
receiver implementations, but they may increase the probability of
nonconfirming ones.  (Someone might misinterpret that recommendation
and code up a receiver that only works correctly with the recommended
ordering.)

If such a recommendation is included, it will be important for
conformance tests to exercise drastically different orderings.

    paul

---

From: Yaron Sheffer <[email-masked]>
Date: Tue, 16 Sep 2008 09:14:19 +0300

Hi Dan,

going back to your original mail on this thread, I'd like to point out 
that the goal of IKEv2bis is very specifically to clarify the standard, 
rather than to change or extend it. If extensions are needed, there are 
other ways to have them, namely a separate RFC.

Thanks,

    Yaron

Dan McDonald wrote:

> Scanned by Check Point Total Security Gateway.
>
>