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 <s...@spacehopper.org> writes:
> On 2017/05/22 01:52, Tim Stewart wrote:
>> Hello again,
>>
>> Tim Stewart <t...@stoo.org> writes:
>>
>> > 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.
>>
>> 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.
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/emailAddress=ad...@example.com" \
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:
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);
