(SEC-944) Handle duplicate system rules #1030
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
david22swan
previously approved these changes
Feb 21, 2022
chelnak
force-pushed
the
SEC-944-handle_duplicate_rule_comments_v2
branch
2 times, most recently
from
8acd01a
to
c05cdaa
Compare
chelnak
force-pushed
the
SEC-944-handle_duplicate_rule_comments_v2
branch
from
c05cdaa
to
7f25c62
Compare
In certain situations it is possible for an unmanaged rule to exist on the target system that has the same comment as the rule specified in the manifest. When this condition is true puppet will ignore the the unmanaged rule and continue to apply the rule in the manifest. This is because the firewall module uses the comment field in IPT as it's namevar and therefore expects it to be a unique identifier. In the case of IPT this is not true given that you can have multiple rules with the same comment. This commit adds a check that will identify system rules that have their comment field set to the same value as a rule in the manifest. If we enter a situation where any of the duplicate counts are greater than 1 then we will respond with a configurable action. The behaviour of this can be configured via the onduplicaterulebehaviour parameter.
chelnak
force-pushed
the
SEC-944-handle_duplicate_rule_comments_v2
branch
from
7f25c62
to
91d4d48
Compare
michaeltlombardi
suggested changes
Feb 22, 2022
Here we add a new parameter that determines how the puppet run will behave if a duplicate system rule is encountered. The default is to warn and continue.
chelnak
force-pushed
the
SEC-944-handle_duplicate_rule_comments_v2
branch
from
91d4d48
to
1c9914f
Compare
This commit adds a new section to inform users about how the module will behave when it encounters duplicate rules. It also inclues a small bit of house keeping.
|
@michaeltlombardi README updated 👍 |
Co-authored-by: Michael T Lombardi (He/Him) <michael.lombardi@puppet.com>
michaeltlombardi
previously approved these changes
Feb 22, 2022
chelnak
force-pushed
the
SEC-944-handle_duplicate_rule_comments_v2
branch
2 times, most recently
from
aff7a87
to
5487964
Compare
Prior to this commit there we no test cases to validate our changes to the module. This commit adds test cases for each of the configurations for onduplicaterulebehaviour.
chelnak
force-pushed
the
SEC-944-handle_duplicate_rule_comments_v2
branch
from
5487964
to
30db99b
Compare
|
#1031 raised for unrelated failing tests |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
In certain situations it is possible for an unmanaged rule to exist on the target system that has the same comment as the rule specified in the manifest.
When this condition is true puppet will ignore the the unmanaged rule and continue to apply the rule in the manifest. This is because the firewall module uses the comment field in IPT as it's namevar and therefore expects it to be a unique identifier. In the case of IPT this is not true given that you can have multiple rules with the same comment.
What has changed?
This commit adds a check that will identify system rules that have their comment field set to the same value as a rule in the manifest. If we enter a situation where any of the duplicate counts are greater than 1 then we will respond with a configurable action. The behaviour of this can be configured via the onduplicaterulebehaviour parameter.