Re: [PATCH 2/2] cipso: make cipso_v4_skbuff_delattr() fully remove the CIPSO options

[Paul Moore <paul@paul-moore.com>]

On Fri, Jun 7, 2024 at 11:47 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Fri, May 17, 2024 at 9:49 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Tue, May 14, 2024 at 7:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > I tried to test what you describe - hopefully I got close enough:
> > >
> > > My test setup has 3 VMs (running Fedora 39 from the stock qcow2 image)
> > > A, B, and R, connected via separate links as A <--> R <--> B, where R
> > > acts as a router between A and B (net.ipv4.ip_forward is set to 1 on
> > > R, and the appropriate routes are set on A, B, R).
> > >
> > > The A <--> R link has subnet 10.123.123.0/24, A having address
> > > 10.123.123.2 and R having 10.123.123.1.
> > > The B <--> R link has subnet 10.123.124.0/24, B having address
> > > 10.123.124.2 and R having 10.123.124.1.
> > >
> > > The links are implemented as GRE tunnels over the main network that is
> > > shared between the VMs.
> > >
> > > Netlabel configuration on A:
> > > netlabelctl cipsov4 add pass doi:16 tags:5
> > > netlabelctl map del default
> > > netlabelctl map add default address:0.0.0.0/0 protocol:unlbl
> > > netlabelctl map add default address:::/0 protocol:unlbl
> > > netlabelctl map add default address:10.123.123.0/24 protocol:cipsov4,16
> > > netlabelctl map add default address:10.123.124.0/24 protocol:cipsov4,16
> > >
> > > Netlabel configuration on R:
> > > netlabelctl cipsov4 add pass doi:16 tags:5
> > > netlabelctl map del default
> > > netlabelctl map add default address:0.0.0.0/0 protocol:unlbl
> > > netlabelctl map add default address:::/0 protocol:unlbl
> > > netlabelctl map add default address:10.123.123.0/24 protocol:cipsov4,16
> > >
> > > B has no netlabel configured.
> > >
> > > (I.e. A tries to send CIPSO-labeled packets to B, but R treats the
> > > 10.123.124.0/24 network as unlabeled, so should strip/add the CIPSO
> > > label when forwarding between A and B.)
> > >
> > > A basic TCP connection worked just fine in both directions with and
> > > without these patches applied (I installed the patched kernel on all
> > > machines, though it should only matter on machine R). I ignored the
> > > actual labels/CIPSO content - i.e. I didn't change the default SELinux
> > > policy and put SELinux into permissive mode on machines A and R.
> > >
> > > Capturing the packets on R showed the following IP option content
> > > without the patches:
> > > A -> R: CIPSO
> > > R -> B: NOPs
> > > B -> R: (empty)
> > > R -> A: CIPSO
> > >
> > > With the patches this changed to:
> > > A -> R: CIPSO
> > > R -> B: (empty)
> > > B -> R: (empty)
> > > R -> A: CIPSO
> > >
> > > Is this convincing enough or do you have different scenarios in mind?
> >
> > Thanks for verifying your patch, the methodology looks good to me, but
> > as I mentioned in my previous email I would feel much better if you
> > verified this with a different client OS/stack.  Do you have access to
> > Windows/MacOS/BSD/non-Linux system you could use in place of B in your
> > test above?
>
> I don't think I can easily plug that into the framework I used for the
> testing (there doesn't seem to be a convenient way to install a
> FreeBSD VM without manual interaction and the rest is proprietary).

Surely you can perform a manual unit test with some VMs on your local
machine if whatever test automation you are using doesn't support
this.

> I still don't quite understand what exactly you expect to break under
> that scenario and why - could you elaborate on that? If anything, I'd
> expect the IP header growing along the path (which already happens
> pretty much by design in the opposite direction) to be more likely to
> cause an issue.

I'm concerned about potential oddities caused by the changes in IP
header sizes while the packet is in flight.  Every OS's network stack
is a bit different and I don't think it is too much to ask to test at
least one non-Linux network stack as a client.

-- 
paul-moore.com