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

change iptables default policy to accept and create last rule matchin… #486

Merged
merged 3 commits into from Dec 7, 2018

Conversation

Projects
None yet
2 participants
@hoedlmoser
Copy link
Contributor

hoedlmoser commented Nov 25, 2018

change iptables default policy to accept and create last rule matching all and rejecting, solves #483 iptables default policy

change iptables default policy to accept and create last rule matchin…
…g all and rejecting, solves #483 iptables default policy
@jens-maus

This comment has been minimized.

Copy link
Owner

jens-maus commented Nov 25, 2018

Thanks for your PR. I am, however, not sure if your proposed changes should be completely integrated.

While I can understand the practical implications of your proposed changes, I still feel that

  1. the default policy should be kept to DROP because nobody will actually manually flush iptables on accident and it is more or less IMHO more secure just in case a process is actually flushing on accident.
  2. changing the last rule (or the default policy) to REJECT instead of DROP will not only tell systems that access to a certain port is not allowed, it will also tell scanners (e.g. nmap) that there is actually a system accessible in principle.

So IMHO this would to some extend (even minor) degrade the security. What do you think?

@hoedlmoser

This comment has been minimized.

Copy link
Contributor

hoedlmoser commented Nov 25, 2018

  1. the default policy should be kept to DROP because nobody will actually manually flush iptables on accident and it is more or less IMHO more secure just in case a process is actually flushing on accident.

your first priority is on security, mine was on accessability. in the meantime I checked one of my servers and also there all main chains are on policy DROP and last rule is REJECTing (either with or w/o logging).

  1. changing the last rule (or the default policy) to REJECT instead of DROP will not only tell systems that access to a certain port is not allowed, it will also tell scanners (e.g. nmap) that there is actually a system accessible in principle.

ähem, security by obscurity. :-) what applies for ports also applies for ping.

if you ping a system either you get back the echo or just nothing. good indication that its there.
if you ping a system which currently is not available you get back a Destination Host Unreachable from your local PC or your gateway.
many thanks to ARP.

So IMHO this would to some extend (even minor) degrade the security. What do you think?

moving from DROP to ACCEPT policy of course, minor, plz let me refine my PR. trying to hide a system with DROP, no.

@jens-maus jens-maus referenced this pull request Dec 1, 2018

Closed

iptables default policy #483

@hoedlmoser

This comment has been minimized.

Copy link
Contributor

hoedlmoser commented Dec 6, 2018

refined my PR to keep default policy but reject all in last rule. btw, there was also a typo on IPv6 ping.

@jens-maus

This comment has been minimized.

Copy link
Owner

jens-maus commented Dec 6, 2018

Thanks for your refined PR. I immediately took over your ip6tables fix.

However, regarding changing the default policy to REJECT rather than DROP, I still believe that this only brings too little benefit and slightly reduce the security since system can then be more easily spoofed. So please allow me to reject this PR since I feel that the current default policy is more adequate.

@hoedlmoser

This comment has been minimized.

Copy link
Contributor

hoedlmoser commented Dec 6, 2018

challenge accepted. :-) in the meantime I will do my personal fix in /usr/local/etc/rc.local

@hoedlmoser hoedlmoser closed this Dec 6, 2018

@jens-maus jens-maus reopened this Dec 7, 2018

@jens-maus jens-maus added this to the next release milestone Dec 7, 2018

@jens-maus jens-maus merged commit 30a7a20 into jens-maus:master Dec 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment