search

dimikot / vzfirewall

vzfirewall is an extremely simple tool to configure opened ports and hosts for incoming connections in OpenVZ environment
GNU Lesser General Public License v2.1
15 stars 6 forks source link

Reworking options handling and default settings to be more flexible #2

Closed jnorell closed 10 years ago

dimikot commented 10 years ago

Great! I've made a little code review (comments are tied to line numbers), could you please correct the changes and push to your repo?.. The pull request will be updated automatically if I am not wrong.

jnorell commented 10 years ago

That last commit wraps up most of the actual functionality I have in mind to implement in the short term, pending the decision of whether or not to support a config file.

I might add debian packaging files if time allows.

dimikot commented 10 years ago

Jesse, overall - great work! A couple of more comments (maybe) from my side, and I'll merge it into the upstream. :-) BTW large pull requests are evil: it's quite hard to review it. I'm not sure GitHub allows to create a number of small pull requests instead of a big one.

jnorell commented 10 years ago

BTW large pull requests are evil: it's quite hard to review it. I'm not sure GitHub allows to create a number of small pull requests instead of a big one.

Yes, sorry, I didn't realize it would do that, and once I got started, there was no stopping. :) Perhaps you need to create a separate branch for each collection of commits to go into the pull request.

Jesse Norell Kentec Communications, Inc. 970-522-8107 - www.kci.net

dimikot commented 10 years ago

but at times it's invaluable help in catching why some service isn't working (ie. need to allow something in the firewall)

Definitely.

  1. So is it for a debugging purpose or for a production purpose? If it is for debugging, I vote to leave it turned off by default (but allow to turn the option on in the config file). If it is for a production purpose, then I don't understand it quite: 99.99% of such logs are useless, because they are generated by worms' port scanners. (Frankly, during last 5 years I never turned on logging in iptables, because else I got immediately spoofed with lots of useless info.) Why waste resources then? BTW SSH port on a hardware note is practically always opened, and because it is opened, SSH logs connection attempts by itself in /var/log/auth.log. So why log it in addition by vzfirewall?
  2. Where are these logs saved at by default? /var/log/syslog?
jnorell commented 10 years ago

On Thu, 2014-01-23 at 11:40 -0800, Dmitry Koterov wrote:

    but at times it's invaluable help in catching why some service
    isn't working (ie. need to allow something in the firewall)
    Definitely.
    1. So is it for a debugging purpose or for a production
    purpose? If it is for debugging, I vote to leave it turned off
    by default (but allow to turn the option on in the config
    file). If it is for a production purpose, then I don't
    understand it quite: 99.99% of such logs are useless, because
    they are generated by worms' port scanners.

Yes, most are worthless, but what about the other .01% of the time when they'd actually be used/needed?

     (Frankly, during last 5 years I never turned on logging in
    iptables, because else I got immediately spoofed with lots of
    useless info.) Why waste resources then? BTW SSH port on a
    hardware note is practically always opened, and because it is
    opened, SSH logs connection attempts by itself
    in /var/log/auth.log. So why log it in addition by vzfirewall?
    2. Where are these logs saved at by default? /var/log/syslog?

The logs go to syslog, so wherever it's setup to put them. Around here we have them go to a central log server (in addition to /var/log/ file).

On your previous question, why log in addition to what sshd logs, yes, it's mostly redundant for successful/failed logins; it's not for port scans or possibly on occasions that sshd is exploited (we've had that happen in years past). But yes, as long as syslog is sending to a central log server you'll usually be able to recover logs even if you are compromised and local logs are scrubbed.

I'll work on getting the on/off feature implemented, and we can set the default either way (I'll make a system config var for local preference).

Jesse Norell Kentec Communications, Inc. 970-522-8107 - www.kci.net

dimikot commented 10 years ago

Splendid! Thanks for your patience.

dimikot commented 10 years ago

Jesse, may I convert tabs to spaces now and commit? Will it conflict with any of your not-yet-pushed changes?

jnorell commented 10 years ago

Go for it, I don't have anything else underway. Also, if you have preferred .vimrc settings I can switch to whatever.

dimikot commented 10 years ago

Done. Please pull. Tab size is 4 spaces (it's your defaults, I suppose).