Re: Add Diffie-Hellman group negotiation to iked
Patrick Wildt writes:
> On Mon, Nov 27, 2017 at 06:12:22PM +0100, Patrick Wildt wrote:
>> On Mon, Nov 27, 2017 at 04:21:08PM +0100, Patrick Wildt wrote:
>> > On Wed, Nov 22, 2017 at 05:26:24PM +0100, Patrick Wildt wrote:
>> > > On 2017/06/25 21:44, Tim Stewart wrote:
>> > > > 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.
>> > >
>> > > Reading RFC 7296 it looks like throwing INVALID_KE_PAYLOAD is fine for
>> > > both establishing the IKE SA and rekeying the Child SAs. If we select a
>> > > proposal from the msg that uses a different DH group than the one that's
>> > > used in the KEi (in the same msg) we need to throw INVALID_KE_PAYLOAD.
>> > >
>> > > Since all messages subsequent to the initial exchange must be encrypted,
>> > > the INVALID_KE_PAYLOAD message on rekeying Child SAs must be encrypted.
>> > > Apparently with the previous diff the Child SA rekeying failed. This is
>> > > because the code sends the INVALID_KE_PAYLOAD response unencrypted.
>> > >
>> > > Also I have found inconsistencies in handling INVALID_KE_PAYLOAD with us
>> > > acting as initiator. I will take a look at both cases and will follow
>> > > up.
>> > >
>> > > Patrick
>> >
>> > Hi,
>> >
>> > This diff adds suport to rejecting IKE SA messages. This means that we
>> > can reply to IKE SA INIT messages with no proposal chosen, as we already
>> > do for Child SAs. For that the error "adding" is done in a new function
>> > shared by both send error handlers. We need two "send error" functions
>> > because the init error is unencrypted, while all later ones are not.
>> > Now we can add more cases, like Child SA not found or that the DH group
>> > is not what we expect. I think that is quite an improvement.
>> >
>> > One "issue" is retransmits. The RFC specifies that IKEv2 is a reliable
>> > protocol. When a packet is lost, the initiator has to retransmit the
>> > request and the responder must retransmit the saved reply. On IKE SA
>> > INIT error we don't save our response, which means that we will handle
>> > the request as if we never had it. The RFC says we shouldn't do that,
>> > because DoS protection and so on. What we should do is keep our reply
>> > around and check if it is indeed a retransmit. If so we reply with the
>> > same reply, thus saving resources. If it is an attempt to create a new
>> > IKE SA, drop the old SA so that iked(8) can attempt to create a new SA.
>> >
>> > Thoughts?
>> >
>> > Patrick
>
> Updated diff also responds with INVALID_KE_PAYLOAD when the responder
> has a lower SA lifetime and initiates the CHILD_SA_CREATE.
>
> Handling rejections (as in receiving INVALID_KE_PAYLOAD) is still a ToDo
> I want to tackle next.
Patrick, thanks for running with this. I see from the CVS log that a
version of this patch and other patches have been applied. I will
update my system and do some testing.
> ok?
>
> diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
> index a0059682bc8..01f8ac00506 100644
> --- a/sbin/iked/iked.h
> +++ b/sbin/iked/iked.h
> @@ -508,6 +508,7 @@ struct iked_message {
> struct iked_proposalsmsg_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;
> diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
> index b505f763153..b8cef9061fc 100644
> --- a/sbin/iked/ikev2.c
> +++ b/sbin/iked/ikev2.c
> @@ -76,6 +76,7 @@ int ikev2_resp_ike_eap(struct iked *, struct iked_sa *,
> struct ibuf *);
> int ikev2_send_auth_failed(struct iked *, struct iked_sa *);
> int ikev2_send_error(struct iked *, struct iked_sa *,
> struct iked_message *, uint8_t);
> +int ikev2_send_init_error(struct iked *, struct iked_message *);
>
> int ikev2_send_create_child_sa(struct iked *, struct iked_sa *,
> struct iked_spi *, uint8_t);
> @@ -125,6 +126,7 @@ ssize_tikev2_add_certreq(struct ibuf *, struct
> ikev2_payload **, ssize_t,
> ssize_t ikev2_add_ipcompnotify(struct iked *, struct ibuf *,
> struct ikev2_payload **, ssize_t, struct iked_sa *);
> ssize_t ikev2_add_ts_payload(struct ibuf *, unsigned int, struct
> iked_sa *);
> +ssize_t ikev2_add_error(struct iked *, struct ibuf *, struct
> iked_message *);
> int ikev2_add_data(struct ibuf *, void *, size_t);
> int ikev2_add_buf(struct ibuf *buf, struct ibuf *);
>
> @@ -455,6 +457,23 @@ ikev2_rec
Re: Add Diffie-Hellman group negotiation to iked
Apologies for disappearing for a while. I was moving across town and I had to drop many things! Stuart Henderson writes: > 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 writes: >> >> > On 2017/05/22 01:52, Tim Stewart wrote: >> >> Hello again, >> >> >> >> Tim Stewart writes: >> >> >> >> > Tim Stewart 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, bu
Re: Add Diffie-Hellman group negotiation to iked
On Mon, Nov 27, 2017 at 06:12:22PM +0100, Patrick Wildt wrote:
> On Mon, Nov 27, 2017 at 04:21:08PM +0100, Patrick Wildt wrote:
> > On Wed, Nov 22, 2017 at 05:26:24PM +0100, Patrick Wildt wrote:
> > > On 2017/06/25 21:44, Tim Stewart wrote:
> > > > 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.
> > >
> > > Reading RFC 7296 it looks like throwing INVALID_KE_PAYLOAD is fine for
> > > both establishing the IKE SA and rekeying the Child SAs. If we select a
> > > proposal from the msg that uses a different DH group than the one that's
> > > used in the KEi (in the same msg) we need to throw INVALID_KE_PAYLOAD.
> > >
> > > Since all messages subsequent to the initial exchange must be encrypted,
> > > the INVALID_KE_PAYLOAD message on rekeying Child SAs must be encrypted.
> > > Apparently with the previous diff the Child SA rekeying failed. This is
> > > because the code sends the INVALID_KE_PAYLOAD response unencrypted.
> > >
> > > Also I have found inconsistencies in handling INVALID_KE_PAYLOAD with us
> > > acting as initiator. I will take a look at both cases and will follow
> > > up.
> > >
> > > Patrick
> >
> > Hi,
> >
> > This diff adds suport to rejecting IKE SA messages. This means that we
> > can reply to IKE SA INIT messages with no proposal chosen, as we already
> > do for Child SAs. For that the error "adding" is done in a new function
> > shared by both send error handlers. We need two "send error" functions
> > because the init error is unencrypted, while all later ones are not.
> > Now we can add more cases, like Child SA not found or that the DH group
> > is not what we expect. I think that is quite an improvement.
> >
> > One "issue" is retransmits. The RFC specifies that IKEv2 is a reliable
> > protocol. When a packet is lost, the initiator has to retransmit the
> > request and the responder must retransmit the saved reply. On IKE SA
> > INIT error we don't save our response, which means that we will handle
> > the request as if we never had it. The RFC says we shouldn't do that,
> > because DoS protection and so on. What we should do is keep our reply
> > around and check if it is indeed a retransmit. If so we reply with the
> > same reply, thus saving resources. If it is an attempt to create a new
> > IKE SA, drop the old SA so that iked(8) can attempt to create a new SA.
> >
> > Thoughts?
> >
> > Patrick
Updated diff also responds with INVALID_KE_PAYLOAD when the responder
has a lower SA lifetime and initiates the CHILD_SA_CREATE.
Handling rejections (as in receiving INVALID_KE_PAYLOAD) is still a ToDo
I want to tackle next.
ok?
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index a0059682bc8..01f8ac00506 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -508,6 +508,7 @@ struct iked_message {
struct iked_proposalsmsg_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;
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index b505f763153..b8cef9061fc 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -76,6 +76,7 @@ intikev2_resp_ike_eap(struct iked *, struct iked_sa *,
struct ibuf *);
int ikev2_send_auth_failed(struct iked *, struct iked_sa *);
int ikev2_send_error(struct iked *, struct iked_sa *,
struct iked_message *, uint8_t);
+int ikev2_send_init_error(struct iked *, struct iked_message *);
int ikev2_send_create_child_sa(struct iked *, struct iked_sa *,
struct iked_spi *, uint8_t);
@@ -125,6 +126,7 @@ ssize_t ikev2_add_certreq(struct ibuf *, struct
ikev2_payload **, ssize_t,
ssize_t ikev2_add_ipcompnotify(struct iked *, struct ibuf *,
struct ikev2_payload **, ssize_t, struct iked_sa *);
ssize_t ikev2_add_ts_payload(struct ibuf *, unsigned int, struct
iked_sa *);
+ssize_t ikev2_add_error(struct iked *, struct ibuf *, struct
iked_message *);
int ikev2_add_data(struct ibuf *, void *, size_t);
int ikev2_add_buf(struct ibuf *buf, struct ibuf *);
@@ -455,6 +457,23 @@ ikev2_recv(struct iked *env, struct iked_message *msg)
if ((m = ikev2_msg_lookup(env, &sa->sa_requests, msg, hdr)))
ikev2_msg_dispose(env, &sa->sa_requests, m);
} else {
+ /*
+* IKE_SA_INIT is special since it always uses the message i
Re: Add Diffie-Hellman group negotiation to iked
On Wed, Nov 22, 2017 at 05:26:24PM +0100, Patrick Wildt wrote:
> On 2017/06/25 21:44, Tim Stewart wrote:
> > 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.
>
> Reading RFC 7296 it looks like throwing INVALID_KE_PAYLOAD is fine for
> both establishing the IKE SA and rekeying the Child SAs. If we select a
> proposal from the msg that uses a different DH group than the one that's
> used in the KEi (in the same msg) we need to throw INVALID_KE_PAYLOAD.
>
> Since all messages subsequent to the initial exchange must be encrypted,
> the INVALID_KE_PAYLOAD message on rekeying Child SAs must be encrypted.
> Apparently with the previous diff the Child SA rekeying failed. This is
> because the code sends the INVALID_KE_PAYLOAD response unencrypted.
>
> Also I have found inconsistencies in handling INVALID_KE_PAYLOAD with us
> acting as initiator. I will take a look at both cases and will follow
> up.
>
> Patrick
Hi,
This diff adds suport to rejecting IKE SA messages. This means that we
can reply to IKE SA INIT messages with no proposal chosen, as we already
do for Child SAs. For that the error "adding" is done in a new function
shared by both send error handlers. We need two "send error" functions
because the init error is unencrypted, while all later ones are not.
Now we can add more cases, like Child SA not found or that the DH group
is not what we expect. I think that is quite an improvement.
One "issue" is retransmits. The RFC specifies that IKEv2 is a reliable
protocol. When a packet is lost, the initiator has to retransmit the
request and the responder must retransmit the saved reply. On IKE SA
INIT error we don't save our response, which means that we will handle
the request as if we never had it. The RFC says we shouldn't do that,
because DoS protection and so on. What we should do is keep our reply
around and check if it is indeed a retransmit. If so we reply with the
same reply, thus saving resources. If it is an attempt to create a new
IKE SA, drop the old SA so that iked(8) can attempt to create a new SA.
Thoughts?
Patrick
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index b536d58e157..050277a2530 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -502,6 +502,7 @@ struct iked_message {
struct iked_proposalsmsg_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;
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index e908b6230a5..234f8329791 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -76,6 +76,7 @@ intikev2_resp_ike_eap(struct iked *, struct iked_sa *,
struct ibuf *);
int ikev2_send_auth_failed(struct iked *, struct iked_sa *);
int ikev2_send_error(struct iked *, struct iked_sa *,
struct iked_message *, uint8_t);
+int ikev2_send_init_error(struct iked *, struct iked_message *);
int ikev2_send_create_child_sa(struct iked *, struct iked_sa *,
struct iked_spi *, uint8_t);
@@ -125,6 +126,7 @@ ssize_t ikev2_add_certreq(struct ibuf *, struct
ikev2_payload **, ssize_t,
ssize_t ikev2_add_ipcompnotify(struct iked *, struct ibuf *,
struct ikev2_payload **, ssize_t, struct iked_sa *);
ssize_t ikev2_add_ts_payload(struct ibuf *, unsigned int, struct
iked_sa *);
+ssize_t ikev2_add_error(struct iked *, struct ibuf *, struct
iked_message *);
int ikev2_add_data(struct ibuf *, void *, size_t);
int ikev2_add_buf(struct ibuf *buf, struct ibuf *);
@@ -446,6 +448,23 @@ ikev2_recv(struct iked *env, struct iked_message *msg)
if ((m = ikev2_msg_lookup(env, &sa->sa_requests, msg, hdr)))
ikev2_msg_dispose(env, &sa->sa_requests, m);
} else {
+ /*
+* IKE_SA_INIT is special since it always uses the message id 0.
+* Even when the message was rejected, and the new message has
+* different proposals, the id will be the same. To discern
+* retransmits and new messages, the RFC suggests to compare the
+* the messages.
+*/
+ if (sa->sa_state == IKEV2_STATE_CLOSED && sa->sa_1stmsg &&
+ hdr->ike_exchange == IKEV2_EXCHANGE_IKE_SA_INIT &&
+ msg->msg_msgid == 0 &&
+ (ibuf_length(msg->msg_data) != ibuf_length(sa->s
Re: Add Diffie-Hellman group negotiation to iked
On 2017/06/25 21:44, Tim Stewart wrote: > 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. Reading RFC 7296 it looks like throwing INVALID_KE_PAYLOAD is fine for both establishing the IKE SA and rekeying the Child SAs. If we select a proposal from the msg that uses a different DH group than the one that's used in the KEi (in the same msg) we need to throw INVALID_KE_PAYLOAD. Since all messages subsequent to the initial exchange must be encrypted, the INVALID_KE_PAYLOAD message on rekeying Child SAs must be encrypted. Apparently with the previous diff the Child SA rekeying failed. This is because the code sends the INVALID_KE_PAYLOAD response unencrypted. Also I have found inconsistencies in handling INVALID_KE_PAYLOAD with us acting as initiator. I will take a look at both cases and will follow up. Patrick
Re: Add Diffie-Hellman group negotiation to iked
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 writes: > > > On 2017/05/22 01:52, Tim Stewart wrote: > >> Hello again, > >> > >> Tim Stewart writes: > >> > >> > Tim Stewart 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 ae
Re: Add Diffie-Hellman group negotiation to iked
viq writes: > On 17-07-18 23:20:26, Tim Stewart wrote: >> viq writes: >> >> > On 17-06-25 21:44:24, 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. >> > >> > This patch gets my android phone much closer to being able to negotiate >> > a connection, but there are still issues. Paraphrasing analysis mikeb >> > performed on IRC: >> > android sends incorrect (for us) group, and with this patch we now send >> > a failure message and android retries. But, we don't increment msgid >> > "because we did sa_free and restarted, so we can assume that android >> > thinks that negotiation continues, that's why it re-sends the >> > IKE_SA_INIT" >> >> I'm glad it seems to help, though it's too bad that the patch doesn't >> work completely for you. >> >> I haven't really considered msgids--I'll do some more reading to see >> what I might be missing there. I do know that resending an IKE_SA_INIT >> message with a different DH group is correct, however, and this does >> work on my phone. For your reference, the first line of my strongSwan >> log tells me that I'm using strongSwan 5.5.3 and Android 7.1.1. >> >> I see that you forwarded the iked logs in a reply to this email. Is >> this the full log after a fresh iked startup with no existing SAs? > > This is after a fresh startup, there exists an SA but for a separete > site-to-site config I have in place. If completely fresh logs are > needed I could comment that out. Well, my thinking here was that incorrect policy matching could be confusing the issue. I often find it helpful to comment out other policies to eliminate policy matching as a failure point during testing. >> Also, would it be possible to forward an anonymized config and the >> strongSwan logs so that I can compare to mine? (I can also post my >> logs, but I'll have to do it in the next day or two as I'm out of time >> for today.) > > First, sorry for the delay with replying to this. Second, I'm not sure > how to get to the logs, seeing as I'm using the built-in VPN client that > came with Samsung S8. Oh, for some reason I assumed you were using strongSwan! My mistake. Can you provide a link to some more info about the Samsung S8's built-in client? I haven't had any time since my last post, but I still plan to look into the msgids. -TimS -- Tim Stewart --- Mail: t...@stoo.org Matrix: @tim:stoo.org
Re: Add Diffie-Hellman group negotiation to iked
On 17-07-18 23:20:26, Tim Stewart wrote: > viq writes: > > > On 17-06-25 21:44:24, 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. > > > > This patch gets my android phone much closer to being able to negotiate > > a connection, but there are still issues. Paraphrasing analysis mikeb > > performed on IRC: > > android sends incorrect (for us) group, and with this patch we now send > > a failure message and android retries. But, we don't increment msgid > > "because we did sa_free and restarted, so we can assume that android > > thinks that negotiation continues, that's why it re-sends the > > IKE_SA_INIT" > > I'm glad it seems to help, though it's too bad that the patch doesn't > work completely for you. > > I haven't really considered msgids--I'll do some more reading to see > what I might be missing there. I do know that resending an IKE_SA_INIT > message with a different DH group is correct, however, and this does > work on my phone. For your reference, the first line of my strongSwan > log tells me that I'm using strongSwan 5.5.3 and Android 7.1.1. > > I see that you forwarded the iked logs in a reply to this email. Is > this the full log after a fresh iked startup with no existing SAs? This is after a fresh startup, there exists an SA but for a separete site-to-site config I have in place. If completely fresh logs are needed I could comment that out. > Also, would it be possible to forward an anonymized config and the > strongSwan logs so that I can compare to mine? (I can also post my > logs, but I'll have to do it in the next day or two as I'm out of time > for today.) First, sorry for the delay with replying to this. Second, I'm not sure how to get to the logs, seeing as I'm using the built-in VPN client that came with Samsung S8.
Re: Add Diffie-Hellman group negotiation to iked
viq writes: > On 17-06-25 21:44:24, 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. > > This patch gets my android phone much closer to being able to negotiate > a connection, but there are still issues. Paraphrasing analysis mikeb > performed on IRC: > android sends incorrect (for us) group, and with this patch we now send > a failure message and android retries. But, we don't increment msgid > "because we did sa_free and restarted, so we can assume that android > thinks that negotiation continues, that's why it re-sends the > IKE_SA_INIT" I'm glad it seems to help, though it's too bad that the patch doesn't work completely for you. I haven't really considered msgids--I'll do some more reading to see what I might be missing there. I do know that resending an IKE_SA_INIT message with a different DH group is correct, however, and this does work on my phone. For your reference, the first line of my strongSwan log tells me that I'm using strongSwan 5.5.3 and Android 7.1.1. I see that you forwarded the iked logs in a reply to this email. Is this the full log after a fresh iked startup with no existing SAs? Also, would it be possible to forward an anonymized config and the strongSwan logs so that I can compare to mine? (I can also post my logs, but I'll have to do it in the next day or two as I'm out of time for today.) Good luck! -TimS -- Tim Stewart --- Mail: t...@stoo.org Matrix: @tim:stoo.org
Re: Add Diffie-Hellman group negotiation to iked
And now with log. ikev2_recv: IKE_SA_INIT request from initiator 37.47.4.5:9911 to 31.178.147.125:500 policy 'roadwarrior' id 0, 652 bytes ikev2_recv: ispi 0x5e13d636599e1781 rspi 0x ikev2_policy2id: srcid FQDN/keibi.viq.im length 16 ikev2_pld_parse: header ispi 0x5e13d636599e1781 rspi 0x nextpayload SA version 0x20 exchange IKE_SA_INIT flags 0x08 msgid 0 length 652 response 0 ikev2_pld_payloads: payload SA nextpayload KE critical 0x00 length 244 ikev2_pld_sa: more than one proposal specified ikev2_pld_sa: more 2 reserved 0 length 136 proposal #1 protoid IKE spisize 0 xforms 15 spi 0 ikev2_pld_xform: more 3 reserved 0 length 12 type ENCR id AES_CBC ikev2_pld_attr: attribute type KEY_LENGTH length 256 total 4 ikev2_pld_xform: more 3 reserved 0 length 12 type ENCR id AES_CBC ikev2_pld_attr: attribute type KEY_LENGTH length 128 total 4 ikev2_pld_xform: more 3 reserved 0 length 8 type INTEGR id HMAC_SHA2_512_256 ikev2_pld_xform: more 3 reserved 0 length 8 type INTEGR id HMAC_SHA2_384_192 ikev2_pld_xform: more 3 reserved 0 length 8 type INTEGR id HMAC_SHA2_256_128 ikev2_pld_xform: more 3 reserved 0 length 8 type INTEGR id HMAC_SHA1_96 ikev2_pld_xform: more 3 reserved 0 length 8 type PRF id HMAC_SHA2_512 ikev2_pld_xform: more 3 reserved 0 length 8 type PRF id HMAC_SHA2_384 ikev2_pld_xform: more 3 reserved 0 length 8 type PRF id HMAC_SHA2_256 ikev2_pld_xform: more 3 reserved 0 length 8 type PRF id HMAC_SHA1 ikev2_pld_xform: more 3 reserved 0 length 8 type DH id ikev2_pld_xform: more 3 reserved 0 length 8 type DH id ECP_384 ikev2_pld_xform: more 3 reserved 0 length 8 type DH id ECP_256 ikev2_pld_xform: more 3 reserved 0 length 8 type DH id MODP_2048 ikev2_pld_xform: more 0 reserved 0 length 8 type DH id MODP_1536 ikev2_pld_payloads: payload KE nextpayload NONCE critical 0x00 length 264 ikev2_pld_ke: dh group reserved 0 ikev2_pld_payloads: payload NONCE nextpayload NOTIFY critical 0x00 length 36 ikev2_pld_payloads: payload NOTIFY nextpayload NOTIFY critical 0x00 length 28 ikev2_pld_notify: protoid NONE spisize 0 type NAT_DETECTION_SOURCE_IP ikev2_nat_detection: peer source 0x5e13d636599e1781 0x 37.47.4.5:9911 ikev2_pld_notify: NAT_DETECTION_SOURCE_IP detected NAT, enabling UDP encapsulation ikev2_pld_payloads: payload NOTIFY nextpayload NOTIFY critical 0x00 length 28 ikev2_pld_notify: protoid NONE spisize 0 type NAT_DETECTION_DESTINATION_IP ikev2_nat_detection: peer destination 0x5e13d636599e1781 0x 31.178.147.125:500 ikev2_pld_payloads: payload NOTIFY nextpayload NOTIFY critical 0x00 length 16 ikev2_pld_notify: protoid NONE spisize 0 type SIGNATURE_HASH_ALGORITHMS ikev2_pld_notify: signature hash SHA1 (1) ikev2_pld_notify: signature hash SHA2_256 (2) ikev2_pld_notify: signature hash SHA2_384 (3) ikev2_pld_notify: signature hash SHA2_512 (4) ikev2_pld_payloads: payload NOTIFY nextpayload NONE critical 0x00 length 8 ikev2_pld_notify: protoid NONE spisize 0 type REDIRECT_SUPPORTED sa_state: INIT -> SA_INIT ikev2_sa_negotiate: score 4 sa_stateok: SA_INIT flags 0x, require 0x sa_stateflags: 0x -> 0x0020 sa (required 0x ) ikev2_sa_responder_dh: want dh MODP_2048, KE has ikev2_resp_ike_invalid_ke: rejecting with INVALID_KE_PAYLOAD ikev2_next_payload: length 10 nextpayload NONE ikev2_pld_parse: header ispi 0x5e13d636599e1781 rspi 0x02275a5878cc81a8 nextpayload NOTIFY version 0x20 exchange IKE_SA_INIT flags 0x20 msgid 0 length 38 response 1 ikev2_pld_payloads: payload NOTIFY nextpayload NONE critical 0x00 length 10 ikev2_pld_notify: protoid NONE spisize 0 type INVALID_KE_PAYLOAD ikev2_msg_send: IKE_SA_INIT response from 31.178.147.125:500 to 37.47.4.5:9911 msgid 0, 38 bytes ikev2_resp_recv: failed to get IKE SA keys sa_state: SA_INIT -> CLOSED from any to any policy 'roadwarrior' config_free_proposals: free 0x1825d2eef480 ikev2_recv: IKE_SA_INIT request from initiator 37.47.4.5:9911 to 31.178.147.125:500 policy 'roadwarrior' id 0, 652 bytes ikev2_recv: ispi 0x5e13d636599e1781 rspi 0x ikev2_recv: updated SA to peer 37.47.4.5:9911 local 31.178.147.125:500 ikev2_resp_recv: SA already exists ikev2_recv: closing SA sa_free: ispi 0x5e13d636599e1781 rspi 0x02275a5878cc81a8 config_free_proposals: free 0x1825d2eefe00 ikev2_recv: IKE_SA_INIT request from initiator 37.47.4.5:9911 to 31.178.147.125:500 policy 'roadwarrior' id 0, 652 bytes ikev2_recv: ispi 0x5e13d636599e1781 rspi 0x ikev2_policy2id: srcid FQDN/keibi.viq.im length 16 ikev2_pld_parse: header ispi 0x5e13d636599e1781 rspi 0x nextpayload SA version 0x20 exchange IKE_SA_INIT flags 0x08 msgid 0 length 652 response 0 ikev2_pld_payloads: payload SA nextpayload KE critical 0x00 length 244 ikev2_pld_sa: more than one proposal specified ikev2_pld_sa: more 2 reserved 0 length 136 proposal #1 protoid IKE spisize 0 xforms 15 spi 0 ikev2_pld_xform: more 3 reserved 0 length 12 type ENCR id AES_CBC ikev2_pld_attr: attribute type KEY_LENGTH l
Re: Add Diffie-Hellman group negotiation to iked
On 17-06-25 21:44:24, 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. This patch gets my android phone much closer to being able to negotiate a connection, but there are still issues. Paraphrasing analysis mikeb performed on IRC: android sends incorrect (for us) group, and with this patch we now send a failure message and android retries. But, we don't increment msgid "because we did sa_free and restarted, so we can assume that android thinks that negotiation continues, that's why it re-sends the IKE_SA_INIT" > Stuart Henderson writes: > > > On 2017/05/22 01:52, Tim Stewart wrote: > >> Hello again, > >> > >> Tim Stewart writes: > >> > >> > Tim Stewart 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 faile
Re: Add Diffie-Hellman group negotiation to iked
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 writes: > On 2017/05/22 01:52, Tim Stewart wrote: >> Hello again, >> >> Tim Stewart writes: >> >> > Tim Stewart 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
Re: Add Diffie-Hellman group negotiation to iked
On 2017/05/22 01:52, Tim Stewart wrote: > Hello again, > > Tim Stewart writes: > > > Tim Stewart 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. > > -TimS > > P.S. Is there someone I could add to the To: or Cc: headers of these > iked-related messages? Or should I simply be patient? 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). 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.
Re: Add Diffie-Hellman group negotiation to iked
Hello again, Tim Stewart writes: > Tim Stewart 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. -TimS P.S. Is there someone I could add to the To: or Cc: headers of these iked-related messages? Or should I simply be patient? -- Tim Stewart --- Mail: t...@stoo.org Matrix: @tim:stoo.org
Re: Add Diffie-Hellman group negotiation to iked
Tim Stewart 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 - 1.115
+++ iked.h 22 May 2017 05:29:17 -
@@ -502,6 +502,7 @@ struct iked_message {
struct iked_proposalsmsg_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 - 1.154
+++ ikev2.c 22 May 2017 05:29:18 -
@@ -71,6 +71,8 @@ intikev2_init_done(struct iked *, stru
voidikev2_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 @@ intikev2_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);
voidikev2_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)) == NU
Add Diffie-Hellman group negotiation to iked
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.
-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 - 1.115
+++ iked.h 17 May 2017 05:21:24 -
@@ -502,6 +502,7 @@ struct iked_message {
struct iked_proposalsmsg_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 - 1.154
+++ ikev2.c 17 May 2017 05:21:24 -
@@ -71,6 +71,8 @@ intikev2_init_done(struct iked *, stru
voidikev2_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 @@ intikev2_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);
voidikev2_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
