Bug 4976 - Remove unused enable method of FirewallBackend* classes in tl-setup
Summary: Remove unused enable method of FirewallBackend* classes in tl-setup
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Server Installer (show other bugs)
Version: 4.1.1
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.2.0
Assignee: Henrik Andersson
URL:
Keywords: derfian_tester, prosaic
Depends on:
Blocks:
 
Reported: 2014-01-31 10:24 CET by Karl Mikaelsson
Modified: 2014-05-16 09:17 CEST (History)
1 user (show)

See Also:
Acceptance Criteria:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Description Karl Mikaelsson cendio 2014-01-31 10:24:44 CET 
The point of the firewall module of tl-setup is to:

 - Add service definitions to the firewall.
 - Persistently enable services.

It should not:

 - Enable/disable the entire firewall.

We currently have enable() methods in each firewall backend which allows tl-setup to enable the firewall. This is IMO wrong, and we should never touch the running state of firewall other than to have it reload the new firewall rules.

Also, the enable() methods are never used save for in the file-local main().
Comment 1 Henrik Andersson cendio 2014-02-04 12:28:06 CET 
Removed enable() implementations from backends and frontend in commit 28369.
Comment 2 Karl Mikaelsson cendio 2014-03-24 09:06:56 CET 
Code review looks good, but waiting for reports of successful installations on SuSE, Ubuntu and RHEL before closing.
Comment 3 Karl Mikaelsson cendio 2014-03-25 16:03:51 CET 
Adding testers for Ubuntu, SuSE, RHEL and Fedora. Please remove yourself from the tester list when you've made sure that the firewall part of tl-setup works as indented.
Comment 4 Henrik Andersson cendio 2014-04-03 13:34:29 CEST 
Verified functionality on RHEL6 using build 4312.
Comment 5 Samuel Mannehed cendio 2014-04-09 16:47:55 CEST 
Verified functionality on Fedora 20 (32-bit) using build 4318.
Comment 6 Peter Åstrand cendio 2014-05-06 13:18:03 CEST 
Works on SLED11 using build 4346. iptables diff:

+LOG        tcp  --  anywhere             anywhere            limit: avg 3/min burst 5 tcp dpt:904 flags:FIN,SYN,RST,ACK/SYN LOG level warning tcp-options ip-options prefix `SFW2-INext-ACC-TCP ' 
+ACCEPT     tcp  --  anywhere             anywhere            tcp dpt:904 
+LOG        tcp  --  anywhere             anywhere            limit: avg 3/min burst 5 tcp dpt:cslistener flags:FIN,SYN,RST,ACK/SYN LOG level warning tcp-options ip-options prefix `SFW2-INext-ACC-TCP ' 
+ACCEPT     tcp  --  anywhere             anywhere            tcp dpt:cslistener 
+LOG        tcp  --  anywhere             anywhere            limit: avg 3/min burst 5 tcp dpt:300 flags:FIN,SYN,RST,ACK/SYN LOG level warning tcp-options ip-options prefix `SFW2-INext-ACC-TCP ' 
+ACCEPT     tcp  --  anywhere             anywhere            tcp dpt:300 
+LOG        tcp  --  anywhere             anywhere            limit: avg 3/min burst 5 tcp dpt:surf flags:FIN,SYN,RST,ACK/SYN LOG level warning tcp-options ip-options prefix `SFW2-INext-ACC-TCP ' 
+ACCEPT     tcp  --  anywhere             anywhere            tcp dpt:surf
Comment 7 Pierre Ossman cendio 2014-05-12 16:01:30 CEST 
Works on Ubuntu 14.04 (which had ufw disabled by default, so it needed to be activated first).
Comment 8 Karl Mikaelsson cendio 2014-05-16 09:17:05 CEST 
No reports of breakage from any platforms during testing, considering it done.
Note You need to log in before you can comment on or make changes to this bug.