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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shorewall::blacklist #3

Merged
merged 4 commits into from Apr 13, 2014

Conversation

Projects
None yet
2 participants
@raoulbhatia
Copy link
Contributor

raoulbhatia commented Mar 23, 2014

My first shot at adding my own class to your module.
Please let me know if this is OK with you ;)

Cheers,
Raoul
PS. Thanks for merging that fast! 馃憤

@inkblot

This comment has been minimized.

Copy link
Owner

inkblot commented Mar 24, 2014

This looks fine, except that Shorewall's blacklist file is deprecated in versions that support it, and no longer supported as of Shorewall 4.5.7. There are also related options in shorewall.conf that you may want to control. The deprecation of this file is part of the reason why I had not already implemented a blacklist define. Tom Eastep recommends using the blrules file instead at http://shorewall.net/blacklisting_support.htm, which is supported as of version 4.4.25.

@raoulbhatia

This comment has been minimized.

Copy link
Contributor

raoulbhatia commented Mar 24, 2014

Thanks for pointing this out.

What about supporting both options?

I've started to add support for blrules in bf4a2ec and am curios about your opinion.

PS. I could not find that blacklist support has been dropped all together. Using shorewall 4.5.21.6-1, i can still use the "blacklist" file to block traffic. Nevertheless, blrules support would be better.

@inkblot

This comment has been minimized.

Copy link
Owner

inkblot commented Mar 24, 2014

The documentation claims that support for the blacklist file is dropped as of 4.5.7, but I have not verified that this is actually true.

I have looked at bf4a2ec, but I need some time to think about it. I'm not comfortable with the shorewall_version fact, since the module is responsible for the version of shorewall that is installed. The support you've added for the blrules file itself looks fine, though.

@raoulbhatia

This comment has been minimized.

Copy link
Contributor

raoulbhatia commented Mar 27, 2014

Hi!

is there any other option to fetch the version of an installed package?

I am quite new to puppet and therefore am not aware of all its features.

Moreover, as blrules seems to be quite similar to the "rules" file, maybe there is a possibility to reuse manifests/rule.pp ore give them a common code base.

What do you think?

Cheers,
Raoul

Add initial support for shorewall[6]-blrules
Beginning with Shorewall 4.4.25, the preferred method of blacklisting and whitelisting is to use the blrules file."
@raoulbhatia

This comment has been minimized.

Copy link
Contributor

raoulbhatia commented Mar 27, 2014

bugfix and rebase for using ipv4 and ipv6 side-by-side found in 7c5e604

@raoulbhatia

This comment has been minimized.

Copy link
Contributor

raoulbhatia commented Apr 2, 2014

Hi!

I was thinking about the fact today and still come to the conclusion, that using shorewall version is the correct way to determine the shorewall version which is running.

It is true that the module is responsible for managing the package but there might be a difference in the version string which is used by the package manager because distributions may alter the version string of a pacakge, e.g. dpkg -l samba reports 2:4.1.6+dfsg-1 for Samba 4.1.6 with a leading "2" and an additional version suffix.

Thus, using the software itself to get the version information seems most appropriate.

Please let me know if there is any method present within puppet or any other standard library which i am not aware of.

Cheers,
Raoul

@inkblot

This comment has been minimized.

Copy link
Owner

inkblot commented Apr 13, 2014

I'm going to merge this, but I need to make a small change first, so it'll be a merge of a new branch.

Some minor fixups before merging
- Move static headers to files and source them
- Fix a typo in the header name
- Can't reset a puppet manifest value
@inkblot

This comment has been minimized.

Copy link
Owner

inkblot commented Apr 13, 2014

If you could please merge my add-shorewall-blacklist branch to your master, I'll merge this pull request.

@raoulbhatia

This comment has been minimized.

Copy link
Contributor

raoulbhatia commented Apr 13, 2014

I've merged your branch but without any time for a review.

I hope that everything is ok now.
Thanks,
Raoul

@inkblot

This comment has been minimized.

Copy link
Owner

inkblot commented Apr 13, 2014

Sorry, one more commit. Could you merge again, please?

@raoulbhatia

This comment has been minimized.

Copy link
Contributor

raoulbhatia commented Apr 13, 2014

On 13 April 2014 17:21:45 CEST, Nate Riffe notifications@github.com wrote:

Sorry, one more commit. Could you merge again, please?


Reply to this email directly or view it on GitHub:
#3 (comment)

Done

Raoul Bhatia

inkblot added a commit that referenced this pull request Apr 13, 2014

@inkblot inkblot merged commit c8ea705 into inkblot:master Apr 13, 2014

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