NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/52074: -current npf map directive broken
The following reply was made to PR kern/52074; it has been noted by GNATS.
From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc:
Subject: Re: kern/52074: -current npf map directive broken
Date: Tue, 16 May 2017 06:17:51 +0000
Huge pile more not sent to gnats.
------
From: Christos Zoulas <christos%zoulas.com@localhost>
To: Roy Marples <roy%marples.name@localhost>, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>,
Frank Kardel <kardel%netbsd.org@localhost>
Cc: netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost
Subject: Re: kern/52074: -current npf map directive broken
Date: Sun, 7 May 2017 17:17:28 -0400
On May 7, 8:50pm, roy%marples.name@localhost (Roy Marples) wrote:
-- Subject: Re: kern/52074: -current npf map directive broken
| I think xtos already comitted a fix, but I'm unsure his fix is correct.
| I think the workflow should be this:
|
| if (ia != NULL)
| error = ip_ifaddrvalid(ia);
| else
| error = flags & IP_FORWARDING ? 0 : -1;
| if (error != 0) { ...
|
| The idea is that if we claim to send from an address it has to be valid,
| but allow the NULL address if forwarded from the filter.
|
| Does this make sense?
| The same path probably needs adjustment in inet6.
Sure, go for it. Why not put all the logic in ip_ifaddrvalid then?
christos
From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: Christos Zoulas <christos%zoulas.com@localhost>
cc: Roy Marples <roy%marples.name@localhost>, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>,
Frank Kardel <kardel%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost
Subject: Re: kern/52074: -current npf map directive broken
Date: Mon, 08 May 2017 06:12:04 +0700
Date: Sun, 7 May 2017 17:17:28 -0400
From: christos%zoulas.com@localhost (Christos Zoulas)
Message-ID: <20170507211728.8AF0A17FDA8%rebar.astron.com@localhost>
| On May 7, 8:50pm, roy%marples.name@localhost (Roy Marples) wrote:
| | The idea is that if we claim to send from an address it has to be valid,
| | but allow the NULL address if forwarded from the filter.
| |
| | Does this make sense?
| | The same path probably needs adjustment in inet6.
|
| Sure, go for it. Why not put all the logic in ip_ifaddrvalid then?
While you are fixing this, please also fix ...
* Address exists, but is tentative or detached.
* We can't send from it because it's invalid,
Tentative addresses are not invalid, and we *have* to be able to send
from them, without that DAD does't work correctly.
Consider a (perhaps unlikely, but possible) case where 2 systems attempt
to allocate the same address at approximately exactly the same time
(broken dhcp server, or common reboot after power fail and they have both
been configured to use the same addr by a broken sys-admin).
In that case they both make the addr tentative, each then starts DAD by
sending a "does anyone own..." query. Each receives the other's DAD
packet. The protocol then requires that, as we have that address, even
as a tentative one, we must reply (from the tentative address) and
claim the addr - both systems should do that, both receive the other's
defence of the addr, DAD fails (on both) and they both abandon the address.
But if we are not able to send from the addr, because it is considered
"invalid" then neither defends its addr, neither receives the other's
defence, and both then conclude that the addr is safe, and both (eventually)
reset the tentative flag, and chaos ensues.
What we should be doing with tentative addreses is allowing them to be
used (for any purpose), but just not announcing them to the rest of the
system (except to tools like ifconfig and netstat) until DAD has
completed. That means no other process will discover that the addr
exists, and so will not start to use it (nor will TCP accidentally see
the addr as existing, and start to use it) and so the incentive that
led to prohibiting sending from tentative addrs, or considering them
to be invalid, is gone.
It also permits applications that really need to start running quickly, and
know how to deal with addresses that appear and disappear, to start using
the tentative addr before it is made permanent, if they like (and if they
can find some mechanism - perhaps using whatever hook ifconfig/netstat use -
to discover that the addr exists.)
kre
From: Roy Marples <roy%marples.name@localhost>
To: Robert Elz <kre%munnari.OZ.AU@localhost>, Christos Zoulas <christos%zoulas.com@localhost>
Cc: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, Frank Kardel
<kardel%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost
Subject: Re: kern/52074: -current npf map directive broken
Date: Mon, 8 May 2017 01:17:18 +0100
On 08/05/2017 00:12, Robert Elz wrote:
> Date: Sun, 7 May 2017 17:17:28 -0400
> From: christos%zoulas.com@localhost (Christos Zoulas)
> Message-ID: <20170507211728.8AF0A17FDA8%rebar.astron.com@localhost>
>
> | On May 7, 8:50pm, roy%marples.name@localhost (Roy Marples) wrote:
>
> | | The idea is that if we claim to send from an address it has to be valid,
> | | but allow the NULL address if forwarded from the filter.
> | |
> | | Does this make sense?
> | | The same path probably needs adjustment in inet6.
> |
> | Sure, go for it. Why not put all the logic in ip_ifaddrvalid then?
>
> While you are fixing this, please also fix ...
>
> * Address exists, but is tentative or detached.
> * We can't send from it because it's invalid,
>
> Tentative addresses are not invalid, and we *have* to be able to send
> from them, without that DAD does't work correctly.
DAD happens at the ARP level, not the IP level.
Different checks are in place there.
> Consider a (perhaps unlikely, but possible) case where 2 systems attempt
> to allocate the same address at approximately exactly the same time
> (broken dhcp server, or common reboot after power fail and they have both
> been configured to use the same addr by a broken sys-admin).
>
> In that case they both make the addr tentative, each then starts DAD by
> sending a "does anyone own..." query. Each receives the other's DAD
> packet. The protocol then requires that, as we have that address, even
> as a tentative one, we must reply (from the tentative address) and
> claim the addr - both systems should do that, both receive the other's
> defence of the addr, DAD fails (on both) and they both abandon the address.
Address defence only happens once DAD has passed.
In the scenario you describe this has not occured for either host.
If both recieves each others initial probe then both will abort DAD and mark
as duplicate.
This is described in RFC5227, 2.1.1:
In addition, if during this period the host receives any ARP Probe
where the packet's 'target IP address' is the address being probed
for, and the packet's 'sender hardware address' is not the hardware
address of any of the host's interfaces, then the host SHOULD
similarly treat this as an address conflict and signal an error to
the configuring agent as above. This can occur if two (or more)
hosts have, for whatever reason, been inadvertently configured with
the same address, and both are simultaneously in the process of
probing that address to see if it can safely be used.
And RFC5227, 2.4:
(b) If a host currently has active TCP connections or other reasons
to prefer to keep the same IPv4 address, and it has not seen any
other conflicting ARP packets within the last DEFEND_INTERVAL
seconds, then it MAY elect to attempt to defend its address by
recording the time that the conflicting ARP packet was received,
and then broadcasting one single ARP Announcement, giving its own
IP and hardware addresses as the sender addresses of the ARP,
with the 'target IP address' set to its own IP address, and the
'target hardware address' set to all zeroes. Having done this,
the host can then continue to use the address normally without
any further special action. However, if this is not the first
conflicting ARP packet the host has seen, and the time recorded
for the previous conflicting ARP packet is recent, within
DEFEND_INTERVAL seconds, then the host MUST immediately cease
using this address and signal an error to the configuring agent
as described above. This is necessary to ensure that two hosts
do not get stuck in an endless loop with both hosts trying to
defend the same address.
> But if we are not able to send from the addr, because it is considered
> "invalid" then neither defends its addr, neither receives the other's
> defence, and both then conclude that the addr is safe, and both (eventually)
> reset the tentative flag, and chaos ensues.
This is not what happens.
DAD has not passed, there is no address to defend, it's just marked DUPLICATED
and stays that way until manual intervention.
> What we should be doing with tentative addreses is allowing them to be
> used (for any purpose), but just not announcing them to the rest of the
> system (except to tools like ifconfig and netstat) until DAD has
> completed. That means no other process will discover that the addr
> exists, and so will not start to use it (nor will TCP accidentally see
> the addr as existing, and start to use it) and so the incentive that
> led to prohibiting sending from tentative addrs, or considering them
> to be invalid, is gone.
On a bad day, DAD on IPv4 can take about 10 seconds to complete.
That's a big window to run ifconfig or getifaddrs in to find and start using
these non announced addresses.
Consider the wireless interface
dhcpcd starts, sees all interfaces are currently down, forks right away bootup
continues
more daemons start up
wifi comes up, dhcpcd is offered an address and adds it
ntpd starts at this point and tries to start using the tentative address.
> It also permits applications that really need to start running quickly, and
> know how to deal with addresses that appear and disappear, to start using
> the tentative addr before it is made permanent, if they like (and if they
> can find some mechanism - perhaps using whatever hook ifconfig/netstat use -
> to discover that the addr exists.)
If the application really neeeds to start running quickly, it can disable DAD
like so:
net.inet.ip.dad_count=0
Otherwise it can wait for the address to finish DAD and then it can start
using it.
Roy
From: Frank Kardel <kardel%kardel.name@localhost>
To: Roy Marples <roy%marples.name@localhost>, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>,
Frank Kardel <kardel%netbsd.org@localhost>
CC: netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, Christos Zoulas
<christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Mon, 08 May 2017 08:51:21 +0200
Hi Roy !
I had a quick look at IPv6. There the packet filter seems to be after local
address checking and scope related checks.
I haven't read up on the scope rules - so somebody should look at the
different pfil run location and whether it is appropriate to handle in case
the packet filter changes to source address to be a non local source address.
Frank
From: Roy Marples <roy%marples.name@localhost>
To: Frank Kardel <kardel%netbsd.org@localhost>, Mindaugas Rasiukevicius
<rmind%netbsd.org@localhost>
Cc: netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, Christos Zoulas
<christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Mon, 8 May 2017 10:04:01 +0100
Hi Frank!
On 07/05/2017 22:07, Frank Kardel wrote:
> Hmm, wouldn't this bring us back the bug again? ia == NULL for a
> non-local source addresses (generated via pfil_run_hooks-NAT operation)
> and IP_FORWARDING is not set as tcp_input.c:syn_cache_respond does
> rightfully not set IP_FORWARDING and pfil_run_hooks has no means to set
> that flag. That gives us error == -1 with your sequence.
> So we would return EADDRNOTAVAIL breaking packet filter NAT action
> again, if I didn't overlook something.
Ah yes.
I didn't actually read your initial report .... my bad!
OK, so I don't fully understand the use case for sending packets from an
address we don't have locally. Could you fill me in on this please?
> From what I understand this code originally attempted to avoid sending
> from invalid/unusable local address (e. g. duplicate IP - error,
> tentative and detached should just be dropped).
> No validation can be done for non-local addresses at all. IP_FORWARDING
> formerly used to be used to suppress infinite recursion on mcast
> forwarding, but it seems the semantics where extended a little bit in
> the mean time (like here to suppress a check).
> So I cannot say something about the intentions for the IP_FORWARDING check.
I *think* the idea was IP_FORWARDING would be set and we could skip
source address validation because the filter may have changed it.
https://nxr.netbsd.org/xref/src/sys/net/npf/npf_sendpkt.c#178
Does NAT not hit that? I ask because I do run NPF+NAT on my erlite
router which uses -current, but my mapping rule uses a local address -
hence asking for your use case about a non local address.
Could NAT set another flag we could check? rmind?
Roy
From: Frank Kardel <kardel%netbsd.org@localhost>
To: Roy Marples <roy%marples.name@localhost>, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>
CC: netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, Christos Zoulas
<christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Mon, 08 May 2017 13:23:40 +0200
On 05/08/17 11:04, Roy Marples wrote:
> Hi Frank!
>
> On 07/05/2017 22:07, Frank Kardel wrote:
> > Hmm, wouldn't this bring us back the bug again? ia == NULL for a
> > non-local source addresses (generated via pfil_run_hooks-NAT operation)
> > and IP_FORWARDING is not set as tcp_input.c:syn_cache_respond does
> > rightfully not set IP_FORWARDING and pfil_run_hooks has no means to set
> > that flag. That gives us error == -1 with your sequence.
> > So we would return EADDRNOTAVAIL breaking packet filter NAT action
> > again, if I didn't overlook something.
> Ah yes.
> I didn't actually read your initial report .... my bad!
No worries - I am just trying to keep us from running too far in the wrong
direction.
> OK, so I don't fully understand the use case for sending packets from an
> address we don't have locally. Could you fill me in on this please?
The mapping is as follows
Machine A (Client) Machine B (Gateway/NAT/redirect box) Machine C
(target)
a.b.c.d:x -> W.X.Y.Z -> 10.200.1.2 (will never see a
packet from a.b.c.dx)
map 127.0.0.1 25 <- 10.200.1.2 25
redirects to 127.0.0.1 25 (local)
A must see the association a.b.c.d:x <->
10.200.1.2:25
Thus local postfix will see a.b.c.d x <-> 127.0.0.1
25
So dest addr 10.200.1.2 will be replaced incoming
with
127.0.0.1 25 in pf_fil_run_hooks in ip_input and on
the way back
the src addr of 127.0.0.1 is replaced with
10.200.1.2 (not local)
in pfil_run_hooks in ip_output.
To quote the condensed analysis from the bug + where in the stack it happens:
Following happens:
1) ip_input: packet arrives on interface (a.b.c.d:x -> 10.200.1.2:25
1a) ip_input call pfil_run_hooks -> NPF
2) NPF: NAT assoc gets created
3) NPF: packet gets NATed (=> a.b.c.d:x -> 127.0.0.1:25)
3a) NPF returns
3b) ip_input hands over to tcp_input
4) tcp_input: packet gets entered in syn_cache
5) tcp_input: syn_cache attempts to send SYN,ACK response (127.0.0.1:25 ->
a.b.c.d:x) via ip_output()
6) ip_output: ip_output calls pfil_hooks (->NPF)
7) NPF: find nat assoc
8) NPF: SYN,ACK packet gets reverse NATed (10.200.1.2:25 -> a.b.c.d:x)
8a) NPF returns
9) ip_output: ip_output tries for find the interface adress (ia) for 10.200.1.2 and
fails (ia == NULL)
10) ip_output: ip_output fumbles badly at ip_output:1.276:620 and declares the
packet coming from an invalid address(ip_ifaddrvalid(NULL))
11) confusion commences from here on
>
> > From what I understand this code originally attempted to avoid sending
> > from invalid/unusable local address (e. g. duplicate IP - error,
> > tentative and detached should just be dropped).
> > No validation can be done for non-local addresses at all. IP_FORWARDING
> > formerly used to be used to suppress infinite recursion on mcast
> > forwarding, but it seems the semantics where extended a little bit in
> > the mean time (like here to suppress a check).
> > So I cannot say something about the intentions for the IP_FORWARDING check.
> I *think* the idea was IP_FORWARDING would be set and we could skip
> source address validation because the filter may have changed it.
> https://nxr.netbsd.org/xref/src/sys/net/npf/npf_sendpkt.c#178
That's what I thought was the intention. As you see above the transmitted
SYN,ACK is
not created by NPF and not by npf_sendpkt.c#178. NPF and other filters do an
in place header modification and the flags variable is not part of
pfil_run_hooks(). So
the IP_FORWARDING logic cannot be used by the packet filters AFAICS.
> Does NAT not hit that?
Nope - see above - coming directly from
tcp_input:syn_cache_add:syn_cache_respond. IP_FORWARD is not set there.
Before the packet filter we still have a valid local address, Maybe we should
move the check there - IPv6 also does the check before IIANM.
> I ask because I do run NPF+NAT on my erlite
> router which uses -current, but my mapping rule uses a local address -
> hence asking for your use case about a non local address.
>
> Could NAT set another flag we could check? rmind?
OTOMHI see
1) extend pfil_run_hooks interface to pass a pointer to flags- relatively
cheap, somewhat dirty
2) add a tag the the mbuf - more expensive b/c we also need to add checks
Would we gain enough speed/functionality/abstraction to warrant an interface
change?
Does the condition "before packet filter we have either IP_FORWARD set or a
local address as source" always hold?
If so, we could just move the check and set up ia correctly for IFASTATS.IPv6
checks before packet filter runs.
I have not analyzed the invariants enough to make a quick recommendation here.
I hope I explained the setup good enough to describe the issue with NAT
redirection.
>
> Roy
Frank
From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: Frank Kardel <kardel%netbsd.org@localhost>
cc: Roy Marples <roy%marples.name@localhost>, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>,
netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, Christos Zoulas
<christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Wed, 10 May 2017 05:45:11 +0700
Date: Sun, 07 May 2017 23:07:42 +0200
From: Frank Kardel <kardel%netbsd.org@localhost>
Message-ID: <590F8C9E.3040102%netbsd.org@localhost>
| From what I understand this code originally attempted to avoid sending
| from invalid/unusable local address (e. g. duplicate IP - error,
| tentative and detached should just be dropped).
You also shouldn't be able to send from an address you don't own
(generally - a router has to be able to forward, as distinct from
originate, packets from anywhere of course).
Maybe you could explain what you're trying to achieve, rather than
worry too much (right now) at some particular kernel test which seems
to be defeating the way you are currently trying to accomplish that.
If your aim is to have machine "B" (the router/NAT box) from your
later e-mail example, intercept SMTP (and perhaps other) connection
attempts that your internal system (A) is attempting to make to
some external system (C) - so that the connection is handled by B
instead (acting as a proxy) then I suspect that someone can share
a config which accomplishes that ... but I very much doubt it will
involve something so weird as attempting to NAT into the loopback address.
If you have some other aim, explain it, and someone can probably
show how to accomplish it. But please explain the objective, not
the technique you believe you can use to meet that objective.
Once we know what it is you want to do, it is still possible that
kernel (or other) changes might be needed to accomplish it (or it
might just not be possible at all), but if changes are needed the
right ones to make could be somewhere quite different from what
you have been concentrating on.
kre
ps: this issue is of course totally unrelated to the other question of
whether tentative addresses should be considered invalid, which certainly
has nothing to do with your problem.
From: Frank Kardel <kardel%netbsd.org@localhost>
To: Robert Elz <kre%munnari.OZ.AU@localhost>
CC: Roy Marples <roy%marples.name@localhost>, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>,
netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, Christos Zoulas
<christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Wed, 10 May 2017 11:11:13 +0200
Hi Robert !
On 05/10/17 00:45, Robert Elz wrote:
> Date: Sun, 07 May 2017 23:07:42 +0200
> From: Frank Kardel <kardel%netbsd.org@localhost>
> Message-ID: <590F8C9E.3040102%netbsd.org@localhost>
>
> | From what I understand this code originally attempted to avoid sending
> | from invalid/unusable local address (e. g. duplicate IP - error,
> | tentative and detached should just be dropped).
>
> You also shouldn't be able to send from an address you don't own
> (generally - a router has to be able to forward, as distinct from
> originate, packets from anywhere of course).
You are correct - in this case (52074) we are looking at both aspects - the
local machine and the router/NAT box.
It is *not* about originating packets from anywhere. It is about redirecting
packets for non local targets to a locally existing proxy.
>
> Maybe you could explain what you're trying to achieve, rather than
> worry too much (right now) at some particular kernel test which seems
> to be defeating the way you are currently trying to accomplish that.
I am just reporting a regression that was introduced by the code related local
address checks.
>
> If your aim is to have machine "B" (the router/NAT box) from your
> later e-mail example, intercept SMTP (and perhaps other) connection
> attempts that your internal system (A) is attempting to make to
> some external system (C) - so that the connection is handled by B
> instead (acting as a proxy) then I suspect that someone can share
> a config which accomplishes that ...
See man 5 npf.conf - there is an example. It also used to work with pf years
ago but I used pf long ago
and haven't retested it with last years address-check/tentative/detached
changes. pf could
also be impacted by last years change.
Example from pf.conf(5)
Note that redirecting external incoming connections to the loopback
address, as in
rdr on ne3 inet proto tcp to port spamd -> 127.0.0.1 port smtp
Example from npf.conf(5)
map $ext_if dynamic proto tcp 10.1.1.2 port 22 <- $ext_if port 9022
Not necessarily a local address, but local addresses are not forbidden either.
> but I very much doubt it will
> involve something so weird as attempting to NAT into the loopback address.
(Transparent) proxies might very well do that at boundaries between external
and internal networks.
Using any other local address instead of 127.0.0.1 wouldn't change a thing for
the discussion as the
original target address is redirected to the local address and needs to be
rewritten back to the
original target address on the way back.
>
> If you have some other aim, explain it, and someone can probably
> show how to accomplish it. But please explain the objective, not
> the technique you believe you can use to meet that objective.
It is a proxy for a system C that is not always online. And the proxy is
located at the gateway.
The solution is a common redirect scenario. nothing special unless you have a
regression in the stack.
I believe that the current local address check is just placed wrong. But I
just uncovered that
while analyzing why the NAT redirect broke - I am not responsible for the
address check code which broke
NAT redirection to local addresses as an unwanted/unexpected side effect.
I think we have a discussion going on how to implement the address check
properly. And I think
there may be a better solution which needs to be found and implemented which
enforces the local address
check more sensibly..
If we put the local address check before the packet filter call (doing the
reverse NAT)
then all would be fine, But I think it should be discussed (roy@, rmind@,
christos@, ...)
whether this is the right solution to have a valid local address check and the
redirect
to local address NAT functionality.
>
> Once we know what it is you want to do, it is still possible that
> kernel (or other) changes might be needed to accomplish it (or it
> might just not be possible at all), but if changes are needed the
> right ones to make could be somewhere quite different from what
> you have been concentrating on.
A proxy on a gateway is something that should work and it used to work with
npf and pf.
Last year a regression was introduced breaking that functionality.
I am happy learn another way to configure a proxy using local addresses of the
gateway
*without* changing ip_output.c - we just need to fix our own documentation
then,
Can you provide an alternative configuration to accomplish the above?
> kre
>
> ps: this issue is of course totally unrelated to the other question of
> whether tentative addresses should be considered invalid, which certainly
> has nothing to do with your problem.
>
Frank
From: Roy Marples <roy%marples.name@localhost>
To: Robert Elz <kre%munnari.OZ.AU@localhost>, Frank Kardel <kardel%netbsd.org@localhost>
Cc: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost, Christos Zoulas <christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Wed, 10 May 2017 13:22:01 +0100
On 09/05/2017 23:45, Robert Elz wrote:
> Date: Sun, 07 May 2017 23:07:42 +0200
> From: Frank Kardel <kardel%netbsd.org@localhost>
> Message-ID: <590F8C9E.3040102%netbsd.org@localhost>
>
> | From what I understand this code originally attempted to avoid sending
> | from invalid/unusable local address (e. g. duplicate IP - error,
> | tentative and detached should just be dropped).
>
> You also shouldn't be able to send from an address you don't own
> (generally - a router has to be able to forward, as distinct from
> originate, packets from anywhere of course).
This is what my initial code did.
What I'm more concerned about though is the panic.
I think we should revert xtos's change and solve the panic, as this just
masks over it.
Roy
From: Christos Zoulas <christos%zoulas.com@localhost>
To: Roy Marples <roy%marples.name@localhost>, Robert Elz <kre%munnari.OZ.AU@localhost>, Frank
Kardel <kardel%netbsd.org@localhost>
Cc: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost
Subject: Re: kern/52074: -current npf map directive broken
Date: Wed, 10 May 2017 09:33:25 -0400
On May 10, 1:22pm, roy%marples.name@localhost (Roy Marples) wrote:
-- Subject: Re: kern/52074: -current npf map directive broken
| > You also shouldn't be able to send from an address you don't own
| > (generally - a router has to be able to forward, as distinct from
| > originate, packets from anywhere of course).
|
| This is what my initial code did.
| What I'm more concerned about though is the panic.
| I think we should revert xtos's change and solve the panic, as this just
| masks over it.
This is not about fixing the panic. It is about the ability of the packet
filter to construct packets for which the origin interface cannot be
determined from the packet source address.
christos
From: Frank Kardel <kardel%netbsd.org@localhost>
To: Roy Marples <roy%marples.name@localhost>, Robert Elz <kre%munnari.OZ.AU@localhost>
CC: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost, Christos Zoulas <christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Wed, 10 May 2017 15:37:50 +0200
Hi Roy !
Don't worry about the panic at all - it is unrelated - that was a side issue I
already analyzed and discussed with christos@ and rmind@. Root cause there is
currently a semantic problem in NPF regarding naming of dynamic rules.
Frank
From: Christos Zoulas <christos%zoulas.com@localhost>
To: Frank Kardel <kardel%netbsd.org@localhost>, Roy Marples <roy%marples.name@localhost>, Robert
Elz <kre%munnari.OZ.AU@localhost>
Cc: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost
Subject: Re: kern/52074: -current npf map directive broken
Date: Wed, 10 May 2017 09:42:34 -0400
On May 10, 3:37pm, kardel%netbsd.org@localhost (Frank Kardel) wrote:
-- Subject: Re: kern/52074: -current npf map directive broken
| Hi Roy !
|
| Don't worry about the panic at all - it is unrelated - that was a side
| issue I already analyzed and discussed with christos@ and rmind@. Root
| cause there is currently a semantic problem in NPF regarding naming of
| dynamic rules.
To be even more clear: the panic is unrelated to packet forwarding and
has to do with an internal issue of npf; that issue is unrelated to
the formation of packets by npf.
christos
From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: Roy Marples <roy%marples.name@localhost>
cc: Frank Kardel <kardel%netbsd.org@localhost>, Mindaugas Rasiukevicius
<rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
Christos Zoulas <christos%NetBSD.org@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Thu, 11 May 2017 17:43:27 +0700
Date: Thu, 11 May 2017 10:47:28 +0100
From: Roy Marples <roy%marples.name@localhost>
Message-ID: <c2d2d033-a421-df5f-a0cd-4046b7d00808%marples.name@localhost>
| I agree with Robert, we shouldn't be sending packets on the wire from an
| address we don't own.
| But you're not sending on the wire are you?
Actually, he is, he wants to masquerade as some other (remote) host
(proxy for them) and return packets as if the client host was communicating
with that one, when it is really communicating with his NAT box.
That is, he wants to pretend he is forwarding a packet from the remote
host back to the client, when the packet has actually been originated
locally.
This has no chance of working if any protocol that verifies identity
is in use (even SMTP has that available now, I think), but otherwise
could work.
Personally I wouldn't be trying to fake it at that level though, I'd
be returning MX records to the client listing the NAT box as the
preferred (only, probably) mail relay for the host in question
(or for anyone that the client wants to talk to, if that is the
objective.) Of course, that only works for SMTP.
kre
From: Joerg Sonnenberger <joerg%bec.de@localhost>
To: Roy Marples <roy%marples.name@localhost>
Cc: netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost
Subject: Re: kern/52074: -current npf map directive broken
Date: Thu, 11 May 2017 14:19:49 +0200
On Thu, May 11, 2017 at 10:47:28AM +0100, Roy Marples wrote:
> I agree with Robert, we shouldn't be sending packets on the wire from an
> address we don't own.
That depends. The transparent proxy case is tricky in this regard...
Joerg
From: Frank Kardel <kardel%netbsd.org@localhost>
To: Roy Marples <roy%marples.name@localhost>, Robert Elz <kre%munnari.OZ.AU@localhost>
CC: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost, Christos Zoulas <christos%NetBSD.org@localhost>, Martin
Husemann <martin%duskware.de@localhost>, Joerg Sonnenberger <joerg%bec.de@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Thu, 11 May 2017 22:33:58 +0200
Hi Roy and all!
On 05/11/17 11:47, Roy Marples wrote:
> Hi Frank On 10/05/2017 10:11, Frank Kardel wrote:
> > On 05/10/17 00:45, Robert Elz wrote:
> > > Date: Sun, 07 May 2017 23:07:42 +0200 From: Frank Kardel
> > > <kardel%netbsd.org@localhost> Message-ID: <590F8C9E.3040102%netbsd.org@localhost> |
> > > From what I understand this code originally attempted to avoid
> > > sending | from invalid/unusable local address (e. g. duplicate IP
> > > - error, | tentative and detached should just be dropped). You
> > > also shouldn't be able to send from an address you don't own
> > > (generally - a router has to be able to forward, as distinct from
> > > originate, packets from anywhere of course).
> > You are correct - in this case (52074) we are looking at both aspects
> > - the local machine and the router/NAT box. It is *not* about
> > originating packets from anywhere. It is about redirecting packets for
> > non local targets to a locally existing proxy.
> I agree with Robert, we shouldn't be sending packets on the wire from an
> address we don't own. But you're not sending on the wire are you?
See kre@'s comment. He described it.The local addres gets replaced with the
remote address we proxied for.
> I think a check to satisfy us all would be to test for IP_FORWARDING on
> the packet or IFF_LOOPBACK on the outgoing interface - if either are true
> we can skip address validation.
The IFF_LOOPBACK property is of no use here. What counts is that the redirect
is to a local address to masquerade a remote address.
Any local address used for redirect would trigger the issue as the packet
filter puts back the original remote address and that's violating the
assumption
the we must use local addresses on locally generated packets.
> Thoughts? Roy
I think the solution is much simpler (as I mentioned before).
If we actually take look at ip_output we see roughly following processing
steps:
1) insert ip options
2) create IP header unless forwarding or raw output
3) handle routing (choose output interface)
4) if no source address is set, use the interface address of the outgoing
interface
5) handle IPSEC
6) run packet filter
7) check whether we are sending from a valid local address (our discussion
point)
8) do xmit accounting
9) handle TSO offloading or packet not needing fragmentation and output via
interface -> done
10) handle checksum offloading/calculation
11) handle fragmentation output fragmented packets
12) finalize
During 1...5 the assumption/requirement that non IP_FORWARDING packets must
originate from local addresses holds.
Number 6 breaks this assumption in case of a redirected remote address to a
local address. There the test in 7 relies
on a requirement that cannot be guaranteed after packet filter NAT operations.
Moving ia pickup and !IP_FORWARDING local address check from 7 to 5a will
achieve what we want (guard against from invalid local addresses). Also
IFA accounting would correctly measure bytes-sent at the right local address
involved in the redirect scenario.
I am currently running a check on local address violations before the packet
filter call. The invariant that 1..5 follow the local address requirement
holds so
far. Traffic processed is: normal connections, ipsec transport/NATT, some
multicast.
Can somebody verify that strategy as a correct solution?
NB: the current fix has the drawback of at least miscounting output bytes and
missing some local checks.
Frank
From: Roy Marples <roy%marples.name@localhost>
To: Frank Kardel <kardel%netbsd.org@localhost>, Robert Elz <kre%munnari.OZ.AU@localhost>
Cc: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost, Christos Zoulas <christos%NetBSD.org@localhost>, Martin
Husemann <martin%duskware.de@localhost>, Joerg Sonnenberger <joerg%bec.de@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Thu, 11 May 2017 21:42:07 +0100
Yo
On 11/05/2017 21:33, Frank Kardel wrote:
> I think the solution is much simpler (as I mentioned before).
>
> If we actually take look at ip_output we see roughly following
> processing steps:
> 1) insert ip options
> 2) create IP header unless forwarding or raw output
> 3) handle routing (choose output interface)
> 4) if no source address is set, use the interface address of the
> outgoing interface
> 5) handle IPSEC
> 6) run packet filter
> 7) check whether we are sending from a valid local address (our
> discussion point)
> 8) do xmit accounting
> 9) handle TSO offloading or packet not needing fragmentation and output
> via interface -> done
> 10) handle checksum offloading/calculation
> 11) handle fragmentation output fragmented packets
> 12) finalize
>
> During 1...5 the assumption/requirement that non IP_FORWARDING packets
> must originate from local addresses holds.
> Number 6 breaks this assumption in case of a redirected remote address
> to a local address. There the test in 7 relies
> on a requirement that cannot be guaranteed after packet filter NAT
> operations.
>
> Moving ia pickup and !IP_FORWARDING local address check from 7 to 5a
> will achieve what we want (guard against from invalid local addresses).
> Also
> IFA accounting would correctly measure bytes-sent at the right local
> address involved in the redirect scenario.
>
> I am currently running a check on local address violations before the
> packet filter call. The invariant that 1..5 follow the local address
> requirement holds so
> far. Traffic processed is: normal connections, ipsec transport/NATT,
> some multicast.
>
> Can somebody verify that strategy as a correct solution?
>
> NB: the current fix has the drawback of at least miscounting output
> bytes and missing some local checks.
Why not grab the pre-filter address at 5 and if the post filter address is
NULL, use the pre-filter address?
Would that work and solve the counting issue as well?
Roy
From: Frank Kardel <kardel%netbsd.org@localhost>
To: Roy Marples <roy%marples.name@localhost>, Robert Elz <kre%munnari.OZ.AU@localhost>
CC: Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost, Christos Zoulas <christos%NetBSD.org@localhost>, Martin
Husemann <martin%duskware.de@localhost>, Joerg Sonnenberger <joerg%bec.de@localhost>
Subject: Re: kern/52074: -current npf map directive broken
Date: Thu, 11 May 2017 23:06:42 +0200
Hi Roy !
Would be possible, but I think we are not gaining any benefit from the
additional look-up.
Before the packet filter we see the originating interface. That is the one for
which we need to count the bytes sent.
After the packet filter we see either no change most of the time, some other
local interface or nothing.
So I would prefer just to look the the originating interface.
There is one subtle case for not being able to find the correct local
interface from the source address.
In BSD multiple interfaces can have the same IP address. In this situation we
cannot correctly determine
the correct interface from the source address alone for bytes-sent accounting.
As this didn't work properly up to now we are not making things worse with
respect to bytes-sent accounting.
Frank
Home |
Main Index |
Thread Index |
Old Index