Apologies for disappearing for a while.  I was moving across town and I
had to drop many things!

Stuart Henderson <s...@spacehopper.org> 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 <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.
>
> 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 expect you're right about this.  It's nice to see the ultimate cause
resolved :)

>> 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:
>
> 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?

(snipped remainder of email/patch)

It seems that Patrick Wildt grabbed the reins and drove this forward, so
I don't think there's much to say here now.  I'm going to update to
-current and see how things go.

Thanks for testing, Stuart!

-TimS

--
Tim Stewart
-----------
Mail:   t...@stoo.org
Matrix: @tim:stoo.org

Reply via email to