Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

routing/firewall: no longer depend on SuSEfirewall2 for IPv4 forwarding #503

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
5 participants
@mgerstner
Copy link

commented Mar 9, 2017

IPv4 and IPv6 forwarding have been handled inconsistently, IPv4
forwarding being configured implicitly via SuSEfirewall2, if enabled.

To straighten out things yast should handle both cases IPV4 and IPv6 the
same, ignoring what SuSEfirewall2 does. SuSEfirewall2 routing will
continue to function if the user configures FW_ROUTE=yes. In the future
SuSEfirewall2 will fiddle with ip_forwarding only if it's not been
already enabled via yast/sysctl.

Also see bnc#572202.

routing/firewall: no longer depend on SuSEfirewall2 for IPv4 forwarding
IPv4 and IPv6 forwarding have been handled inconsistently, IPv4
forwarding being configured implicitly via SuSEfirewall2, if enabled.

To straighten out things yast should handle both cases IPV4 and IPv6 the
same, ignoring what SuSEfirewall2 does. SuSEfirewall2 routing will
continue to function if the user configures FW_ROUTE=yes. In the future
SuSEfirewall2 will fiddle with ip_forwarding only if it's not been
already enabled via yast/sysctl.

Also see bnc#572202.
@coveralls

This comment has been minimized.

Copy link

commented Mar 9, 2017

Coverage Status

Coverage decreased (-0.02%) to 48.76% when pulling 8e6e14c on mgerstner:master into 46cd5b8 on yast:master.

mgerstner added some commits Mar 9, 2017

@coveralls

This comment has been minimized.

Copy link

commented Mar 9, 2017

Coverage Status

Coverage decreased (-0.02%) to 48.76% when pulling e867ba0 on mgerstner:master into 46cd5b8 on yast:master.

@mchf

This comment has been minimized.

Copy link
Member

commented Mar 9, 2017

just a note. Yast handles ipv4 / ipv6 forwarding inconsistently just bcs susefirewall2 supports only ipv4 (regarding FW_ROUTE)

@mgerstner

This comment has been minimized.

Copy link
Author

commented Mar 20, 2017

Yes it's true that susefirewall2 was the cause for this inconsisency, which is discussed in bnc#572202.

I'm new susefirewall2 maintainer since recently and would like to remove this coupling of yast and susefirewall2 to address the bug.

@lslezak

This comment has been minimized.

Copy link
Member

commented Apr 4, 2017

Please add a new changes entry with a bugzilla or FATE number and bump the package version. To me it looks good, but I'll ask a network expert for a review...

@lslezak lslezak requested a review from mchf Apr 4, 2017

mgerstner added some commits Apr 4, 2017

@coveralls

This comment has been minimized.

Copy link

commented Apr 4, 2017

Coverage Status

Coverage decreased (-0.02%) to 48.721% when pulling 55cf076 on mgerstner:master into a865e3c on yast:master.

@mgerstner

This comment has been minimized.

Copy link
Author

commented Apr 4, 2017

Okay, version bump and changes added

@mchf
Copy link
Member

left a comment

I curently don't like the patch much bcs:

  • enabling routing will not work anymore on machine with the firewall enabled
  • hides enabled routing when enabled via FW_ROUTE

both cases are of course taken from IPv4's POV.

In my POV the first one will confuse a user and will be reported as a bug / regression. The second one can be considered a kind of security issue.

@mgerstner

This comment has been minimized.

Copy link
Author

commented Apr 5, 2017

enabling routing will not work anymore on machine with the firewall enabled

I don't see how you come to that conclusion. With the change IPv4 routing is always enabled via sysctl, if enabled in yast, independently of the firewall.

hides enabled routing when enabled via FW_ROUTE

This is the case now, too. After the patch this is only the case if IPv4 forwarding is disabled in yast but FW_ROUTE is enabled in SuSEfirewall, which is only for backwards compatibility.

@mchf

This comment has been minimized.

Copy link
Member

commented Apr 7, 2017

enabling routing will not work anymore on machine with the firewall enabled

I don't see how you come to that conclusion. With the change IPv4 routing is always enabled via sysctl, if enabled in yast, independently of the firewall.

AFAIK sysctl is must have but not only thing which needs to be set up when firewall is enabled.

Currently, IPv4 routing starts to work regardless SuSEfirewall is enabled or not, when asked so in YaST. This will change if this patch is applied in my POV in the "enabled SuSEfirewall" use case.

AFAIK default policy for FORWAD is DROP when SuSEfirewall is enabled. Yast itself doesn't modify any rule / policy, it currently only sets FW_ROUTE and iptables are modified by SuSEfirewall then. So, when setting FW_ROUTE is dropped, IPv4 will not start to work after enabling IPv4 routing in YaST.

@mgerstner

This comment has been minimized.

Copy link
Author

commented Apr 7, 2017

Currently, IPv4 routing starts to work regardless SuSEfirewall is enabled or not, when asked so in YaST. > This will change if this patch is applied in my POV in the "enabled SuSEfirewall" use case.

Okay now I understand what you mean.

I don't think that forwarding arbitrary traffic is actually allowed simply by setting FW_ROUTE to yes. Quoting from the SFW2 documentation:

# Enabling this option alone doesn't do anything. Either activate
# masquerading [...], or configure FW_FORWARD to
# define what is allowed to be forwarded. You also need to define
# internal or dmz interfaces [...].

I've just done a comparison of iptables rules between FW_ROUTE=yes/no on a Leap 42.2 install and the only difference with FW_ROUTE=yes seems to be that forwarding icmp pings are allowed and dropped forwarding packets are logged.

Also we have the inconsistency between IPv4 and IPv6. ATM if you enable forwarding only for IPv6 then FW_ROUTE will also stay off and not even icmpv6 pings will pass between interfaces. On the other hand if you only set FW_ROUTE=yes and don't enable IPv6 forwarding in yast then there also won't be any IPv6 routing.

Personally I find that it is an unexpected side effect that enabling IPv4 forwarding in yast causes changes in my firewall setup (and that also only if the firewall is enabled ATM). The worst thing is that IPv4 and IPv6 behave differently here in my POV.

We should also think of future changes here: We're planning on changing the default firewall to firewalld for SLE-13. What will we do then?

@mchf

This comment has been minimized.

Copy link
Member

commented May 2, 2017

We should also think of future changes here: We're planning on changing the default firewall to firewalld for SLE-13. What will we do then?

this is another thing. When we switch from SuSEFirewall to something else, then I've nothing against this change. But yast-network:master is for SLE-12-SP3 now. We will fork SLE-12-SP3 and SLE-15 in approx 2 weeks. However, I haven't seen any fate request for default firewall change yet.

@mchf

This comment has been minimized.

Copy link
Member

commented May 2, 2017

In general I'm not that sure how it behaves in Leap. But I have to be interested how it behaves in SLE. As what you wrote above is a bit different than what I thought about that I need to do some testing here.

@mgerstner

This comment has been minimized.

Copy link
Author

commented May 2, 2017

However, I haven't seen any fate request for default firewall change yet.

Yes there is one. It's FATE#320794.

This change is not critical for me. Just wanted to clean up these things on both sides, SuSEfirewall2 and yast-network. The current situation is difficult and some of your points are valid concerns.

@teclator

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2017

@mgerstner I will close it as I already added your changes in #578

Thnx a lot!

@teclator teclator closed this Nov 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.