OpenIKED Keepalive Broken

[William Ahern]

The logic of ikev2_ike_sa_alive presumes too much, and breaks NAT traversal
when the _actual_ initiator of real traffic (as opposed to merely being the
SA initiator) is behind NAT.

Background: I currently have an IPSec tunnel configured to provide access to
a corporate network from a remote office. The SA initiator peer on the
corporate nework uses DHCP for its address and is behind a NAT gateway. The
passive peer at the remote office has a static routable address. Currently
I'm using isakmpd and everything has been working relatively fine.

I've been wanting to move to IKEv2 (and iked) for various reasons. But
whenever I tried using iked, the tunnel kept becoming unresponsive almost
immediately. I've spent a couple of days, approximately 200 miles of travel,
and a good deal of my reserve of patience trying to figure out what _I_ was
doing wrong or what idiotic firewall rules IT had setup with their fancy new
deep-packet filtering equipment. (Avoiding IT and DevOps and their
fascination with complex firewall and routing rules being a major reason for
managing my own tunnel.) But now I realize the problem was with iked, and
perhaps another classic case of premature optimization/specialization.

isakmpd unconditionally sends NAT-T keepalive messages every 30 seconds,
whereas iked's ikev2_ike_sa_alive only sends a keepalive message iff
`!foundin && foundout`. But that presumes that the SA initiator is also the
initiator of traffic, which definitely isn't the case in my situation, and
seems dubious and unreliable even for real road warriors.

Everything worked again with this simple patch:

diff -u -p -r1.128 ikev2.c
--- ikev2.c 22 Oct 2015 15:55:18 -0000 1.128
+++ ikev2.c 6 Aug 2016 00:02:55 -0000
@@ -3270,7 +3270,7 @@ ikev2_ike_sa_alive(struct iked *env, voi
}

/* send probe if any outging SA has been used, but no incoming SA */
- if (!foundin && foundout) {
+ if (1 || (!foundin && foundout)) {

I'd be happy to create a proper patch if someone could explain the purpose
of the conditional logic. I wouldn't want to accidentally break something.

I also wouldn't mind making the keepalive interval configurable--rather than
a compile-time constant--so users could deal with NAT gateways which
aggressively flush state.