WAF: Add support for handling IP ranges in allow/block lists #29131

nateweller · 2023-02-23T18:13:06Z

This PR introduces support for IP ranges in the firewall allow and block lists.

Ranges can be denoted by a dash - between two IP addresses. For example, 1.2.3.4-5.6.7.8 or 2001:0db8:85a3:0000:0000:0000:0000:0001-2001:0db8:85a3:0000:0000:0000:0000:9999

Proposed changes:

Remove Waf_Runner::ip_option_to_array() in favor of a new IP\Utils::get_ip_addresses_from_string() method, which takes the existing ip_option_to_array() code and adds support for IP ranges (i.e. "1.1.1.1-2.2.2.2").
Adds new IP\Utils::validate_ip_range() function to ensure the given IP addresses for a range are valid and ordered correctly.
Update Waf_Runtime::is_ip_in_array() to support IP ranges.

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

#28633 (comment)

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Add an IP range to your WAF IP block list, which includes your IP address i.e. "1.1.1.1-255.255.255.255".
Validate that the rules file at wp-content/jetpack-waf/rules/block-ip.php now includes the range in $waf_block_list.
Validate that your IP address is blocked from accessing the site.

…iven string with range support

github-actions · 2023-02-23T18:14:13Z

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run

bin/jetpack-downloader test jetpack add/waf-ip-ranges

to get started. More details: p9dueE-5Nn-p2

github-actions · 2023-02-23T18:14:33Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: March 7, 2023.
Scheduled code freeze: February 28, 2023.

Protect plugin:

Next scheduled release: March 7, 2023.
Scheduled code freeze: February 27, 2023.

ArSn

Only flew over this and have this one comment. Someone should certainly do a deeper review.

ArSn · 2023-02-24T12:51:01Z

projects/packages/ip/tests/php/test-utils.php

+		foreach ( $ips as $ips_index => $ip ) {
+			$string .= $ip;
+
+			$is_last_loop = $ips_index === count( $ips ) - 1;
+			if ( ! $is_last_loop ) {
+				$string          .= $delimiters[ $delimiters_index ];
+				$delimiters_index = count( $delimiters ) === $delimiters_index + 1 ? 0 : $delimiters_index + 1;
+			}
+		}


I'd certainly prefer if our expectation was hardcoded here, because this makes it pretty hard to understand what exactly we are expecting.

Thanks for checking this out Kolja! Good point - it is much more understandable by hardcoding it: 677b08d

dkmyta · 2023-02-24T16:03:28Z

projects/packages/ip/src/class-utils.php

+			// Validate both IP values from the range
+			$range = explode( '-', $ip );
+			if ( count( $range ) === 2 ) {
+				if ( filter_var( $range[0], FILTER_VALIDATE_IP ) !== false || filter_var( $range[1], FILTER_VALIDATE_IP ) !== false ) {


I think here we'll want && over || to determine that both the starting and ending addresses are valid. Otherwise, I believe things like 12.12.12.1-not.a.valid.ip and not.a.valid.ip-12.12.12.5 will pass through the filter.

I also wonder, during this filtering process, if is there anything else we need to check for as far as inaccuracies go before passing the range as valid. For example, is $range[0] in fact lower than, and not equal to $range[1]?

Nice catch, thanks Dean - I've added some additional validation which should cover everything you've mentioned: f4c2215 👍

dkmyta

Looks great, and works great! Covers all the bases - nice work! 👍🏻

nateweller · 2023-02-24T23:39:23Z

Looks great, and works great! Covers all the bases - nice work! 👍🏻

Thanks Dean! I'm now just trying to figure out why the continuous-integration/a8c-teamcity build is failing. It looks like the failures in D102785-code seem unrelated...

sdixon194

This works for me 👍 The wpcom teamcity failures are unrelated. More discussion here.

nateweller · 2023-02-27T22:13:01Z

Thanks all!

… lists (#29131)

nateweller added 5 commits February 23, 2023 10:46

Add a method to jetpack-ip Utils class to parse IP addresses from a g…

9cff414

…iven string with range support

Add support for IP ranges to Waf_Runtime::is_ip_in_array

2ef1be9

Integrate IP_Utils::get_ip_addresses_from_string into the WAF package

baf29be

Add test

b26e38c

Changelogs

f7ce0d0

nateweller requested a review from a team February 23, 2023 18:13

nateweller self-assigned this Feb 23, 2023

nateweller added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Package] WAF [Package] IP labels Feb 23, 2023

nateweller added this to the protect/1.3.0 milestone Feb 23, 2023

github-actions bot added the [Status] Needs Test Review label Feb 23, 2023

Fix project versions

0fe6efd

github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Plugin] Protect A plugin with features to protect a site: brute force protection, security scanning, and a WAF. labels Feb 24, 2023

nateweller added 2 commits February 23, 2023 19:35

Update test

1c9f3fb

Merge branch 'trunk' into add/waf-ip-ranges

892c987

ArSn reviewed Feb 24, 2023

View reviewed changes

dkmyta reviewed Feb 24, 2023

View reviewed changes

nateweller added 4 commits February 24, 2023 11:20

Ensure both addresses in a range are valid

c23e71d

Update test

677b08d

Add additional validation when extracting IP ranges from a string

f4c2215

Improve specificity of IP conversion checks in validate_ip_range

3edf570

nateweller added [Status] Needs Team Review and removed [Status] In Progress labels Feb 24, 2023

nateweller marked this pull request as ready for review February 24, 2023 18:54

nateweller requested a review from dkmyta February 24, 2023 18:54

github-actions bot added the [Status] In Progress label Feb 24, 2023

nateweller added 3 commits February 24, 2023 12:10

Fix explode()

218b293

Fix missing use

8a62784

Merge branch 'trunk' into add/waf-ip-ranges

28ff072

nateweller removed the [Status] In Progress label Feb 24, 2023

dkmyta approved these changes Feb 24, 2023

View reviewed changes

nateweller added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Team Review labels Feb 24, 2023

sdixon194 approved these changes Feb 27, 2023

View reviewed changes

nateweller merged commit 8fa35bc into trunk Feb 27, 2023

nateweller deleted the add/waf-ip-ranges branch February 27, 2023 22:12

github-actions bot removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Feb 27, 2023

nateweller mentioned this pull request Feb 27, 2023

WAF: Integrate the WAF IP allow list into the brute force protection feature #28633

Merged

3 tasks

donnchawp pushed a commit that referenced this pull request Mar 1, 2023

[not verified] WAF: Add support for handling IP ranges in allow/block…

f5e71a1

… lists (#29131)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WAF: Add support for handling IP ranges in allow/block lists #29131

WAF: Add support for handling IP ranges in allow/block lists #29131

nateweller commented Feb 23, 2023 •

edited

Loading

github-actions bot commented Feb 23, 2023 •

edited

Loading

github-actions bot commented Feb 23, 2023 •

edited

Loading

ArSn left a comment

ArSn Feb 24, 2023

nateweller Feb 24, 2023

dkmyta Feb 24, 2023 •

edited

Loading

nateweller Feb 24, 2023

dkmyta left a comment

nateweller commented Feb 24, 2023

sdixon194 left a comment

nateweller commented Feb 27, 2023

WAF: Add support for handling IP ranges in allow/block lists #29131

WAF: Add support for handling IP ranges in allow/block lists #29131

Conversation

nateweller commented Feb 23, 2023 • edited Loading

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented Feb 23, 2023 • edited Loading

github-actions bot commented Feb 23, 2023 • edited Loading

ArSn left a comment

Choose a reason for hiding this comment

ArSn Feb 24, 2023

Choose a reason for hiding this comment

nateweller Feb 24, 2023

Choose a reason for hiding this comment

dkmyta Feb 24, 2023 • edited Loading

Choose a reason for hiding this comment

nateweller Feb 24, 2023

Choose a reason for hiding this comment

dkmyta left a comment

Choose a reason for hiding this comment

nateweller commented Feb 24, 2023

sdixon194 left a comment

Choose a reason for hiding this comment

nateweller commented Feb 27, 2023

nateweller commented Feb 23, 2023 •

edited

Loading

github-actions bot commented Feb 23, 2023 •

edited

Loading

github-actions bot commented Feb 23, 2023 •

edited

Loading

dkmyta Feb 24, 2023 •

edited

Loading