On Mon, Aug 31, 2009 at 10:18:14AM -0400, Sebastien Roy wrote:
<mucho snippage deleted!>
> > > > * 949-991: The locking here might be fine, but I can't make
> > heads
> > > > or tails of it. Seems like a lot of IPsec machinery is
> > directly
> > > > exposed to consumers.
> > >
> > > DEFER: There is quite a bit of direct manipulation of internal data
> > > structures from the IPsec implementation here, but I don't think there's
> > > much that I can do about it short of implementing a higher level kernel
> > > API for this stuff (there doesn't appear to be one).
> >
> > OK. In general, it seems like an IPsec API layer is missing.
>
> Is seems so, yes.
There *is* an API layer for per-endpoint policy (IP_SEC_OPT socket option) --
and indeed prior to IPsec Tunnel Reform, this codepath took the passed-in
ipsr and rebundled it as a T_OPTMGMT_REQ to be passed to IP.
Post-Tunnel-Reform, policy enforcement had to be moved up into the
decapsulation path, because for Tunnel-Mode protection, enforcement decisions
require knowlege of inner-packet fields. The fanout logic in IP for the
outer-header doesn't know if the tunnel is enforcing full Tunnel Mode, or
not.
It was a question of where to put the mess, and I decided in Tunnel Reform to
put the mess in the tunnelling code. The API you desire can't be used.
You could move iptun_set_sec_simple() and friends out of iptun.c, but then
someone would complain that iptun internals are in the IPsec code. Someone
must lose. (If you really think it needs relocation, put it in spd.c.)
> > > > * 1079-1092: Punting this to the hapless caller via EAGAIN
> > > > complicates the interface with an implementation artifact.
> > > > Seems like another mechanism is needed here. (IIRC, GLDv3
> > > > ioctls are not allowed to block, which means the mechanism
> > > > might need to be quite a bit different.)
> > >
> > > DEFER: I've brought this up with the IPsec team and Erik Nordmark, and
> > > the consensus was that since we'll never run into this problem (you need
> > > IPsec keys and policy to use IPsec tunnels, and the creation of those
> > > objects will have loaded IPsec before tunnels are created), it was not
> > > worth my re-architecting how IPsec is loaded at this point.
> >
> > If we can never get here, then I'd think this would be a panic(). If we
> > can get here, then is EAGAIN really good enough?
>
> We can get here, but only under contrived scenarios. You'd have to set
> IPsec policy on the tunnel without having created any IPsec keys nor
> started IKE nor loaded any other global IPsec policy. When we create
> tunnels during boot, those other things are done first. When IPsec
> tunnels are created by something like a VPN, those things are also done
> first. It would only be in a case where someone is playing around and
> doing things out of order that this would come up. They'd get EAGAIN,
> and indeed, trying again would work.
Boot-time dependencies should be (Seb, I asked you this explicitly offline)
such that EAGAIN doesn't happen during boot-time.
Here's how you could get EAGAIN:
1.) Boot system with NO IPsec policy.
2.) Before doing anything else, plumb a tunnel like so:
ifconfig ip.tun0 plumb foo bar tsrc o-foo tdst o-bar \
encr_algs aes encr_auth_algs sha512 up
You can reduce the chances of EAGAIN happening by waiting for the loader
thread to finish (say a kernel-sleep of 100-500ms) and checking
ipsec_failed() and ipsec_loaded() a second time.
Ultimately, we want to reduce the cruft in ifconfig(1M), right? We might as
well not worry too much about the path that only occurs via ifconfig(1M)
features that in the medium- or long-term we wish to discard.
Dan
