ikev1: Properly handle different proposal numbering schemes

While the examples in RFC 2408 show proposal numbers starting at 1 and
increasing by one for each subsequent proposal this is not mandatory.
Actually, IKEv1 proposals may start at any number, the only requirement
is that the proposal numbers increase monotonically they don't have to
do so consecutively.

Most implementations follow the examples and start numbering at 1 (charon,
racoon, Shrew, Cisco, Windows XP, FRITZ!Box) but pluto was one of the
implementations that started with 0 and there might be others out there.

The previous assumption that implementations always start numbering proposals
at 0 caused problems with clients that start numbering with 1 and whose first
proposal consists of multiple protocols (e.g. ESP+IPComp).

Fixes #661.

diff --git a/src/libcharon/encoding/payloads/sa_payload.c b/src/libcharon/encoding/payloads/sa_payload.c
index 8e3a012..59dac21 100644 (file)
--- a/src/libcharon/encoding/payloads/sa_payload.c
+++ b/src/libcharon/encoding/payloads/sa_payload.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Tobias Brunner
+ * Copyright (C) 2012-2014 Tobias Brunner
  * Copyright (C) 2005-2010 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * Hochschule fuer Technik Rapperswil
@@ -296,7 +296,7 @@ METHOD(sa_payload_t, get_proposals, linked_list_t*,
        linked_list_t *substructs, *list;
 
        if (this->type == PLV1_SECURITY_ASSOCIATION)
-       {       /* IKEv1 proposals start with 0 */
+       {       /* IKEv1 proposals may start with 0 or 1 (or any other number really) */
                struct_number = ignore_struct_number = -1;
        }
 
@@ -309,17 +309,22 @@ METHOD(sa_payload_t, get_proposals, linked_list_t*,
        enumerator = this->proposals->create_enumerator(this->proposals);
        while (enumerator->enumerate(enumerator, &substruct))
        {
+               int current_number = substruct->get_proposal_number(substruct);
+
                /* check if a proposal has a single protocol */
-               if (substruct->get_proposal_number(substruct) == struct_number)
+               if (current_number == struct_number)
                {
                        if (ignore_struct_number < struct_number)
-                       {       /* remove an already added, if first of series */
+                       {       /* remove an already added substruct, if first of series */
                                substructs->remove_last(substructs, (void**)&substruct);
                                ignore_struct_number = struct_number;
                        }
                        continue;
                }
-               struct_number++;
+               /* for IKEv1 the numbers don't have to be consecutive, for IKEv2 they do
+                * but since we don't really care for the actual number we accept them
+                * anyway. we already verified that they increase monotonically. */
+               struct_number = current_number;
                substructs->insert_last(substructs, substruct);
        }
        enumerator->destroy(enumerator);