On Tue, May 14, 2024 at 7:29 AM Ondrej Mosnacek [off-list ref] wrote:

On Thu, Apr 25, 2024 at 11:48 PM Paul Moore [off-list ref] wrote:

quoted

On Wed, Apr 17, 2024 at 9:03 AM Ondrej Mosnacek [off-list ref] wrote:

quoted

On Tue, Apr 16, 2024 at 8:39 PM Paul Moore [off-list ref] wrote:

quoted

On Apr 16, 2024 Ondrej Mosnacek [off-list ref] wrote:

quoted

As the comment in this function says, the code currently just clears the
CIPSO part with IPOPT_NOP, rather than removing it completely and
trimming the packet. This is inconsistent with the other
cipso_v4_*_delattr() functions and with CALIPSO (IPv6).

This sentence above implies an equality in handling between those three
cases that doesn't exist.  IPv6 has a radically different approach to
IP options, comparisions between the two aren't really valid.

I don't think it's that radically different.

They are very different in my mind.  The IPv4 vs IPv6 option format
and handling should be fairly obvious and I'm sure there are plenty of
things written that describe the differences and motivations in
excruciating detail so I'm not going to bother trying to do that here;
as usual, Google is your friend.  I will admit that the skbuff vs
socket-based labeling differences are a bit more subtle, but I believe
if you look at how the packets are labeled in the two approaches as
well as how they are managed and hooked into the LSMs you will start
to get a better idea.  If that doesn't convince you that these three
cases are significantly different, I'm not sure what else I can say
other than we have a difference of opinion.  Regardless, I stand by my
original comment that I don't like the text you chose and would like
you to remove or change it.

Ok, I amended this part for v2 to hopefully better express what I'm
alluding to. I also added a paragraph about the routers dropping
packets with IP options, which explains the motivation better, anyway.

Okay, I'll refrain from further comment until I see the v2 patch.

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?

-- 
paul-moore.com