From 3ecfc5845189f17d35f854f690e45c450b522529 Mon Sep 17 00:00:00 2001 From: patrick Date: Thu, 7 Dec 2017 22:47:28 +0000 Subject: [PATCH] Change the SA payload parser to parse more than the first proposal. This allows us to select one of the peer's proposals (and not only the first). ok sthen@ hshoexer@ --- sbin/iked/ikev2_pld.c | 156 +++++++++++++++++++++--------------------- 1 file changed, 77 insertions(+), 79 deletions(-) diff --git a/sbin/iked/ikev2_pld.c b/sbin/iked/ikev2_pld.c index 71dd30af0dd..9c04fcc82c9 100644 --- a/sbin/iked/ikev2_pld.c +++ b/sbin/iked/ikev2_pld.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ikev2_pld.c,v 1.68 2017/12/04 17:22:39 patrick Exp $ */ +/* $OpenBSD: ikev2_pld.c,v 1.69 2017/12/07 22:47:28 patrick Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -326,10 +326,6 @@ ikev2_validate_sa(struct iked_message *msg, size_t offset, size_t left, return (0); } -/* - * NB: This function parses both the SA header and the first proposal. - * Additional proposals are ignored. - */ int ikev2_pld_sa(struct iked *env, struct ikev2_payload *pld, struct iked_message *msg, size_t offset, size_t left) @@ -342,95 +338,97 @@ ikev2_pld_sa(struct iked *env, struct ikev2_payload *pld, struct iked_proposals *props; size_t total; - if (ikev2_validate_sa(msg, offset, left, &sap)) - return (-1); + do { + if (ikev2_validate_sa(msg, offset, left, &sap)) + return (-1); + + /* Assumed size of the first proposals, including SPI if present. */ + total = (betoh16(sap.sap_length) - sizeof(sap)); - if (sap.sap_more) - log_debug("%s: more than one proposal specified", __func__); + props = &msg->msg_parent->msg_proposals; - /* Assumed size of the first proposals, including SPI if present. */ - total = (betoh16(sap.sap_length) - sizeof(sap)); + offset += sizeof(sap); + left -= sizeof(sap); - props = &msg->msg_parent->msg_proposals; + if (sap.sap_spisize) { + if (left < sap.sap_spisize) { + log_debug("%s: malformed payload: SPI larger than " + "actual payload (%zu < %d)", __func__, left, + sap.sap_spisize); + return (-1); + } + if (total < sap.sap_spisize) { + log_debug("%s: malformed payload: SPI larger than " + "proposal (%zu < %d)", __func__, total, + sap.sap_spisize); + return (-1); + } + switch (sap.sap_spisize) { + case 4: + memcpy(&spi32, msgbuf + offset, 4); + spi = betoh32(spi32); + break; + case 8: + memcpy(&spi64, msgbuf + offset, 8); + spi = betoh64(spi64); + break; + default: + log_debug("%s: unsupported SPI size %d", + __func__, sap.sap_spisize); + return (-1); + } - offset += sizeof(sap); - left -= sizeof(sap); + offset += sap.sap_spisize; + left -= sap.sap_spisize; - if (sap.sap_spisize) { - if (left < sap.sap_spisize) { - log_debug("%s: malformed payload: SPI larger than " - "actual payload (%zu < %d)", __func__, left, - sap.sap_spisize); - return (-1); - } - if (total < sap.sap_spisize) { - log_debug("%s: malformed payload: SPI larger than " - "proposal (%zu < %d)", __func__, total, - sap.sap_spisize); - return (-1); + /* Assumed size of the proposal, now without SPI. */ + total -= sap.sap_spisize; } - switch (sap.sap_spisize) { - case 4: - memcpy(&spi32, msgbuf + offset, 4); - spi = betoh32(spi32); - break; - case 8: - memcpy(&spi64, msgbuf + offset, 8); - spi = betoh64(spi64); - break; - default: - log_debug("%s: unsupported SPI size %d", - __func__, sap.sap_spisize); + + /* + * As we verified sanity of packet headers, this check will + * be always false, but just to be sure we keep it. + */ + if (left < total) { + log_debug("%s: malformed payload: too long for payload " + "(%zu < %zu)", __func__, left, total); return (-1); } - offset += sap.sap_spisize; - left -= sap.sap_spisize; - - /* Assumed size of the proposal, now without SPI. */ - total -= sap.sap_spisize; - } + log_debug("%s: more %d reserved %d length %d" + " proposal #%d protoid %s spisize %d xforms %d spi %s", + __func__, sap.sap_more, sap.sap_reserved, + betoh16(sap.sap_length), sap.sap_proposalnr, + print_map(sap.sap_protoid, ikev2_saproto_map), sap.sap_spisize, + sap.sap_transforms, print_spi(spi, sap.sap_spisize)); - /* - * As we verified sanity of packet headers, this check will - * be always false, but just to be sure we keep it. - */ - if (left < total) { - log_debug("%s: malformed payload: too long for payload " - "(%zu < %zu)", __func__, left, total); - return (-1); - } + if (ikev2_msg_frompeer(msg)) { + if ((msg->msg_parent->msg_prop = config_add_proposal(props, + sap.sap_proposalnr, sap.sap_protoid)) == NULL) { + log_debug("%s: invalid proposal", __func__); + return (-1); + } + prop = msg->msg_parent->msg_prop; + prop->prop_peerspi.spi = spi; + prop->prop_peerspi.spi_protoid = sap.sap_protoid; + prop->prop_peerspi.spi_size = sap.sap_spisize; - log_debug("%s: more %d reserved %d length %d" - " proposal #%d protoid %s spisize %d xforms %d spi %s", - __func__, sap.sap_more, sap.sap_reserved, - betoh16(sap.sap_length), sap.sap_proposalnr, - print_map(sap.sap_protoid, ikev2_saproto_map), sap.sap_spisize, - sap.sap_transforms, print_spi(spi, sap.sap_spisize)); + prop->prop_localspi.spi_protoid = sap.sap_protoid; + prop->prop_localspi.spi_size = sap.sap_spisize; + } - if (ikev2_msg_frompeer(msg)) { - if ((msg->msg_parent->msg_prop = config_add_proposal(props, - sap.sap_proposalnr, sap.sap_protoid)) == NULL) { - log_debug("%s: invalid proposal", __func__); + /* + * Parse the attached transforms + */ + if (sap.sap_transforms && + ikev2_pld_xform(env, &sap, msg, offset, total) != 0) { + log_debug("%s: invalid proposal transforms", __func__); return (-1); } - prop = msg->msg_parent->msg_prop; - prop->prop_peerspi.spi = spi; - prop->prop_peerspi.spi_protoid = sap.sap_protoid; - prop->prop_peerspi.spi_size = sap.sap_spisize; - prop->prop_localspi.spi_protoid = sap.sap_protoid; - prop->prop_localspi.spi_size = sap.sap_spisize; - } - - /* - * Parse the attached transforms - */ - if (sap.sap_transforms && - ikev2_pld_xform(env, &sap, msg, offset, total) != 0) { - log_debug("%s: invalid proposal transforms", __func__); - return (-1); - } + offset += total; + left -= total; + } while (sap.sap_more); return (0); } -- 2.20.1