diff --git a/lib/libswan/constants.c b/lib/libswan/constants.c
index a003e99..8f3a107 100644
--- a/lib/libswan/constants.c
+++ b/lib/libswan/constants.c
@@ -43,12 +43,11 @@
* The buffer bound (size) must be greater than 0.
* That allows a guarantee that the result is NUL-terminated.
*
- * The result is a pointer:
- * if the string fits, to the NUL at the end of the string in dest;
- * if the string was truncated, to the roof of dest.
+ * The result is a pointer to the NUL at the end of the string in dest.
*
- * Warning: Is it really wise to silently truncate? Only the caller knows.
- * The caller SHOULD check by seeing if the result equals dest's roof.
+ * Warning: no indication of truncation is returned.
+ * An earlier version did indicate truncation, but that feature was never used.
+ * This version is more robust and has a simpler contract.
*/
char *jam_str(char *dest, size_t size, const char *src)
{
@@ -56,12 +55,11 @@ char *jam_str(char *dest, size_t size, const char *src)
{
size_t full_len = strlen(src);
- bool oflow = size - 1 < full_len;
- size_t copy_len = oflow ? size - 1 : full_len;
+ size_t copy_len = size - 1 < full_len ? size - 1 : full_len;
memcpy(dest, src, copy_len);
dest[copy_len] = '\0';
- return dest + copy_len + (oflow ? 1 : 0);
+ return dest + copy_len;
}
}
diff --git a/include/packet.h b/include/packet.h
index c9b0df9..55f8b11 100644
--- a/include/packet.h
+++ b/include/packet.h
@@ -657,6 +657,8 @@ struct isakmp_nat_oa {
extern struct_desc isakmp_nat_d;
extern struct_desc isakmp_nat_oa;
+extern struct_desc isakmp_ignore_desc; /* generic payload (when ignoring) */
+
/* ISAKMP IKE Fragmentation Payload
* Cisco proprietary, undocumented
* Microsoft documentation link: http://msdn.microsoft.com/en-us/library/cc233452.aspx
diff --git a/programs/pluto/ikev1.c b/programs/pluto/ikev1.c
index e8157bb..155a27c 100644
--- a/programs/pluto/ikev1.c
+++ b/programs/pluto/ikev1.c
@@ -1810,8 +1810,7 @@ void process_packet_tail(struct msg_digest **mdp)
switch (np) {
case ISAKMP_NEXT_NATD_RFC:
case ISAKMP_NEXT_NATOA_RFC:
- if (st == NULL ||
- (st->hidden_variables.st_nat_traversal & NAT_T_WITH_RFC_VALUES) == LEMPTY) {
+ if ((st->hidden_variables.st_nat_traversal & NAT_T_WITH_RFC_VALUES) == LEMPTY) {
/*
* don't accept NAT-D/NAT-OA reloc directly in message,
* unless we're using NAT-T RFC
@@ -1832,6 +1831,7 @@ void process_packet_tail(struct msg_digest **mdp)
/* payload type is out of range or requires special handling */
switch (np) {
case ISAKMP_NEXT_ID:
+ /* ??? two kinds of ID payloads */
sd = (IS_PHASE1(from_state) ||
IS_PHASE15(from_state)) ?
&isakmp_identification_desc :
@@ -1839,20 +1839,48 @@ void process_packet_tail(struct msg_digest **mdp)
break;
case ISAKMP_NEXT_NATD_DRAFTS:
- np = ISAKMP_NEXT_NATD_RFC; /* NAT-D was a private use type before RFC-3947 */
+ /* NAT-D was a private use type before RFC-3947 -- same format */
+ np = ISAKMP_NEXT_NATD_RFC;
sd = payload_desc(np);
break;
case ISAKMP_NEXT_NATOA_DRAFTS:
- np = ISAKMP_NEXT_NATOA_RFC; /* NAT-OA was a private use type before RFC-3947 */
+ /* NAT-OA was a private use type before RFC-3947 -- same format */
+ np = ISAKMP_NEXT_NATOA_RFC;
sd = payload_desc(np);
break;
- case ISAKMP_NEXT_SAK: /* AKA ISAKMP_NEXT_NATD_BADDRAFTS */
+ case ISAKMP_NEXT_SAK: /* or ISAKMP_NEXT_NATD_BADDRAFTS */
+ /*
+ * Official standards say that this is ISAKMP_NEXT_SAK,
+ * a part of Group DOI, something we don't implement.
+ * Old non-updated Cisco gear abused this number in ancient NAT drafts.
+ * We ignore (rather than reject) this in support of people
+ * with crufty Cisco machines.
+ */
loglog(RC_LOG_SERIOUS,
- "%smessage with unsupported payload ISAKMP_NEXT_SAK (as ISAKMP_NEXT_NATD_BADDRAFTS) ignored",
+ "%smessage with unsupported payload ISAKMP_NEXT_SAK (or ISAKMP_NEXT_NATD_BADDRAFTS) ignored",
excuse);
- break;
+ /*
+ * Hack to discard payload, whatever it was.
+ * Since we are skipping the rest of the loop
+ * body we must do some things ourself:
+ * - demarshall the payload
+ * - grab the next payload number (np)
+ * - don't keep payload (don't increment pd)
+ * - skip rest of loop body
+ */
+ if (!in_struct(&pd->payload, &isakmp_ignore_desc, &md->message_pbs,
+ &pd->pbs)) {
+ loglog(RC_LOG_SERIOUS,
+ "%smalformed payload in packet",
+ excuse);
+ SEND_NOTIFICATION(PAYLOAD_MALFORMED);
+ return;
+ }
+ np = pd->payload.generic.isag_np;
+ /* NOTE: we do not increment pd! */
+ continue; /* skip rest of the loop */
default:
loglog(RC_LOG_SERIOUS,
@@ -1865,6 +1893,8 @@ void process_packet_tail(struct msg_digest **mdp)
passert(sd != NULL);
}
+ passert(np < LELEM_ROOF);
+
{
lset_t s = LELEM(np);
diff --git a/programs/pluto/packet.c b/programs/pluto/packet.c
index 2b104db..e759b5e 100644
--- a/programs/pluto/packet.c
+++ b/programs/pluto/packet.c
@@ -81,9 +81,6 @@ static field_desc isag_fields[] = {
{ ft_end, 0, NULL, NULL }
};
-static struct_desc isakmp_generic_desc =
- { "ISAKMP Generic Payload", isag_fields, sizeof(struct isakmp_generic) };
-
/* ISAKMP Data Attribute (generic representation within payloads)
* layout from RFC 2408 "ISAKMP" section 3.3
* This is not a payload type.
@@ -647,6 +644,11 @@ struct_desc isakmp_nat_oa =
{ "ISAKMP NAT-OA Payload", isanat_oa_fields,
sizeof(struct isakmp_nat_oa) };
+/* Generic payload (when ignoring) */
+
+struct_desc isakmp_ignore_desc =
+ { "ignored ISAKMP Generic Payload", isag_fields, sizeof(struct isakmp_generic) };
+
/* ISAKMP IKE Fragmentation Payload
* Cisco proprietary, undocumented
*
@@ -1892,7 +1894,7 @@ bool out_generic(u_int8_t np, struct_desc *sd,
{
struct isakmp_generic gen;
- passert(sd->fields == isakmp_generic_desc.fields);
+ passert(sd->fields == isag_fields);
gen.isag_np = np;
return out_struct(&gen, sd, outs, obj_pbs);
}