NPF: TCP options

[Maxime Villard]

In NPF we don't check the length of the TCPOPT_MAXSEG and TCPOPT_WINDOW
options. That's a problem, if the length is bogus we should ignore these
options, just like the kernel does in tcp_dooptions().

It seems to me one could bypass max-mss clamping, by for example giving

nptr[0] = TCPOPT_MAXSEG
nptr[1] = TCPOLEN_MAXSEG + 1 = 5
nptr[2,3] = the maxseg option
nptr[4] = TCPOPT_EOL
nptr[5] = TCPOPT_MAXSEG
nptr[6] = TCPOLEN_MAXSEG = 4
nptr[7,8] = the maxseg option

NPF will see the two first options here, and will stop iterating after
TCPOPT_EOL. The kernel, however, won't see TCPOPT_EOL, and will handle the
third option.

I've written [1], which fixes that. Basically we fetch nptr[1] (length),
sanitize it, and then use it to ignore options with the incorrect length.

Maxime

[1] http://m00nbsd.net/garbage/npf/tcpopt.diff

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Joerg Sonnenberger]

Post by Maxime Villard
In NPF we don't check the length of the TCPOPT_MAXSEG and TCPOPT_WINDOW
options. That's a problem, if the length is bogus we should ignore these
options, just like the kernel does in tcp_dooptions().

I don't think so. A firewall should drop bogus stuff.

Joerg

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Maxime Villard]

Post by Joerg Sonnenberger

Post by Maxime Villard
In NPF we don't check the length of the TCPOPT_MAXSEG and TCPOPT_WINDOW
options. That's a problem, if the length is bogus we should ignore these
options, just like the kernel does in tcp_dooptions().

I don't think so. A firewall should drop bogus stuff.

In fact, it _may_ not be correct to drop here. I did give a look at the RFCs
about this (~two weeks ago), and I also looked at FreeBSD, OpenBSD and Linux;
the RFC does not specify the behavior here, and everybody ignores options
with "bogus" lengths without dropping the packet. That's what we've been doing
for a long time too, not sure it is correct to divert from this behavior.

(I say "bogus", but it's not inherently buggy, it's just an unusual size.)

Maxime

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Maxime Villard]

Post by Maxime Villard

Post by Joerg Sonnenberger

Post by Maxime Villard
In NPF we don't check the length of the TCPOPT_MAXSEG and TCPOPT_WINDOW
options. That's a problem, if the length is bogus we should ignore these
options, just like the kernel does in tcp_dooptions().

I don't think so. A firewall should drop bogus stuff.

In fact, it _may_ not be correct to drop here. I did give a look at the RFCs
about this (~two weeks ago), and I also looked at FreeBSD, OpenBSD and Linux;
the RFC does not specify the behavior here, and everybody ignores options
with "bogus" lengths without dropping the packet. That's what we've been doing
for a long time too, not sure it is correct to divert from this behavior.
(I say "bogus", but it's not inherently buggy, it's just an unusual size.)
Maxime

Small correction:

"That's what we've been doing for a long time too, not sure it is
correct to divert from this behavior."

Here, by "we", I'm talking about the kernel, not NPF. NPF does not give a look
at the length.

Maxime

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Maxime Villard]

Answering in this thread now, to prevent further confusions.

So NPF's behavior should be aligned to that of the kernel; that is to
say, NPF should ignore TCP options with uncommon lengths - which does not
mean dropping the packet. (We can discuss about changing the kernel's
behavior to be that of NPF, but as I said in my answer to Joerg, the
kernel's behavior is the one that is the most "common".)

Not exactly, no. NPF is not a host/kernel. It is a man in the middle,
concerning packets sent by different hosts (which might have different
TCP/IP implementations and applications). It operates based on its own
set of rules.

It sounds like you didn't understand my point.

I'm saying that the TCP-options behavior in the NetBSD kernel and NPF is not
the same. There is a divergence. Since there is a divergence, it is possible
to bypass the normalization procedures on TCP options (and along with that,
to lead to possibly unexpected behavior).

You can say whatever you want about the fact that NPF is not a kernel, that
NetBSD is not the only user of NPF, etc.; it doesn't alter in any way the
fact that the behavior is not the same, and that as a result the normalization
procedures can be bypassed.

So, either we fix this issue - by making NPF behave the same way as the
NetBSD kernel, which is what I suggested -, or we accept to have bypassable
normalization procedures.

Maxime

Side note: differences in behavior, are where bugs and bypasses lie. That's a
universal law of packet processing. (authored by me btw)

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Mindaugas Rasiukevicius]

Post by Maxime Villard
Answering in this thread now, to prevent further confusions.

So NPF's behavior should be aligned to that of the kernel; that is to
say, NPF should ignore TCP options with uncommon lengths - which does
not mean dropping the packet. (We can discuss about changing the
kernel's behavior to be that of NPF, but as I said in my answer to
Joerg, the kernel's behavior is the one that is the most "common".)

Not exactly, no. NPF is not a host/kernel. It is a man in the middle,
concerning packets sent by different hosts (which might have different
TCP/IP implementations and applications). It operates based on its own
set of rules.

It sounds like you didn't understand my point.
I'm saying that the TCP-options behavior in the NetBSD kernel and NPF is
not the same. There is a divergence. Since there is a divergence, it is
possible to bypass the normalization procedures on TCP options (and along
with that, to lead to possibly unexpected behavior).

So what? You talk about differences in behaviour, but you fail to explain
the reasons why any of this matters. The packet filter may be configured
to operate only at layer 3 and whatever happens at TCP would be completely
irrelevant.

Let me repeat again: it is the NPF *rules* (well, overall configuration)
which ultimately decide what is passed and what is blocked. Not a bunch
of if-statements you arbitrarily sprinkle before them. If you want to
introduce something like NPC_L4ERR and give the user an option (possibly
even enabling it by default) to block the packets with malformed (or not
conforming to the standards or "the kernel") L4 payload -- brilliant,
please do! However, do not hardcode these decisions before the rules.
The ruleset/configuration has a purpose.. this should be quite obvious.

[Maxime Villard]

Post by Mindaugas Rasiukevicius

Post by Maxime Villard
Answering in this thread now, to prevent further confusions.

So NPF's behavior should be aligned to that of the kernel; that is to
say, NPF should ignore TCP options with uncommon lengths - which does
not mean dropping the packet. (We can discuss about changing the
kernel's behavior to be that of NPF, but as I said in my answer to
Joerg, the kernel's behavior is the one that is the most "common".)

Not exactly, no. NPF is not a host/kernel. It is a man in the middle,
concerning packets sent by different hosts (which might have different
TCP/IP implementations and applications). It operates based on its own
set of rules.

It sounds like you didn't understand my point.
I'm saying that the TCP-options behavior in the NetBSD kernel and NPF is
not the same. There is a divergence. Since there is a divergence, it is
possible to bypass the normalization procedures on TCP options (and along
with that, to lead to possibly unexpected behavior).

So what? You talk about differences in behaviour, but you fail to explain
the reasons why any of this matters.

Read the very first email of this thread, I said what was wrong about not
looking at the length of the TCP options.

It matters because of bypasses, as I said only an hour ago in the mail you
just quoted.

Post by Mindaugas Rasiukevicius
Let me repeat again: it is the NPF *rules* (well, overall configuration)
[...]

This paragraph has nothing to do here, you are mixing with the other thread.

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Maxime Villard]

Post by Maxime Villard

Post by Mindaugas Rasiukevicius

Post by Maxime Villard
Answering in this thread now, to prevent further confusions.

So NPF's behavior should be aligned to that of the kernel; that is to
say, NPF should ignore TCP options with uncommon lengths - which does
not mean dropping the packet. (We can discuss about changing the
kernel's behavior to be that of NPF, but as I said in my answer to
Joerg, the kernel's behavior is the one that is the most "common".)

Not exactly, no. NPF is not a host/kernel. It is a man in the middle,
concerning packets sent by different hosts (which might have different
TCP/IP implementations and applications). It operates based on its own
set of rules.

It sounds like you didn't understand my point.
I'm saying that the TCP-options behavior in the NetBSD kernel and NPF is
not the same. There is a divergence. Since there is a divergence, it is
possible to bypass the normalization procedures on TCP options (and along
with that, to lead to possibly unexpected behavior).

So what? You talk about differences in behaviour, but you fail to explain
the reasons why any of this matters.

Read the very first email of this thread, I said what was wrong about not
looking at the length of the TCP options.
It matters because of bypasses, as I said only an hour ago in the mail you
just quoted.

Back on this; so I tested, and it works, the scenario I described in my first
email does bypass max-mss clamping.

That is to say, when you have a configuration of the kind:

procedure "norm" {
normalize: "max-mss" 25000
}
group default {
pass in all apply "norm"
}

sending a packet crafted as:

uint8_t *nptr = (uint8_t *)(tcphdr + 1);
uint16_t *mss;

nptr[0] = TCPOPT_MAXSEG;
nptr[1] = TCPOLEN_MAXSEG + 1;
mss = (uint16_t *)&nptr[2];
*mss = htons(20000); /* NPF reads this */
nptr[4] = TCPOPT_EOL;
nptr[5] = TCPOPT_MAXSEG;
nptr[6] = TCPOLEN_MAXSEG;
mss = (uint16_t *)&nptr[7];
*mss = htons(30000); /* the kernel reads this */

allows you to bypass the rule. NPF reads mss=20000, but the kernel reads
mss=30000 and registers the segment size as 30000.

You can easily see that by adding two printfs, one in npf_normalize and one
in tcp_dooptions.

I will commit my patch, unless there is a valid technical reason for not
doing so.

Maxime

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Mindaugas Rasiukevicius]

Post by Maxime Villard
<...>

Post by Maxime Villard
Read the very first email of this thread, I said what was wrong about
not looking at the length of the TCP options.
It matters because of bypasses, as I said only an hour ago in the mail
you just quoted.

Back on this; so I tested, and it works, the scenario I described in my
first email does bypass max-mss clamping.
<...>
allows you to bypass the rule. NPF reads mss=20000, but the kernel reads
mss=30000 and registers the segment size as 30000.

Just to remind: the purpose of MSS clamping is to get things working on
misconfigured networks i.e. you put it as a workaround so the packets
would flow (rather than be dropped). Bypassing a thing which is trying
to *help* you, as a sender, is hardly going to be useful (-- terms and
contions apply).

As I said -- I am not against having stricter defaults, but please keep
the options (an always set flag is fine for now) open for the users.
Even though we are talking about packet *filter*, as an application,
it is concerned with many more aspects that just *filtering* (in a sense
of restricting access). MSS clamping is actually an example of that.