Add support for parsing and using --tcp-option #1126
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
greatflyingsteve
force-pushed
the
support-tcp-option
branch
from
e7fe3dc to
50d5615
Compare
Closed
Rubocop's maximum value for allowed block length needs to be incremented by hand each time the 'firewall' type gains a new feature, because the maximum block length is hand-tuned to the size of that 'newtype' block. Having the 'Metrics/BlockLength' cop instead ignore lengths for the 'newtype' and 'provider' methods (which inherently have very long blocks) instead solves the problem for all 'newtype' and 'provider' blocks at one fell swoop, and with addition of one inline disable comment for the 'validate' block in the 'firewall' type, means all remaining block length warnings are squelched by a max value of only 64 lines.
Add the ability to parse iptables rules we encounter that include the '--tcp-option' flag, instead of issuing a "Skipping unparseable iptables rule" warning. Every Firewall resource requires a full parse of all rules, and the warning appears each time the problem rule is parsed; therefore these warning messages are noisy (especially on long rule sets), and there is no way to work around this, make the parser ignore the rule, or create other rules with Puppet in any order aside from "above the unparseable rule."
Add support (and unit tests) for creating rules that use iptables' '--tcp-option' flag to match the presence or non-presence of numbered TCP options. Remove special casing from '--tcp-flags', previously the only argument from the TCP match extension other than '--dport' and '--sport', and have the TCP match extension treated as a module only during rule assembly, because due to its inconsistent usage in iptables-save output when multiport portspecs are used, '-m tcp' or '-m udp' will throw off parsing badly if preset. Resolves puppetlabs#1124
Move tests for the ip6tables provider into the ip6tables provider unit test file. Even if it's subclassed from iptables, it should be reasonable to expect running all tests in the ip6tables provider's tests file to include all tests for the ip6tables provider. Also update several test definitions that had fallen out of sync with the iptables provider's test framework so the test cases for ip6tables are also no longer sensitive to specific hash order and stringified 'true'.
Provide the same support for rule parse and create for IPv6 as the previous commit provided for IPv4
greatflyingsteve
force-pushed
the
support-tcp-option
branch
from
50d5615 to
a82cd69
Compare
3 tasks
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.
As it says on the tin. Also untangles the snarl of logic around
--tcp-flags, previously the only stand-alone option to the 'tcp' match extension (portspecs are treated specially).--tcp-flagswas trying to eat the-m tcpin its match, which because it falls after--tcp-optionin the iptables-save output if both are present, causes a whole lot of trouble in recognizing that one or the other is present. It turns out to be simpler instead just to strip-m tcpand-m udpout of the lines entirely and parse based on that, which works reliably. Testing shows that a match extension for a protocol can never be loaded when a different protocol (including multiple protocols that include the correct protocol, e.g.-p allor! -p icmp) is specified, so there is no way for a rule containing-p tcp -m udpto ever appear in the output of iptables-save.As an added bonus, also fixes the configuration for the Metrics/BlockLength cop, so that the max block length doesn't need to be constantly hand-incremented each time the Firewall type is given new features.
Additional info in the commit messages, but please ask about anything I forgot to include or messed up!
Resolves #1124