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

[Ondrej Mosnacek <omosnace@redhat.com>]

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).

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.

--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.