We send certificate request (CERTREQ) payloads to notify the peer which CAs or
public key schemes we accept in the authentication.
Our current behaviour is incorrect when multiple policies between the same two
hosts use different kinds of raw public key authentication.
At this point of the exchange the responder can only guess which policy the
message belongs to, because the ID used to match a policy is not yet known,
and temporarily assumes the last matching policy.
It then sends the certificate request payload for this policy,
which may not necessarily be one that is valid for the actual policy of the
exchange and leads to an error in the peer who doesn't support the requested
method.
The diff fixes the problem by sending CERTREQs for all matching policies
instead.
ok?
Index: ikev2.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sbin/iked/ikev2.c,v
retrieving revision 1.183
diff -u -p -r1.183 ikev2.c
--- ikev2.c 3 Dec 2019 12:38:34 -0000 1.183
+++ ikev2.c 5 Dec 2019 14:45:38 -0000
@@ -122,6 +122,8 @@ int ikev2_valid_proposal(struct iked_pr
int ikev2_handle_notifies(struct iked *, struct iked_message *);
+ssize_t ikev2_resp_add_certreqs(struct iked *, struct iked_message *,
+ struct ibuf *, struct ikev2_payload **, ssize_t );
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 *);
@@ -2636,14 +2638,13 @@ ikev2_resp_ike_sa_init(struct iked *env,
goto done;
}
if (sa->sa_statevalid & IKED_REQ_CERT) {
- /* CERTREQ payload(s) */
- if ((len = ikev2_add_certreq(buf, &pld,
- len, env->sc_certreq, env->sc_certreqtype)) == -1)
- goto done;
-
- if (env->sc_certreqtype != sa->sa_policy->pol_certreqtype &&
- (len = ikev2_add_certreq(buf, &pld,
- len, NULL, sa->sa_policy->pol_certreqtype)) == -1)
+ /*
+ * At this stage it is unclear which policy this SA belongs
+ * to because the ID payload is sent in IKE_AUTH.
+ * Add CERTREQ payloads for all matching policies.
+ */
+ if ((len = ikev2_resp_add_certreqs(env, msg, buf, &pld,
+ len)) == -1)
goto done;
}
@@ -2671,6 +2672,85 @@ ikev2_resp_ike_sa_init(struct iked *env,
ikev2_msg_cleanup(env, &resp);
return (ret);
+}
+
+ssize_t
+ikev2_resp_add_certreqs(struct iked *env, struct iked_message *msg,
+ struct ibuf *buf, struct ikev2_payload **pld, ssize_t len)
+{
+ struct iked_policy key, *p;
+ int ecdsa_sent = 0;
+ int rsa_sent = 0;
+
+ /* Request at least own certreqtype */
+ switch (env->sc_certreqtype) {
+ case IKEV2_CERT_ECDSA:
+ ecdsa_sent = 1;
+ break;
+ case IKEV2_CERT_RSA_KEY:
+ rsa_sent = 1;
+ break;
+ default:
+ break;
+ }
+ if ((len = ikev2_add_certreq(buf, pld,
+ len, env->sc_certreq, env->sc_certreqtype)) == -1)
+ return (-1);
+
+ /* Add CERTREQS for all matching policies */
+ bzero(&key, sizeof(key));
+ key.pol_af = msg->msg_peer.ss_family;
+ memcpy(&key.pol_peer.addr, &msg->msg_peer, sizeof(msg->msg_peer));
+ memcpy(&key.pol_local.addr, &msg->msg_local, sizeof(msg->msg_local));
+
+ p = TAILQ_FIRST(&env->sc_policies);
+ while (p != NULL) {
+ if (p->pol_flags & IKED_POLICY_SKIP)
+ p = p->pol_skip[IKED_SKIP_FLAGS];
+ else if (key.pol_af && p->pol_af &&
+ key.pol_af != p->pol_af)
+ p = p->pol_skip[IKED_SKIP_AF];
+ else if (key.pol_ipproto && p->pol_ipproto &&
+ key.pol_ipproto != p->pol_ipproto)
+ p = p->pol_skip[IKED_SKIP_PROTO];
+ else if (sockaddr_cmp((struct sockaddr *)&key.pol_peer.addr,
+ (struct sockaddr *)&p->pol_peer.addr,
+ p->pol_peer.addr_mask) != 0)
+ p = p->pol_skip[IKED_SKIP_DST_ADDR];
+ else if (sockaddr_cmp((struct sockaddr *)&key.pol_local.addr,
+ (struct sockaddr *)&p->pol_local.addr,
+ p->pol_local.addr_mask) != 0)
+ p = p->pol_skip[IKED_SKIP_SRC_ADDR];
+ else {
+ /* Policy matches */
+ switch (p->pol_certreqtype) {
+ case IKEV2_CERT_ECDSA:
+ if (ecdsa_sent == 1) {
+ p = TAILQ_NEXT(p, pol_entry);
+ continue;
+ }
+ ecdsa_sent = 1;
+ break;
+ case IKEV2_CERT_RSA_KEY:
+ if (rsa_sent == 1) {
+ p = TAILQ_NEXT(p, pol_entry);
+ continue;
+ }
+ rsa_sent = 1;
+ break;
+ default:
+ break;
+ }
+ if (p->pol_certreqtype &&
+ (len = ikev2_add_certreq(buf, pld,
+ len, NULL, p->pol_certreqtype)) == -1)
+ return (-1);
+
+ p = TAILQ_NEXT(p, pol_entry);
+ }
+ }
+
+ return len;
}
int
