Hi,
currently some parts of the iked(8) payload parser act on the SA structure
before the message was fully parsed and checked, which is obviously bad for
security.
The parser should store it's results in a temporary location
(e.G. in the iked_message struct) and apply changes only when the message
has passed all checks.
This diff moves the CERTREQ payload handling from the parser to the IKE_AUTH
exchange handler function where it belongs.
Ok?
Index: iked.h
===================================================================
RCS file: /mount/openbsd/cvs/src/sbin/iked/iked.h,v
retrieving revision 1.121
diff -u -p -u -r1.121 iked.h
--- iked.h 11 May 2019 16:30:23 -0000 1.121
+++ iked.h 9 Aug 2019 13:41:00 -0000
@@ -536,6 +536,9 @@ struct iked_message {
struct iked_id msg_cert;
struct ibuf *msg_cookie;
+ uint8_t msg_certreq_type; /* CR Encoding*/
+ struct ibuf *msg_certreq_auth; /* CR Authority */
+
/* MOBIKE */
int msg_update_sa_addresses;
struct ibuf *msg_cookie2;
Index: ikev2.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sbin/iked/ikev2.c,v
retrieving revision 1.171
diff -u -p -u -r1.171 ikev2.c
--- ikev2.c 11 May 2019 16:30:23 -0000 1.171
+++ ikev2.c 9 Aug 2019 13:59:53 -0000
@@ -82,6 +82,8 @@ int ikev2_send_error(struct iked *, str
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 *);
@@ -608,6 +610,9 @@ ikev2_ike_auth_recv(struct iked *env, st
struct iked_policy *policy = sa->sa_policy;
int ret = -1;
+ if (ikev2_handle_certreq(env, msg) != 0)
+ return (-1);
+
if (sa->sa_hdr.sh_initiator) {
id = &sa->sa_rid;
certid = &sa->sa_rcert;
@@ -2996,6 +3001,40 @@ ikev2_set_sa_proposal(struct iked_sa *sa
return (-1);
}
}
+ return (0);
+}
+
+int
+ikev2_handle_certreq(struct iked* env, struct iked_message *msg)
+{
+ struct iked_sa *sa;
+
+ if (msg->msg_certreq_type == IKEV2_CERT_NONE)
+ return (0);
+
+ log_info("%s: handling certreq with type %d.", __func__,
+ msg->msg_certreq_type);
+
+ if ((sa = 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,
+ msg->msg_certreq_type,
+ ibuf_data(msg->msg_certreq_auth),
+ ibuf_length(msg->msg_certreq_auth),
+ PROC_CERT);
+
+ /* Cleanup */
+ ibuf_release(msg->msg_certreq_auth);
+ msg->msg_certreq_auth = NULL;
+ msg->msg_certreq_type = IKEV2_CERT_NONE;
+
return (0);
}
Index: ikev2_pld.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sbin/iked/ikev2_pld.c,v
retrieving revision 1.71
diff -u -p -u -r1.71 ikev2_pld.c
--- ikev2_pld.c 11 May 2019 16:30:23 -0000 1.71
+++ ikev2_pld.c 9 Aug 2019 13:44:07 -0000
@@ -826,7 +826,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;
@@ -847,27 +846,30 @@ ikev2_pld_certreq(struct iked *env, stru
if (!ikev2_msg_frompeer(msg))
return (0);
- if (cert.cert_type == IKEV2_CERT_X509_CERT) {
- if (!len)
- return (0);
- if ((len % SHA_DIGEST_LENGTH) != 0) {
- log_debug("%s: invalid certificate request", __func__);
- return (-1);
- }
+ /* The contents of the Certification Authority field are defined only
+ * for X.509 certificates */
+ switch (cert.cert_type) {
+ case IKEV2_CERT_X509_CERT:
+ case IKEV2_CERT_HASHURL_X509:
+ case IKEV2_CERT_HASHURL_X509_BUNDLE:
+ if (len == 0)
+ return (0);
+ if ((len % SHA_DIGEST_LENGTH) != 0) {
+ log_debug("%s: invalid certificate request",
+ __func__);
+ return (-1);
+ }
+ if ((msg->msg_parent->msg_certreq_auth =
+ ibuf_new(buf, len)) == NULL)
+ return (-1);
+ msg->msg_parent->msg_certreq_type = cert.cert_type;
+ break;
+ default:
+ if (len != 0)
+ return (0);
+ msg->msg_parent->msg_certreq_type = cert.cert_type;
+ break;
}
-
- 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);
-
return (0);
}
