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

Move ICMP stuff to separate classes allowing better customisation #16

Merged
merged 1 commit into from
Nov 24, 2020

Conversation

nbarrientos
Copy link
Collaborator

@nbarrientos nbarrientos commented Nov 20, 2020

This is a proposal to finish off #9. The patch moves all ICMP traffic management away from the global chain, creating specific input and output rules. RHEL8's defaults are followed.

Users can customise the ICMP4/6 types to allow by providing values via Hiera to nftables::rules::icmp::v4_types and other similar parameters. One rule per type is added so it should be possible to add rates as part of the type, for instance:

 nftables::rules::icmp::v4_types:
  - "echo-request limit rate 4/second"

ICMP rule handling can completely be disabled by tweaking nftables::in_icmp and nftables::out_icmp.

Tests will be added if the proposed interface goes forward.

Closes #9

@nbarrientos nbarrientos changed the title Move ICMP stuff to separate classes allowing better customisation Draft: Move ICMP stuff to separate classes allowing better customisation Nov 20, 2020
@nbarrientos nbarrientos marked this pull request as draft November 20, 2020 09:32
@nbarrientos nbarrientos changed the title Draft: Move ICMP stuff to separate classes allowing better customisation Move ICMP stuff to separate classes allowing better customisation Nov 20, 2020
@LionelCons
Copy link

API LGTM.

@nbarrientos
Copy link
Collaborator Author

nbarrientos commented Nov 20, 2020

Tests added (won't pass due to #15 blocking).

@nbarrientos
Copy link
Collaborator Author

Rebased, tests green and ready to be reviewed. Thanks!

@keachi keachi self-requested a review November 21, 2020 22:53
Copy link
Collaborator

@keachi keachi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@keachi keachi requested a review from duritong November 21, 2020 22:54
@duritong duritong merged commit 9246192 into voxpupuli:master Nov 24, 2020
@traylenator traylenator added the enhancement New feature or request label Dec 10, 2020
figless pushed a commit to figless/puppet-nftables that referenced this pull request Aug 25, 2021
9246192 Merge pull request voxpupuli#16 from cernops/icmp
587e522 Merge pull request voxpupuli#20 from cernops/firewalld_mask
ae9872e Make masking Service['firewalld'] configurable
79e9a23 Move ICMP stuff to separate classes

git-subtree-dir: code
git-subtree-split: 9246192
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The global chain contains INPUT specific rules
5 participants