Skip to content

Fix firewall rule discovery so quoted values are not scanned as real iptables options.#1275

Closed
ashishrase wants to merge 1 commit intopuppetlabs:mainfrom
ashishrase:bugfix/proto-number-to-name-malformed-token
Closed

Fix firewall rule discovery so quoted values are not scanned as real iptables options.#1275
ashishrase wants to merge 1 commit intopuppetlabs:mainfrom
ashishrase:bugfix/proto-number-to-name-malformed-token

Conversation

@ashishrase
Copy link

@ashishrase ashishrase commented Mar 10, 2026

Summary
Fix firewall rule discovery so quoted values are not scanned as real iptables options.

Today rule_to_hash in lib/puppet/provider/firewall/firewall.rb parses the full raw iptables-save line with regexes for most attributes. That means flag-like text inside quoted values, such as a comment containing -p -p or ! -p -p, can be misread as a real protocol flag. Once that happens, process_get receives a malformed proto value and discovery can later fail with Unsupported proto number: -p.

This change fixes the parser at the source by:

  • creating a quote-stripped search copy of the rule for non-quoted attributes
  • continuing to parse quoted attributes (name, string, string_hex, bytecode, u32, nflog_prefix, log_prefix) from the original rule
  • preserving existing behavior for real protocol flags such as -p tcp
  • preserving the existing proto => all fallback when no real protocol is present

Also adds provider test coverage for:

  • unit parsing: quoted comments containing -p -p do not set proto
  • unit parsing: quoted comments containing ! -p -p do not set proto
  • unit parsing: a real -p tcp outside quotes still parses as proto => tcp
  • end-to-end parsing: get returns proto => all when no real protocol is present and does not surface -p / ! -p as protocol values

Additional Context
[ X ] Root cause and the steps to reproduce. (If applicable)
[ X ] Thought process behind the implementation.

Root cause / reproduction
On affected systems (Rancher Kubernetes), iptables-save output may include quoted comment text containing flag-like tokens such as -p -p. Because rule_to_hash scans the full raw rule line, that quoted text can be mistaken for a real protocol option and stored as proto => '-p'. Later in the discovery pipeline this can surface as:

Unsupported proto number: -p

This can cause Puppet::Type.type(:firewall).instances to fail during rule discovery.

Thought process behind the implementation
The goal of this change is to fix the root cause in the provider parser rather than make downstream protocol conversion more tolerant of malformed intermediate values.

process_get already handles a missing protocol by defaulting to all, so the correct fix is to stop rule_to_hash from manufacturing invalid protocol values in the first place. Using a quote-stripped search copy for non-quoted attributes keeps the parser behavior intact for real rule options while preventing quoted comment text from being interpreted as flags.

Related Issues (if any)
No linked issue yet.

Checklist
[ X ] 🟢 Spec tests.
[ ] 🟢 Acceptance tests:
Acceptance tests were not run locally because Docker/Litmus provisioning is not available in my environment.
[ X ] Manually verified. (For example puppet agent -t on an affected system)

@ashishrase ashishrase requested a review from a team as a code owner March 10, 2026 14:28
@CLAassistant
Copy link

CLAassistant commented Mar 10, 2026

CLA assistant check
All committers have signed the CLA.

@ashishrase ashishrase force-pushed the bugfix/proto-number-to-name-malformed-token branch from 3b535d0 to 3492bb8 Compare March 11, 2026 12:07
@ashishrase ashishrase changed the title Make proto_number_to_name tolerant of malformed non-numeric proto tokens Fix firewall rule discovery so quoted values are not scanned as real iptables options. Mar 13, 2026
@ashishrase ashishrase force-pushed the bugfix/proto-number-to-name-malformed-token branch from 12f08e3 to 55b1ed8 Compare March 13, 2026 19:33
rule_to_hash scans raw iptables-save output with whole-line regexes.
Quoted values such as comments can contain text like `-p -p`, which may be
misread as a real protocol token and later surface as `Unsupported proto
number: -p` during firewall rule discovery.

Use a quote-stripped search copy of the rule for non-quoted attributes,
while still parsing quoted attributes from the original rule. This fixes
the parser at the source, preserves real protocol parsing, and keeps the
existing `proto => all` fallback when no protocol is set.

Add provider specs covering quoted `-p` / `! -p` comment text and the
end-to-end `get` behavior.
@ashishrase ashishrase force-pushed the bugfix/proto-number-to-name-malformed-token branch from 55b1ed8 to 726bef0 Compare March 14, 2026 08:02
@ashishrase ashishrase closed this Mar 14, 2026
@ashishrase
Copy link
Author

Need few more test to be done

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants