Re: [PATCH net-next v4 0/6] ipsec: add TCP encapsulation support (RFC 8229)

[Sabrina Dubroca <sd@queasysnail.net>]

2019-10-15, 11:46:57 -0700, Jakub Kicinski wrote:
> On Tue, 15 Oct 2019 10:24:24 +0200, Sabrina Dubroca wrote:
> > 2019-10-14, 14:43:27 -0400, David Miller wrote:
> > > From: Sabrina Dubroca <sd@queasysnail.net>
> > > Date: Fri, 11 Oct 2019 16:57:23 +0200
> > >   
> > > > This patchset introduces support for TCP encapsulation of IKE and ESP
> > > > messages, as defined by RFC 8229 [0]. It is an evolution of what
> > > > Herbert Xu proposed in January 2018 [1] that addresses the main
> > > > criticism against it, by not interfering with the TCP implementation
> > > > at all. The networking stack now has infrastructure for this: TCP ULPs
> > > > and Stream Parsers.  
> > > 
> > > So this will bring up a re-occurring nightmare in that now we have another
> > > situation where stacking ULPs would be necessary (kTLS over TCP encap) and
> > > the ULP mechanism simply can't do this.
> > > 
> > > Last time this came up, it had to do with sock_map.  No way could be found
> > > to stack ULPs properly, so instead sock_map was implemented via something
> > > other than ULPs.
> > > 
> > > I fear we have the same situation here again and this issue must be
> > > addressed before these patches are included.
> > > 
> > > Thanks.  
> > 
> > I don't think there's any problem here. We're not stacking ULPs on the
> > same socket. There's a TCP encap socket for IPsec, which belongs to
> > the IKE daemon. The traffic on that socket is composed of IKE messages
> > and ESP packets. Then there's whatever userspace sockets (doesn't have
> > to be TCP), and the whole IPsec and TCP encap is completely invisible
> > to them.
> > 
> > Where we would probably need ULP stacking is if we implement ESP over
> > TLS [1], but we're not there.
> > 
> > [1] https://tools.ietf.org/html/rfc8229#appendix-A
> 
> But can there be any potential issues if the TCP socket with esp ULP is
> also inserted into a sockmap? (well, I think sockmap socket gets a ULP,
> I think we prevent sockmap on top of ULP but not the other way around..)

Yeah, there's nothing preventing a socket that's already in a sockmap
from getting a ULP, only for inserting a socket in a sockmap if it
already has a ULP (see sock_map_update_common).

I gave it a quick test with espintcp, it doesn't quite seem to work: a
sockmap program that drops everything actually drops messages, but a
sockmap program that drops some messages based on length... doesn't.

Although, to be honest, I don't see a use case for sockmap on espintcp
sockets.

> Is there any chance we could see some selftests here?

For espintcp? That's planned, I need to rework my test scripts so that
they don't need human interaction, and turn them into selftests.

-- 
Sabrina