Tim Stewart <t...@stoo.org> writes:

> This patch teaches iked to reject a KE with a Notify payload of type
> INVALID_KE_PAYLOAD when the KE uses a different Diffie-Hellman group
> than is configured locally.  The rejection indicates the desired
> group.
>
> In my environment, this patch allows stock strongSwan on Android from
> the Google Play store to interop with iked.  strongSwan's logs show
> the following once iked is patched:
>
>   [IKE] initiating IKE_SA android[7] to 192.0.2.1
>   [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) N(NATD_D_IP) 
> N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ]
>   [ENC] parsed IKE_SA_INIT response 0 [ N(INVAL_KE) ]
>   [IKE] peer didn't accept DH group ECP_256, it requested MODP_2048
>   [IKE] initiating IKE_SA android[7] to 192.0.2.1
>   [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) N(NATD_D_IP) 
> N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ]
>   [ENC] parsed IKE_SA_INIT response 0 [ SA KE No N(NATD_S_IP) N(NATD_D_IP) 
> CERTREQ N(HASH_ALG) ]
>
> I'm happy to iterate on this patch to get it into proper shape for
> inclusion.

I discovered a bug in the previous patch that broke renegotiation of
CHILD SAs.  I was ignoring "other than NONE" in the following sentence
from RFC 5996 section 3.4:

  If the selected proposal uses a different Diffie-Hellman group (other
  than NONE), the message MUST be rejected with a Notify payload of type
  INVALID_KE_PAYLOAD.

The new patch below repairs the flaw.

-TimS


Index: iked.h
===================================================================
RCS file: /cvs/src/sbin/iked/iked.h,v
retrieving revision 1.115
diff -u -p -r1.115 iked.h
--- iked.h      26 Apr 2017 10:42:38 -0000      1.115
+++ iked.h      22 May 2017 05:29:17 -0000
@@ -502,6 +502,7 @@ struct iked_message {
        struct iked_proposals    msg_proposals;
        struct iked_spi          msg_rekey;
        struct ibuf             *msg_nonce;     /* dh NONCE */
+       uint16_t                 msg_dhgroup;   /* dh group */
        struct ibuf             *msg_ke;        /* dh key exchange */
        struct iked_id           msg_auth;      /* AUTH payload */
        struct iked_id           msg_id;
Index: ikev2.c
===================================================================
RCS file: /cvs/src/sbin/iked/ikev2.c,v
retrieving revision 1.154
diff -u -p -r1.154 ikev2.c
--- ikev2.c     26 Apr 2017 10:42:38 -0000      1.154
+++ ikev2.c     22 May 2017 05:29:18 -0000
@@ -71,6 +71,8 @@ int    ikev2_init_done(struct iked *, stru
 void    ikev2_resp_recv(struct iked *, struct iked_message *,
            struct ike_header *);
 int     ikev2_resp_ike_sa_init(struct iked *, struct iked_message *);
+int     ikev2_resp_ike_invalid_ke(struct iked *, struct iked_message *,
+           struct iked_kex *);
 int     ikev2_resp_ike_auth(struct iked *, struct iked_sa *);
 int     ikev2_resp_ike_eap(struct iked *, struct iked_sa *, struct ibuf *);
 int     ikev2_send_auth_failed(struct iked *, struct iked_sa *);
@@ -96,8 +98,8 @@ int    ikev2_sa_responder(struct iked *, s
            struct iked_message *);
 int     ikev2_sa_initiator_dh(struct iked_sa *, struct iked_message *,
            unsigned int);
-int     ikev2_sa_responder_dh(struct iked_kex *, struct iked_proposals *,
-           struct iked_message *, unsigned int);
+int     ikev2_sa_responder_dh(struct iked *, struct iked_kex *,
+           struct iked_proposals *, struct iked_message *, unsigned int);
 void    ikev2_sa_cleanup_dh(struct iked_sa *);
 int     ikev2_sa_keys(struct iked *, struct iked_sa *, struct ibuf *);
 int     ikev2_sa_tag(struct iked_sa *, struct iked_id *);
@@ -2279,6 +2281,84 @@ ikev2_resp_ike_sa_init(struct iked *env,
 }
 
 int
+ikev2_resp_ike_invalid_ke(struct iked *env, struct iked_message *msg,
+    struct iked_kex *kex)
+{
+       struct iked_message              resp;
+       struct ike_header               *hdr;
+       struct ikev2_payload            *pld;
+       struct ikev2_notify             *n;
+       struct iked_sa                  *sa = msg->msg_sa;
+       struct ibuf                     *buf;
+       uint8_t                         *ptr;
+       ssize_t                          len;
+       uint16_t                         group;
+       int                              ret = -1;
+
+       if (sa->sa_hdr.sh_initiator) {
+               log_debug("%s: called by initiator", __func__);
+               return (-1);
+       }
+
+       log_debug("%s: rejecting with INVALID_KE_PAYLOAD", __func__);
+
+       if ((buf = ikev2_msg_init(env, &resp,
+           &msg->msg_peer, msg->msg_peerlen,
+           &msg->msg_local, msg->msg_locallen, 1)) == NULL)
+               goto done;
+
+       resp.msg_sa = sa;
+       resp.msg_fd = msg->msg_fd;
+       resp.msg_natt = msg->msg_natt;
+       resp.msg_msgid = 0;
+
+       /* IKE header */
+       if ((hdr = ikev2_add_header(buf, sa, resp.msg_msgid,
+           IKEV2_PAYLOAD_NOTIFY, IKEV2_EXCHANGE_IKE_SA_INIT,
+           IKEV2_FLAG_RESPONSE)) == NULL)
+               goto done;
+
+       /* NOTIFY payload */
+       if ((pld = ikev2_add_payload(buf)) == NULL)
+               goto done;
+       if ((n = ibuf_advance(buf, sizeof(*n))) == NULL)
+               goto done;
+       n->n_protoid = 0;
+       n->n_spisize = 0;
+       n->n_type = htobe16(IKEV2_N_INVALID_KE_PAYLOAD);
+
+       /* add desired dh group to NOTIFY */
+       group = htobe16(kex->kex_dhgroup->id);
+       len = sizeof(group);
+       if ((ptr = ibuf_advance(buf, len)) == NULL)
+               goto done;
+       memcpy(ptr, &group, sizeof(group));
+       len += sizeof(*n);
+
+       if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_NONE) == -1)
+               goto done;
+
+       if (ikev2_set_header(hdr, ibuf_size(buf) - sizeof(*hdr)) == -1)
+               goto done;
+
+       (void)ikev2_pld_parse(env, hdr, &resp, 0);
+
+       ibuf_release(sa->sa_2ndmsg);
+       if ((sa->sa_2ndmsg = ibuf_dup(buf)) == NULL) {
+               log_debug("%s: failed to copy 2nd message", __func__);
+               goto done;
+       }
+
+       resp.msg_sa = NULL;     /* Don't save the response */
+       ret = ikev2_msg_send(env, &resp);
+
+done:
+       ikev2_msg_cleanup(env, &resp);
+
+       return (ret);
+}
+
+int
 ikev2_send_auth_failed(struct iked *env, struct iked_sa *sa)
 {
        struct ikev2_notify             *n;
@@ -3397,7 +3477,7 @@ ikev2_resp_create_child_sa(struct iked *
                /* check KE payload for PFS */
                if (ibuf_length(msg->msg_parent->msg_ke)) {
                        log_debug("%s: using PFS", __func__);
-                       if (ikev2_sa_responder_dh(kex, &proposals,
+                       if (ikev2_sa_responder_dh(env, kex, &proposals,
                            msg->msg_parent, protoid) < 0) {
                                log_debug("%s: failed to setup DH", __func__);
                                goto fail;
@@ -4102,8 +4182,9 @@ ikev2_sa_initiator(struct iked *env, str
 }
 
 int
-ikev2_sa_responder_dh(struct iked_kex *kex, struct iked_proposals *proposals,
-    struct iked_message *msg, unsigned int proto)
+ikev2_sa_responder_dh(struct iked *env, struct iked_kex *kex,
+    struct iked_proposals *proposals, struct iked_message *msg,
+    unsigned int proto)
 {
        struct iked_transform   *xform;
 
@@ -4134,6 +4215,19 @@ ikev2_sa_responder_dh(struct iked_kex *k
                }
        }
 
+       /* Look for dhgroup mismatch */
+       if (msg->msg_dhgroup != IKEV2_XFORMDH_NONE &&
+           msg->msg_dhgroup != kex->kex_dhgroup->id) {
+               log_debug("%s: want dh %s, KE has %s", __func__,
+                   print_map(kex->kex_dhgroup->id, ikev2_xformdh_map),
+                   print_map(msg->msg_dhgroup, ikev2_xformdh_map));
+               if (ikev2_resp_ike_invalid_ke(env, msg,
+                   kex) != 0)
+                       log_debug("%s: failed to send init response",
+                           __func__);
+               return (-1);
+       }
+
        if (!ibuf_length(kex->kex_dhiexchange)) {
                if ((kex->kex_dhiexchange = ibuf_dup(msg->msg_ke)) == NULL ||
                    ((ssize_t)ibuf_length(kex->kex_dhiexchange) !=
@@ -4226,7 +4320,7 @@ ikev2_sa_responder(struct iked *env, str
                }
        }
 
-       if (ikev2_sa_responder_dh(&sa->sa_kex, &sa->sa_proposals, msg, 0) < 0)
+       if (ikev2_sa_responder_dh(env, &sa->sa_kex, &sa->sa_proposals, msg, 0) 
< 0)
                return (-1);
 
        return (ikev2_sa_keys(env, sa, osa ? osa->sa_key_d : NULL));
Index: ikev2_pld.c
===================================================================
RCS file: /cvs/src/sbin/iked/ikev2_pld.c,v
retrieving revision 1.62
diff -u -p -r1.62 ikev2_pld.c
--- ikev2_pld.c 13 Apr 2017 07:04:09 -0000      1.62
+++ ikev2_pld.c 22 May 2017 05:29:18 -0000
@@ -680,6 +680,7 @@ ikev2_pld_ke(struct iked *env, struct ik
        log_debug("%s: dh group %s reserved %d", __func__,
            print_map(betoh16(kex.kex_dhgroup), ikev2_xformdh_map),
            betoh16(kex.kex_reserved));
+       msg->msg_dhgroup = betoh16(kex.kex_dhgroup);
 
        buf = msgbuf + offset + sizeof(kex);
        len = betoh16(pld->pld_length) - sizeof(*pld) - sizeof(kex);

Reply via email to