Hi,
this is a somewhat bigger diff that tries to separate the
state-changing message handling parts from the message parsing in iked.
Touching the SA structure in the parser is generally a bad idea
as it means we're changing global state before the message
is fully validated (or decrypted/integrity checked).
The main offenders in iked are the "Notify" and "Certificate Request"
payloads.
With the diff, the parsed message content is attached to the
iked_message structure (temporarily) and the SA (global state)
is modified only when the full message is considered valid.
This fixes a few problems:
Broken message could change global state, now they only change state
when we consider them not broken.
The evaluation of Notify/Certreq contents depended on the order
of the payloads, which is problematic because IKEv2 does not put any
restrictions on the order of payloads. With the diff applied,
message contents are evaluated in a fixed order.
Functionally there should be no difference, with or without the diff,
in use-cases that have worked before and I haven't found any regressions
in my tests. I would appreciate if people running iked could apply and
test this diff in their setups, just to be sure.
Tobias
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index bc64f5b3a7b..c2429daf7a3 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -537,6 +537,14 @@ struct iked_message {
struct iked_id msg_id;
struct iked_id msg_cert;
struct ibuf *msg_cookie;
+ uint16_t msg_group;
+ uint16_t msg_cpi;
+ uint8_t msg_transform;
+ uint16_t msg_flags;
+
+ uint8_t msg_certreq_type; /* CR Encoding*/
+ struct ibuf *msg_certreq_auth; /* CR Authority */
+ struct ibuf *msg_certreq_ocsp; /* CR OCSP values */
/* MOBIKE */
int msg_update_sa_addresses;
@@ -554,6 +562,19 @@ struct iked_message {
#define IKED_RETRANSMIT_TRIES 5 /* try 5 times */
};
+#define IKED_MSG_NAT_SRC_IP 0x01
+#define IKED_MSG_NAT_DST_IP 0x02
+
+#define IKED_MSG_FLAGS_FRAGMENTATION 0x0001
+#define IKED_MSG_FLAGS_MOBIKE 0x0002
+#define IKED_MSG_FLAGS_SIGSHA2 0x0004
+#define IKED_MSG_FLAGS_CHILD_SA_NOT_FOUND 0x0008
+#define IKED_MSG_FLAGS_NO_ADDITIONAL_SAS 0x0010
+#define IKED_MSG_FLAGS_AUTHENTICATION_FAILED 0x0020
+#define IKED_MSG_FLAGS_INVALID_KE 0x0040
+#define IKED_MSG_FLAGS_IPCOMP_SUPPORTED 0x0080
+
+
struct iked_user {
char usr_name[LOGIN_NAME_MAX];
char usr_pass[IKED_PASSWORD_SIZE];
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index bd22bda0255..3b4300ca7b3 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -82,6 +82,8 @@ int ikev2_send_error(struct iked *, struct iked_sa *,
struct iked_message *, uint8_t);
int ikev2_send_init_error(struct iked *, struct iked_message *);
+int ikev2_handle_certreq(struct iked*, struct iked_message *);
+
int ikev2_send_create_child_sa(struct iked *, struct iked_sa *,
struct iked_spi *, uint8_t);
int ikev2_ikesa_enable(struct iked *, struct iked_sa *, struct iked_sa *);
@@ -118,6 +120,8 @@ int ikev2_match_proposals(struct iked_proposal *, struct
iked_proposal *,
int ikev2_valid_proposal(struct iked_proposal *,
struct iked_transform **, struct iked_transform **, int *);
+int ikev2_handle_notifies(struct iked *, struct iked_message *);
+
ssize_t ikev2_add_proposals(struct iked *, struct iked_sa *, struct
ibuf *,
struct iked_proposals *, uint8_t, int, int, int);
ssize_t ikev2_add_cp(struct iked *, struct iked_sa *, struct ibuf *);
@@ -845,6 +849,9 @@ ikev2_init_recv(struct iked *env, struct iked_message *msg,
if (!ikev2_msg_frompeer(msg))
return;
+ if (ikev2_handle_notifies(env, msg) != 0)
+ return;
+
if (sa && msg->msg_nat_detected && sa->sa_natt == 0 &&
(sock = ikev2_msg_getsocket(env,
sa->sa_local.addr_af, 1)) != NULL) {
@@ -881,6 +888,9 @@ ikev2_init_recv(struct iked *env, struct iked_message *msg,
__func__));
break;
}
+ if (ikev2_handle_certreq(env, msg) != 0)
+ return;
+
(void)ikev2_init_auth(env, msg);
break;
case IKEV2_EXCHANGE_IKE_AUTH:
@@ -2354,6 +2364,9 @@ ikev2_resp_recv(struct iked *env, struct iked_message
*msg,
if (!ikev2_msg_frompeer(msg))
return;
+ if (ikev2_handle_notifies(env, msg) != 0)
+ return;
+
if ((sa = msg->msg_sa) == NULL)
return;
@@ -2398,6 +2411,9 @@ ikev2_resp_recv(struct iked *env, struct iked_message
*msg,
sa->sa_policy->pol_auth.auth_eap)
sa_state(env, sa, IKEV2_STATE_EAP);
+ if (ikev2_handle_certreq(env, msg) != 0)
+ return;
+
if (ikev2_ike_auth_recv(env, sa, msg) != 0) {
log_debug("%s: failed to send auth response", __func__);
ikev2_send_error(env, sa, msg, hdr->ike_exchange);
@@ -2423,6 +2439,102 @@ ikev2_resp_recv(struct iked *env, struct iked_message
*msg,
}
}
+int
+ikev2_handle_notifies(struct iked *env, struct iked_message *msg)
+{
+ struct iked_sa *sa;
+ uint16_t group;
+
+ if ((sa = msg->msg_sa) == NULL)
+ return (-1);
+
+ if (msg->msg_flags & IKED_MSG_FLAGS_CHILD_SA_NOT_FOUND)
+ sa->sa_stateflags &= ~IKED_REQ_CHILDSA;
+
+ if ((msg->msg_flags & IKED_MSG_FLAGS_FRAGMENTATION) && env->sc_frag) {
+ log_debug("%s: fragmentation enabled", __func__);
+ sa->sa_frag = 1;
+ }
+
+ if ((msg->msg_flags & IKED_MSG_FLAGS_MOBIKE) && env->sc_mobike) {
+ log_debug("%s: mobike enabled", __func__);
+ sa->sa_mobike = 1;
+ /* enforce natt */
+ sa->sa_natt = 1;
+ }
+
+ if ((msg->msg_flags & IKED_MSG_FLAGS_NO_ADDITIONAL_SAS)
+ && sa->sa_stateflags & IKED_REQ_CHILDSA) {
+ /* This makes sense for Child SAs only atm */
+ ikev2_disable_rekeying(env, sa);
+ sa->sa_stateflags &= ~IKED_REQ_CHILDSA;
+ }
+
+ if (msg->msg_flags & IKED_MSG_FLAGS_AUTHENTICATION_FAILED) {
+ log_debug("%s: AUTHENTICATION_FAILED, closing SA", __func__);
+ ikev2_ike_sa_setreason(sa,
+ "authentication failed notification from peer");
+ sa_state(env, sa, IKEV2_STATE_CLOSED);
+ msg->msg_sa = NULL;
+ return (-1);
+ }
+
+ if (msg->msg_flags & IKED_MSG_FLAGS_INVALID_KE) {
+ /* XXX chould also happen for PFS */
+ group = betoh16(msg->msg_group);
+ if (!sa->sa_hdr.sh_initiator) {
+ log_debug("%s: not an initiator", __func__);
+ ikev2_ike_sa_setreason(sa,
+ "received invalid KE as responder");
+ sa_state(env, sa, IKEV2_STATE_CLOSED);
+ msg->msg_sa = NULL;
+ return (-1);
+ }
+ if (group_getid(group) == NULL) {
+ log_debug("%s: unable to select DH group %u", __func__,
+ group);
+ ikev2_ike_sa_setreason(sa,
+ "unable to select DH group");
+ sa_state(env, sa, IKEV2_STATE_CLOSED);
+ msg->msg_sa = NULL;
+ return (-1);
+ }
+ msg->msg_policy->pol_peerdh = group;
+ log_debug("%s: responder selected DH group %u", __func__,
+ group);
+ ikev2_ike_sa_setreason(sa,
+ "reinitiating with new DH group");
+ sa_state(env, sa, IKEV2_STATE_CLOSED);
+ msg->msg_sa = NULL;
+
+ /*
+ * XXX should also happen for PFS so we have to check state.
+ */
+ timer_set(env, &env->sc_inittmr, ikev2_init_ike_sa, NULL);
+ timer_add(env, &env->sc_inittmr, IKED_INITIATOR_INITIAL);
+ return (-1);
+ }
+
+ if (msg->msg_flags & IKED_MSG_FLAGS_IPCOMP_SUPPORTED) {
+ /* we only support deflate */
+ if ((msg->msg_policy->pol_flags & IKED_POLICY_IPCOMP) &&
+ (msg->msg_transform == IKEV2_IPCOMP_DEFLATE)) {
+ sa->sa_ipcomp = msg->msg_transform;
+ sa->sa_cpi_out = betoh16(msg->msg_cpi);
+ }
+ }
+
+ if (msg->msg_nat_detected & IKED_MSG_NAT_DST_IP) {
+ /* Send keepalive, since we are behind a NAT-gw */
+ sa->sa_usekeepalive = 1;
+ }
+
+ /* Signature hash algorithm */
+ if (msg->msg_flags & IKED_MSG_FLAGS_SIGSHA2)
+ sa->sa_sigsha2 = 1;
+ return (0);
+}
+
int
ikev2_resp_ike_sa_init(struct iked *env, struct iked_message *msg)
{
@@ -2735,6 +2847,52 @@ ikev2_send_init_error(struct iked *env, struct
iked_message *msg)
return (ret);
}
+int
+ikev2_handle_certreq(struct iked* env, struct iked_message *msg)
+{
+ struct iked_sa *sa;
+
+ if ((sa = msg->msg_sa) == NULL)
+ return (-1);
+
+ if (msg->msg_certreq_ocsp != NULL)
+ ca_setreq(env, sa, &sa->sa_policy->pol_localid,
+ IKEV2_CERT_OCSP,
+ ibuf_data(msg->msg_certreq_auth),
+ ibuf_length(msg->msg_certreq_auth),
+ PROC_CERT);
+
+ if (msg->msg_certreq_type == IKEV2_CERT_NONE)
+ goto done;
+
+ /* Optional certreq for PSK */
+ if (sa->sa_hdr.sh_initiator)
+ sa->sa_stateinit |= IKED_REQ_CERT;
+ else
+ sa->sa_statevalid |= IKED_REQ_CERT;
+
+ ca_setreq(env, sa, &sa->sa_policy->pol_localid,
+ msg->msg_certreq_type,
+ ibuf_data(msg->msg_certreq_auth),
+ ibuf_length(msg->msg_certreq_auth),
+ PROC_CERT);
+
+ done:
+ /* Cleanup */
+ if (msg->msg_certreq_auth != NULL) {
+ ibuf_release(msg->msg_certreq_auth);
+ msg->msg_certreq_auth = NULL;
+ }
+ if (msg->msg_certreq_ocsp != NULL) {
+ ibuf_release(msg->msg_certreq_ocsp);
+ msg->msg_certreq_ocsp = NULL;
+ }
+ msg->msg_certreq_type = IKEV2_CERT_NONE;
+
+ return (0);
+}
+
+
int
ikev2_resp_ike_auth(struct iked *env, struct iked_sa *sa)
{
diff --git a/sbin/iked/ikev2_pld.c b/sbin/iked/ikev2_pld.c
index e92bd3be2a7..166a157b9e8 100644
--- a/sbin/iked/ikev2_pld.c
+++ b/sbin/iked/ikev2_pld.c
@@ -792,7 +792,8 @@ ikev2_pld_cert(struct iked *env, struct ikev2_payload *pld,
certid = &msg->msg_parent->msg_cert;
if (certid->id_type) {
- log_debug("%s: duplicate cert payload", __func__);
+ log_info("%s: multiple cert payloads not supported",
+ SPI_SA(msg->msg_sa, __func__));
return (-1);
}
@@ -826,7 +827,6 @@ int
ikev2_pld_certreq(struct iked *env, struct ikev2_payload *pld,
struct iked_message *msg, size_t offset, size_t left)
{
- struct iked_sa *sa = msg->msg_sa;
struct ikev2_cert cert;
uint8_t *buf;
ssize_t len;
@@ -848,25 +848,35 @@ ikev2_pld_certreq(struct iked *env, struct ikev2_payload
*pld,
return (0);
if (cert.cert_type == IKEV2_CERT_X509_CERT) {
- if (!len)
+ if (len == 0) {
+ log_info("%s: invalid length 0", __func__);
return (0);
+ }
if ((len % SHA_DIGEST_LENGTH) != 0) {
- log_debug("%s: invalid certificate request", __func__);
+ log_info("%s: invalid certificate request",
+ __func__);
return (-1);
}
}
- if (msg->msg_sa == NULL)
- return (-1);
-
- /* Optional certreq for PSK */
- if (sa->sa_hdr.sh_initiator)
- sa->sa_stateinit |= IKED_REQ_CERT;
- else
- sa->sa_statevalid |= IKED_REQ_CERT;
-
- ca_setreq(env, sa, &sa->sa_policy->pol_localid,
- cert.cert_type, buf, len, PROC_CERT);
+ /* OCSP is special and appears in addition to other cert types */
+ if (cert.cert_type == IKEV2_CERT_OCSP &&
+ msg->msg_parent->msg_certreq_ocsp == NULL) {
+ if ((msg->msg_parent->msg_certreq_ocsp =
+ ibuf_new(buf, len)) == NULL) {
+ log_info("%s: failed to get buf", __func__);
+ return (-1);
+ }
+ } else if (msg->msg_parent->msg_certreq_auth == NULL) {
+ if ((msg->msg_parent->msg_certreq_auth =
+ ibuf_new(buf, len)) == NULL) {
+ log_info("%s: failed to get buf", __func__);
+ return (-1);
+ }
+ msg->msg_parent->msg_certreq_type = cert.cert_type;
+ } else {
+ log_info("%s: duplicate certreq ignored.", __func__);
+ }
return (0);
}
@@ -989,10 +999,7 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
uint64_t spi64;
struct iked_spi *rekey;
uint16_t type;
- uint16_t group;
- uint16_t cpi;
uint16_t signature_hash;
- uint8_t transform;
if (ikev2_validate_notify(msg, offset, left, &n))
return (-1);
@@ -1025,11 +1032,12 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
if (memcmp(buf, md, len) != 0) {
log_debug("%s: %s detected NAT", __func__,
print_map(type, ikev2_n_map));
- msg->msg_parent->msg_nat_detected = 1;
- /* Send keepalive, since we are behind a NAT-gw */
- if (msg->msg_sa != NULL &&
- type == IKEV2_N_NAT_DETECTION_DESTINATION_IP)
- msg->msg_sa->sa_usekeepalive = 1;
+ if (type == IKEV2_N_NAT_DETECTION_SOURCE_IP)
+ msg->msg_parent->msg_nat_detected
+ |= IKED_MSG_NAT_SRC_IP;
+ else
+ msg->msg_parent->msg_nat_detected
+ |= IKED_MSG_NAT_DST_IP;
}
print_hex(md, 0, sizeof(md));
/* remember for MOBIKE */
@@ -1061,11 +1069,8 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
return (-1);
}
}
- log_debug("%s: AUTHENTICATION_FAILED, closing SA", __func__);
- ikev2_ike_sa_setreason(msg->msg_sa,
- "authentication failed notification from peer");
- sa_state(env, msg->msg_sa, IKEV2_STATE_CLOSED);
- msg->msg_sa = NULL;
+ msg->msg_parent->msg_flags
+ |= IKED_MSG_FLAGS_AUTHENTICATION_FAILED;
break;
case IKEV2_N_INVALID_KE_PAYLOAD:
if (sa_stateok(msg->msg_sa, IKEV2_STATE_VALID) &&
@@ -1074,36 +1079,14 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
__func__);
return (-1);
}
- if (len != sizeof(group)) {
+ if (len != sizeof(msg->msg_parent->msg_group)) {
log_debug("%s: malformed payload: group size mismatch"
- " (%zu != %zu)", __func__, len, sizeof(group));
- return (-1);
- }
- /* XXX chould also happen for PFS */
- if (!msg->msg_sa->sa_hdr.sh_initiator) {
- log_debug("%s: not an initiator", __func__);
- sa_state(env, msg->msg_sa, IKEV2_STATE_CLOSED);
- msg->msg_sa = NULL;
- return (-1);
- }
- memcpy(&group, buf, len);
- group = betoh16(group);
- if (group_getid(group) == NULL) {
- log_debug("%s: unable to select DH group %u", __func__,
- group);
+ " (%zu != %zu)", __func__, len,
+ sizeof(msg->msg_parent->msg_group));
return (-1);
}
- msg->msg_policy->pol_peerdh = group;
- log_debug("%s: responder selected DH group %u", __func__,
- group);
- sa_state(env, msg->msg_sa, IKEV2_STATE_CLOSED);
- msg->msg_sa = NULL;
-
- /*
- * XXX should also happen for PFS so we have to check state.
- */
- timer_set(env, &env->sc_inittmr, ikev2_init_ike_sa, NULL);
- timer_add(env, &env->sc_inittmr, IKED_INITIATOR_INITIAL);
+ memcpy(&msg->msg_parent->msg_group, buf, len);
+ msg->msg_parent->msg_flags |= IKED_MSG_FLAGS_INVALID_KE;
break;
case IKEV2_N_NO_ADDITIONAL_SAS:
if (!msg->msg_e) {
@@ -1111,11 +1094,7 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
__func__);
return (-1);
}
- /* This makes sense for Child SAs only atm */
- if (msg->msg_sa->sa_stateflags & IKED_REQ_CHILDSA) {
- ikev2_disable_rekeying(env, msg->msg_sa);
- msg->msg_sa->sa_stateflags &= ~IKED_REQ_CHILDSA;
- }
+ msg->msg_parent->msg_flags |= IKED_MSG_FLAGS_NO_ADDITIONAL_SAS;
break;
case IKEV2_N_REKEY_SA:
if (!msg->msg_e) {
@@ -1159,21 +1138,25 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
__func__);
return (-1);
}
- if (len < sizeof(cpi) + sizeof(transform)) {
+ if (len < sizeof(msg->msg_parent->msg_cpi) +
+ sizeof(msg->msg_parent->msg_transform)) {
log_debug("%s: ignoring malformed ipcomp notification",
__func__);
return (0);
}
- memcpy(&cpi, buf, sizeof(cpi));
- memcpy(&transform, buf + sizeof(cpi), sizeof(transform));
- log_debug("%s: cpi 0x%x, transform %s, len %zu", __func__,
- betoh16(cpi), print_map(transform, ikev2_ipcomp_map), len);
- /* we only support deflate */
- if ((msg->msg_policy->pol_flags & IKED_POLICY_IPCOMP) &&
- (transform == IKEV2_IPCOMP_DEFLATE)) {
- msg->msg_sa->sa_ipcomp = transform;
- msg->msg_sa->sa_cpi_out = betoh16(cpi);
- }
+ memcpy(&msg->msg_parent->msg_cpi, buf,
+ sizeof(msg->msg_parent->msg_cpi));
+ memcpy(&msg->msg_parent->msg_transform,
+ buf + sizeof(msg->msg_parent->msg_cpi),
+ sizeof(msg->msg_parent->msg_transform));
+
+ log_debug("%s: %s cpi 0x%x, transform %s, len %zu", __func__,
+ msg->msg_parent->msg_response ? "res" : "req",
+ betoh16(msg->msg_parent->msg_cpi),
+ print_map(msg->msg_parent->msg_transform,
+ ikev2_ipcomp_map), len);
+
+ msg->msg_parent->msg_flags |= IKED_MSG_FLAGS_IPCOMP_SUPPORTED;
break;
case IKEV2_N_CHILD_SA_NOT_FOUND:
if (!msg->msg_e) {
@@ -1181,8 +1164,7 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
__func__);
return (-1);
}
- /* Stop expecting response */
- msg->msg_sa->sa_stateflags &= ~IKED_REQ_CHILDSA;
+ msg->msg_parent->msg_flags |= IKED_MSG_FLAGS_CHILD_SA_NOT_FOUND;
break;
case IKEV2_N_MOBIKE_SUPPORTED:
if (!msg->msg_e) {
@@ -1195,13 +1177,7 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
" notification: %zu", __func__, len);
return (0);
}
- if (!env->sc_mobike) {
- log_debug("%s: mobike disabled", __func__);
- return (0);
- }
- msg->msg_sa->sa_mobike = 1;
- /* enforce natt */
- msg->msg_sa->sa_natt = 1;
+ msg->msg_parent->msg_flags |= IKED_MSG_FLAGS_MOBIKE;
break;
case IKEV2_N_UPDATE_SA_ADDRESSES:
if (!msg->msg_e) {
@@ -1276,11 +1252,7 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
" notification: %zu", __func__, len);
return (0);
}
- if (!env->sc_frag) {
- log_debug("%s: fragmentation disabled", __func__);
- return (0);
- }
- msg->msg_sa->sa_frag = 1;
+ msg->msg_parent->msg_flags |= IKED_MSG_FLAGS_FRAGMENTATION;
break;
case IKEV2_N_SIGNATURE_HASH_ALGORITHMS:
if (msg->msg_e) {
@@ -1309,7 +1281,8 @@ ikev2_pld_notify(struct iked *env, struct ikev2_payload
*pld,
len -= sizeof(signature_hash);
buf += sizeof(signature_hash);
if (signature_hash == IKEV2_SIGHASH_SHA2_256)
- msg->msg_sa->sa_sigsha2 = 1;
+ msg->msg_parent->msg_flags
+ |= IKED_MSG_FLAGS_SIGSHA2;
}
break;
}
