On 2017/06/25 21:44, Tim Stewart wrote:
> Hi,
>
> In this message I've tried to encode everything I've done to allow
> strongSwan on Android to connect with iked, including the latest patch.
> I have also verified that it breaks neither initial negotiation nor
> Child SA rekeying for OpenBSD, Windows, and strongSwan (on Android)
> clients.
>
> Stuart Henderson <[email protected]> writes:
>
> > On 2017/05/22 01:52, Tim Stewart wrote:
> >> Hello again,
> >>
> >> Tim Stewart <[email protected]> writes:
> >>
> >> > Tim Stewart <[email protected]> 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.
> >>
> >> After re-reading relevant parts of the RFC I'm not convinced that my fix
> >> (rejecting with INVALID_KE_PAYLOAD unless msg->msg_dhgroup is
> >> IKEV2_XFORMDH_NONE) is correct. It happens to resolve my local issue
> >> but I think it may accidentally work due to a side effect of the code
> >> path for rekeying a child SA.
> >>
> >> I will look at it more closely this week.
>
> My first patch did, in fact, break Child SAs rekeying. I have a new
> patch at the end of this message that simply restricts DH group
> negotiation to IKE SAs (I *think* that DH group guessing only applies to
> IKE SAs, and perhaps only the IKE_SA_INIT exchange, but I'm still
> working through the RFC). This may not be the ultimate solution, but it
> does allow us to move forward.
>
> > Hi, I'm interested in this, but wasn't able to get strongswan to connect
> > with either of your patches (and had iked exiting on one attempt, though
> > I haven't been able to repeat that).
>
> After making changes I usually rebuild all of /usr/src/sbin/iked/ (make
> clean && make). I started doing this after experiencing similar exits
> and the problem went away. That could just be a coincidence, though!
>
> > If you have any updates please do send them here, it can be a bit slow
> > getting feedback on iked diffs at times but it definitely is worth sending
> > them out.
>
> I'll summarize what I know about strongSwan (on Android) and iked
> interop. strongSwan chooses ECP_256 for its DH group guess when it
> starts the IKE_SA_INIT exchange. The negotiation gets pretty far if I
> specify `group ecp256' in the ikesa directive, but there there is some
> incompatibility between iked and strongSwan that causes the following:
>
> ...
> ikev2_recv: IKE_AUTH request from initiator 5.6.7.8:49317 to 1.2.3.4:4500
> policy 'default' id 1, 2040 bytes
> ikev2_recv: ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8
> ikev2_recv: updated SA to peer 5.6.7.8:49317 local 1.2.3.4:4500
> ikev2_pld_parse: header ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8
> nextpayload SK version 0x20 exchange IKE_AUTH flags 0x08 msgid 1 length 2040
> response 0
> ikev2_pld_payloads: payload SK nextpayload IDi critical 0x00 length 2012
> ikev2_msg_decrypt: IV length 16
> ikev2_msg_decrypt: encrypted payload length 1968
> ikev2_msg_decrypt: integrity checksum length 24
> ikev2_msg_decrypt: integrity check failed
> ikev2_resp_recv: failed to parse message
> ...
>
> I suspect this is the failure many people hit if they try to configure
> iked to match strongSwan on Android's built-in settings. I don't know
> what the issue is with ecp256, but I needed to use modp2048 anyway so I
> decided to write this patch.
Presumably due to the commit on 27 Oct, this part of things is now fixed.
With the following in config ...
ikesa auth hmac-sha2-256 enc aes-256 prf hmac-sha2-256 group ecp256
... I am now able to connect from the stock version of strongSwan in the
play store.
> I also configured iked to match strongSwan's built-in configuration for
> Child SA rekeying. Many other cipher combinations result in
> NO_PROPOSAL_CHOSEN when iked initiates the Child SA rekey and strongSwan
> (usually) decides to rebuild the whole SA, which is awkward and slow
> (and not always successful). Here is the configuration I've settled on:
>
> ikev2 "strongswan" passive esp \
> from 0.0.0.0/0 to 10.1.1.50 \
> local em2 peer any \
> ikesa auth hmac-sha2-384 enc aes-256 prf hmac-sha2-384 group modp2048 \
> childsa enc chacha20-poly1305 group modp3072 \
> srcid "/C=US/ST=New York/L=New York City/O=Stoo
> Labs/OU=iked/CN=foo.example.com/[email protected]" \
> dstid "bar.stoo.org" \
> rsa \
> config address 10.1.1.50 \
> config name-server 10.1.1.8 \
> config name-server 10.1.1.9 \
> tag "$name-$id"
>
> Some notes:
> - em2 is my Internet-facing IPv4 interface.
> - In my case, strongSwan presents the dstid as a FQDN, not an ASN1_DN.
> I'm using certificates generated using `ikectl ca'.
> - `childsa enc chacha20-poly1305 group modp3072' is accpeted by
> strongSwan on Android for rekeying Child SAs. It took me a few tries
> to find a compatible set of parameters here.
>
>
> Finally, the latest patch against -current:
This patch still applies without fuzz / line changes, and I'm able to connect
with your suggested config:
ikesa auth hmac-sha2-384 enc aes-256 prf hmac-sha2-384 group modp2048 \
childsa enc chacha20-poly1305 group modp3072 \
I'm unable to connect if I keep the default iked.conf transforms - is that
expected?
Ideally I'd like to allow multiple options for pfs for clients, so that
windows can use it's supported-by-default modp1024, and other clients
get something better, but afaict the only way to configure a set of
multiple alternatives on the iked side is by using the default values
for ikesa/childsa. So from that perspective, it's appealing to be able
to use default config.
I can't test further today as I hit a kernel hang and need to get a
console cable moved before I can reach my remote endpoint again, not
that this should be anything to do with your diff :-) (Machine pings
but doesn't accept TCP connections -> feels like a kernel deadlock).
Patch verbatim from your mail below (I just stripped the email quote
characters so it's easier for others to apply). There don't seem to
be any real downsides to this, and I think that we're probably at a
good point in the release cycle to get this committed, does anyone
else have opinions on this or is able to review the diff?
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 26 Jun 2017 01:30:22 -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.155
diff -u -p -r1.155 ikev2.c
--- ikev2.c 1 Jun 2017 15:23:43 -0000 1.155
+++ ikev2.c 26 Jun 2017 01:30:22 -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, 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,8 +3477,8 @@ 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,
- msg->msg_parent, protoid) < 0) {
+ if (ikev2_sa_responder_dh(env, kex, &proposals,
+ msg->msg_parent, protoid, 1) < 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, int child_sa)
{
struct iked_transform *xform;
@@ -4121,6 +4202,18 @@ ikev2_sa_responder_dh(struct iked_kex *k
}
}
+ /* Look for dhgroup mismatch during an IKE SA negotiation */
+ if (!child_sa && 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_dhrexchange)) {
if ((kex->kex_dhrexchange = ibuf_new(NULL,
dh_getlen(kex->kex_dhgroup))) == NULL) {
@@ -4226,7 +4319,8 @@ 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) < 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 26 Jun 2017 01:30:22 -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);
