Add explicit port params for nat_port_forward#225
Open
aijayadams wants to merge 1 commit intopfsensible:masterfrom
Open
Add explicit port params for nat_port_forward#225aijayadams wants to merge 1 commit intopfsensible:masterfrom
aijayadams wants to merge 1 commit intopfsensible:masterfrom
Conversation
This file contains hidden or 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
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.
Add explicit port parameters for nat_port_forward
Fixes #59
Summary
This PR adds explicit
source_portanddestination_portparameters to thepfsense_nat_port_forwardmodule, completing the IPv6 support work started in #219 (PR #220).Problem
While #219 fixed IPv6 address parsing in firewall rules by treating IPv6 addresses as single units (not splitting on
:), this created a new problem for NAT port forwarding: there is no way to specify ports with IPv6 addresses.The old
:portsyntax (e.g.,address:port) has two issues:Already deprecated: The module warns: "the :ports syntax at end of addresses is deprecated and support will be removed soon. Please use source_port and destination_port options."
Incompatible with IPv6: IPv6 addresses contain colons (e.g.,
2001:db8::1). After the [pfsense_rule]: Cannot use IPv6 in source and destination #219 fix, the parser correctly treats2001:db8::1as a single address, making2001:db8::1:443ambiguous - is:443part of the address or a port?Proposed Change
This PR adds the parameters that the deprecation warning promised:
source_port: Explicit source port or range specificationdestination_port: Explicit destination port or range specificationThese parameters:
"8000-9000"):portsyntaxExamples
Before this patch (BROKEN for IPv6)
Problem: Cannot specify destination port. The old syntax
2001:db8:cafe::1:443doesn't work because after #219, IPv6 addresses are not split on:.After this patch (WORKS)
IPv4 benefits too
Implementation Details
Changes to
plugins/module_utils/nat_port_forward.py:source_portparameter toNAT_PORT_FORWARD_ARGUMENT_SPECdestination_portparameter toNAT_PORT_FORWARD_ARGUMENT_SPECis_port_or_alias()methodChanges to
plugins/modules/pfsense_nat_port_forward.py:source_portparameter with examplesdestination_portparameter with examplesBackward Compatibility
This change is fully backward compatible:
:portsyntax continue to workTesting
Tested with:
Example test cases are provided in:
Related Work
This PR completes the IPv6 support started in:
That work fixed IPv6 parsing in the
pfsense_rulemodule, butpfsense_nat_port_forwardneeded additional parameters to make port specifications work with IPv6.Notes for Reviewers
The implementation closely follows the pattern used in
pfsense_rulemodule and the existing address parsing infrastructure. The validation uses the existingis_port_or_alias()method to ensure consistency with other modules.References to Issue #219
The fix for #219 (commit 7d91ba4) introduced this code in
addresses.py:This correctly handles IPv6 addresses but eliminates the ability to use
:portsyntax with IPv6. The deprecation warning (line 148-149) already pointed toward using explicit parameters: