[Swan-dev] whack [-oppotproto <protocol>]

[Andrew Cagney]

On Tue, 9 Feb 2021 at 22:50, Paul Wouters <paul at nohats.ca> wrote:
>
> On Tue, 9 Feb 2021, Andrew Cagney wrote:
>
> > My understanding of the kernel's opportunistic code is that the
> > following happens:
>
> I think you need to separate ondemand and opportunistic.....
>
> > - something tries send a packet to LOCAL
> > - the packet is captured and the all the details - version, address,
> > protocol [and port when applicable], ... are all provided to pluto
>
> Yes. and pluto then tries to find a matching connection.
>
> > - the details are formed into endpoints and then fed into
> > build_outgoing_opportunistic_connection() which finds and instantiates
> > a connection
>
> Only if the best match found was an opportunistic group.
>
> > the big takeaway is that two endpoints precisely define the
> > source/dest for the triggering packet - nothing left to chance (well
> > ok, for ICMP some information is currently lost :-().
>
> ICMP types are piggyback'ed into "ports". Blame the RFC, not the kernel.

Capturing the information provided by the kernel and feeding it,
meaningful, into pluto is our problem.

>
> > However, when initiating using whack and the options:
> >
> >        { "oppohere", required_argument, NULL, OPT_OPPO_HERE + OO },
> >        { "oppothere", required_argument, NULL, OPT_OPPO_THERE + OO },
> >        { "oppoproto", required_argument, NULL, OPT_OPPO_PROTO + OO },
> >        { "oppodport", required_argument, NULL, OPT_OPPO_DPORT + OO },
>
> oppohere is our IP, oppothere is peer IP. Oppoproto and oppodort are
> for OE connections with protoport selectors.
>
> > (whack --help needs another update)
> > (no there isn't oppoSport and what's with oppoDport vs oppoTHERE?)
>
> opphere/oppothere are 25 years old. oppoproto/oppodport just a few
> years. You match destination port and protocol, not source port, because
> well, source ports for that tend to be ephemeral. And we don't have OE
> support for that because basically all those single port outgoing
> connections, dont really care about source port.

The command should be injecting the exact same information that we'd
otherwise get from the kernel but because it doesn't ...

> > build_outgoing_opportunistic_connection() ends having to also handle
> > what I think is best described as a homeopathic memory of the kernel
> > trigger.
> >
> > I'd like to see this changed so that the whack command, like for the
> > kernel, exactly specifies the trigger.  Is it safe to assume this is
> > ok and we've a fairly free hand here?  Any suggestions?
>
> There is no point doing this really. The whack oppohere/oppothere and
> oppoproto/oppodport are mostly for debugging and test cases. The code
> paths for "packet triggered" and "whack triggered" are already a bit
> different. I wouldn't build more cards on top.

... we've ended up with this pile of ...

                                if (!routed(sr->routing) ||
                                    !addrinsubnet(our_client,
&sr->this.client) ||
                                    !addrinsubnet(peer_client,
&sr->that.client) ||
                                    ((sr->this.protocol != 0) &&
transport_proto != 0 && sr->this.protocol != transport_proto) ||
                                    ((sr->this.protocol != 0) &&
sr->that.port != 0 && peer_port != sr->that.port) ||
                                    ((sr->this.protocol != 0) &&
sr->this.port != 0 && our_port != sr->this.port)
                                   )

cards.  I'm pretty sure that the above can be reduced to:

    if (!routed(sr->routing) ||
        !endpoint_in_selector(local_client, sr->this.client)) continue
        !endpoint_in_selector(remote_client, sr->that.client)) continue

but this is only true if the whack command does its job and emulates the kernel.

I'm not seeing any reason to not change this.

> What _does_ need fixing, which might help reduce your problem, is that
> we should use the reqid in the ACQUIRE to match packets to inserted
> policies to connections. So each static conn, when ondemand triggers,
> you already know based on reqid which connection to match this to.

Different horses.

Between:
- kernel notifies us of a packet
- an outgoing opportunistic connection gets instantiated
there's this huge black box I'm trying to ignore.

My interest is focused on cleaning up silliness such as:
-               tmp_ip = c->spd.that.host_addr;
-               tmp_ip.version = c->spd.that.host_addr.version;
-               tmp_ip.hport = c->spd.that.host_addr.hport;



> Two dragons there. One, is if you get one conns but multiple ipsec sa's
> from it (eg both sides initaite to each other, and you end up with two
> IKE SA's and two IPsec SA's. Now your reqid look is tricky. We currently
> have a bug with that.
>
> For OE we currently cannot use reqid, because if you insert a policy
> for private-or-clear#192.0.2.0/24 and you get a hit, we install a new
> tunnel policy with a different reqid. That means we didn't overwrite
> the larval state since reqid doesn't match, meaning some new ACQUIREs
> are suppressed for a few seconds. If we use the same reqid then we
> get agaun multiple policies with the same reqid and xfrm state/key
> confusion. We need a way to tell the kernel "this reqid is used for
> a range, but we will handle the ACQUIRE and only install a new policy
> that's narrower. keep sending us ACQUIREs for different tuples in
> that reqid policy.
>
> ExecSum: talk to me please before you make any changes.
>
> Paul