Re: iked(8): move CERTREQ handler out of parser
One more update: I moved to ikev2_handle_certreq() calls somewhere else where
they fit better.
I'd be happy to get some feedback from different iked setups,
in my tests all seems to work as intended. In the best case you
won't notice any difference at all (only the CERTREQ handling will
happen a bit later in recv function).
Here is the diff:
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 - 1.121
+++ iked.h 12 Aug 2019 15:19:56 -
@@ -536,6 +536,10 @@ 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 */
+ struct ibuf *msg_certreq_ocsp; /* CR OCSP values */
+
/* 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 - 1.171
+++ ikev2.c 12 Aug 2019 16:03:46 -
@@ -82,6 +82,8 @@ intikev2_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 *);
@@ -874,6 +876,9 @@ ikev2_init_recv(struct iked *env, struct
"IKE_SA_INIT exchange", __func__);
break;
}
+ if (ikev2_handle_certreq(env, msg) != 0)
+ return;
+
(void)ikev2_init_auth(env, msg);
break;
case IKEV2_EXCHANGE_IKE_AUTH:
@@ -2384,6 +2389,9 @@ ikev2_resp_recv(struct iked *env, struct
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);
@@ -2996,6 +3004,46 @@ 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 ((sa = msg->msg_sa) == NULL)
+ return (-1);
+
+ if (msg->msg_certreq_type != IKEV2_CERT_NONE) {
+ /* 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);
+ }
+
+ 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);
+ }
+
+ /* Cleanup */
+ ibuf_release(msg->msg_certreq_auth);
+ msg->msg_certreq_auth = NULL;
+ ibuf_release(msg->msg_certreq_ocsp);
+ msg->msg_certreq_ocsp = 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 - 1.71
+++ ikev2_pld.c 12 Aug 2019 15:52:02 -
@@ -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_certcert;
uint8_t *buf;
ssize_t len;
@@ -847,27 +846,39 @@ ikev2_pld_certreq(struct iked *env, stru
if (!ikev2_msg_frompeer(msg))
return (0);
- if (cert.cert_type == IKEV2_CERT_X509_CERT) {
Re: iked(8): move CERTREQ handler out of parser
Update because I forgot to add OCSP support which needs a bit of extra
treatment as it uses an additional CERTREQ payload.
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 - 1.121
+++ iked.h 9 Aug 2019 14:20:27 -
@@ -536,6 +536,10 @@ 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 */
+ struct ibuf *msg_certreq_ocsp; /* CR OCSP values */
+
/* 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 - 1.171
+++ ikev2.c 9 Aug 2019 14:23:15 -
@@ -82,6 +82,8 @@ intikev2_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,52 @@ 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);
+
+ if ((sa = msg->msg_sa) == NULL)
+ return (-1);
+
+ if (msg->msg_certreq_type != IKEV2_CERT_NONE) {
+ log_debug("%s: handling certreq with type %d.", __func__,
+ msg->msg_certreq_type);
+
+ /* 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);
+ }
+
+ 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);
+ }
+
+ /* Cleanup */
+ ibuf_release(msg->msg_certreq_auth);
+ msg->msg_certreq_auth = NULL;
+ ibuf_release(msg->msg_certreq_ocsp);
+ msg->msg_certreq_ocsp = 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 - 1.71
+++ ikev2_pld.c 9 Aug 2019 14:24:15 -
@@ -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_certcert;
uint8_t *buf;
ssize_t len;
@@ -847,27 +846,39 @@ 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_BU
iked(8): move CERTREQ handler out of parser
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 - 1.121
+++ iked.h 9 Aug 2019 13:41:00 -
@@ -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 - 1.171
+++ ikev2.c 9 Aug 2019 13:59:53 -
@@ -82,6 +82,8 @@ intikev2_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 - 1.71
+++ ikev2_pld.c 9 Aug 2019 13:44:07 -
@@ -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_certcert;
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__);
+
