Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WAF: Improve cross-compatibility of IP list options #38395

Merged
merged 3 commits into from
Jul 19, 2024
Merged

WAF: Improve cross-compatibility of IP list options #38395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f2f01b
Ensure cross compatibility of legacy jetpack_waf_ip_lists and and new…
nateweller Jul 17, 2024
69fc2fe
changelog
nateweller Jul 17, 2024
5cb95bf
Minor adjustments
nateweller Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions projects/packages/waf/changelog/waf-ip-list-additional-compatibility
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Improved backwards/cross compatibility of option deprecation related to IP block/allow lists.


16 changes: 8 additions & 8 deletions projects/packages/waf/src/class-compatibility.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,23 +244,23 @@ public static function is_brute_force_running_in_jetpack() {
* @return mixed The default value to return if the option does not exist in the database.
*/
public static function default_option_waf_ip_allow_list_enabled( $default, $option, $passed_default ) {
// Allow get_option() to override this default value
if ( $passed_default ) {
return $default;
}

// If the deprecated IP lists option was set to false, disable the allow list.
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
$deprecated_option = get_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, true );
$deprecated_option = Jetpack_Options::get_raw_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, true );
if ( ! $deprecated_option ) {
return false;
}

// If the allow list is empty, disable the allow list.
if ( ! get_option( Waf_Rules_Manager::IP_ALLOW_LIST_OPTION_NAME ) ) {
if ( ! Jetpack_Options::get_raw_option( Waf_Rules_Manager::IP_ALLOW_LIST_OPTION_NAME ) ) {
return false;
}

// Allow get_option() to override this default value
if ( $passed_default ) {
return $default;
}

// Default to enabling the allow list.
return true;
}
Expand All @@ -283,6 +283,6 @@ public static function default_option_waf_ip_block_list_enabled( $default, $opti
}

// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
return get_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, false );
return Jetpack_Options::get_raw_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, false );
}
}
31 changes: 17 additions & 14 deletions projects/packages/waf/src/class-rest-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,50 +103,53 @@ public static function waf() {
public static function update_waf( $request ) {
// Automatic Rules Enabled
if ( isset( $request[ Waf_Rules_Manager::AUTOMATIC_RULES_ENABLED_OPTION_NAME ] ) ) {
update_option( Waf_Rules_Manager::AUTOMATIC_RULES_ENABLED_OPTION_NAME, (bool) $request->get_param( Waf_Rules_Manager::AUTOMATIC_RULES_ENABLED_OPTION_NAME ) );
update_option( Waf_Rules_Manager::AUTOMATIC_RULES_ENABLED_OPTION_NAME, $request->get_param( Waf_Rules_Manager::AUTOMATIC_RULES_ENABLED_OPTION_NAME ) ? '1' : '' );
}

// IP Lists Enabled
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
if ( isset( $request[ Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME ] ) ) {
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
update_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, (bool) $request->get_param( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME ) );
/**
* IP Lists Enabled
*
* @deprecated $next-version$ This is a legacy option maintained here for backwards compatibility.
*/
if ( isset( $request['jetpack_waf_ip_list'] ) ) {
update_option( Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME, $request['jetpack_waf_ip_list'] ? '1' : '' );
update_option( Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME, $request['jetpack_waf_ip_list'] ? '1' : '' );
}

// IP Block List
if ( isset( $request[ Waf_Rules_Manager::IP_BLOCK_LIST_OPTION_NAME ] ) ) {
update_option( Waf_Rules_Manager::IP_BLOCK_LIST_OPTION_NAME, $request[ Waf_Rules_Manager::IP_BLOCK_LIST_OPTION_NAME ] );
}
if ( isset( $request[ Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME ] ) ) {
update_option( Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME, $request[ Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME ] );
update_option( Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME, $request[ Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME ] ? '1' : '' );
}

// IP Allow List
if ( isset( $request[ Waf_Rules_Manager::IP_ALLOW_LIST_OPTION_NAME ] ) ) {
update_option( Waf_Rules_Manager::IP_ALLOW_LIST_OPTION_NAME, $request[ Waf_Rules_Manager::IP_ALLOW_LIST_OPTION_NAME ] );
}
if ( isset( $request[ Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME ] ) ) {
update_option( Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME, $request[ Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME ] );
update_option( Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME, $request[ Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME ] ? '1' : '' );
}

// Share Data
if ( isset( $request[ Waf_Runner::SHARE_DATA_OPTION_NAME ] ) ) {
// If a user disabled the regular share we should disable the debug share data option.
if ( false === $request[ Waf_Runner::SHARE_DATA_OPTION_NAME ] ) {
update_option( Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME, false );
if ( ! $request[ Waf_Runner::SHARE_DATA_OPTION_NAME ] ) {
update_option( Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME, '' );
}

update_option( Waf_Runner::SHARE_DATA_OPTION_NAME, (bool) $request[ Waf_Runner::SHARE_DATA_OPTION_NAME ] );
update_option( Waf_Runner::SHARE_DATA_OPTION_NAME, $request[ Waf_Runner::SHARE_DATA_OPTION_NAME ] ? '1' : '' );
}

// Share Debug Data
if ( isset( $request[ Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME ] ) ) {
// If a user toggles the debug share we should enable the regular share data option.
if ( true === $request[ Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME ] ) {
update_option( Waf_Runner::SHARE_DATA_OPTION_NAME, true );
if ( $request[ Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME ] ) {
update_option( Waf_Runner::SHARE_DATA_OPTION_NAME, 1 );
}

update_option( Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME, (bool) $request[ Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME ] );
update_option( Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME, $request[ Waf_Runner::SHARE_DEBUG_DATA_OPTION_NAME ] ? '1' : '' );
}

// Brute Force Protection
Expand Down
10 changes: 4 additions & 6 deletions projects/packages/waf/src/class-waf-rules-manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,13 @@ public static function add_hooks() {
// Re-activate the WAF any time an option is added or updated.
add_action( 'add_option_' . self::AUTOMATIC_RULES_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'update_option_' . self::AUTOMATIC_RULES_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
add_action( 'add_option_' . self::IP_LISTS_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
add_action( 'update_option_' . self::IP_LISTS_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'add_option_' . self::IP_ALLOW_LIST_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'add_option_' . self::IP_ALLOW_LIST_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'update_option_' . self::IP_ALLOW_LIST_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'add_option_' . self::IP_ALLOW_LIST_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'update_option_' . self::IP_ALLOW_LIST_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'add_option_' . self::IP_BLOCK_LIST_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'add_option_' . self::IP_BLOCK_LIST_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'update_option_' . self::IP_BLOCK_LIST_ENABLED_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'add_option_' . self::IP_BLOCK_LIST_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
add_action( 'update_option_' . self::IP_BLOCK_LIST_OPTION_NAME, array( static::class, 'reactivate_on_rules_option_change' ), 10, 0 );
// Register the cron job.
add_action( 'jetpack_waf_rules_update_cron', array( static::class, 'update_rules_cron' ) );
Expand Down
2 changes: 1 addition & 1 deletion projects/packages/waf/src/class-waf-runner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public static function get_config() {
* @deprecated $next-version$
*/
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME => get_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME ),
Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME => get_option( Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME ) || get_option( Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To maintain consistency with other related operations, should we use Jetpack_Options::get_raw_option() here also?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting question! After double checking the logic, I don't think these should be changed.

I'm using get_option here so that it uses the default value hooks for the block and allow list. This ensures that if the deprecated option was set, and the new options are not, they will base their value on the old one and still return an accurate value 👍

In the compatibility class, I used get_raw_option to bypass those filters, in order to make those default value decisions based on the "true" value of the options in the database.

But at this point, in the runner class, we want those default values to be used 👍

);
}

Expand Down
33 changes: 25 additions & 8 deletions projects/packages/waf/tests/php/integration/test-waf-compatibility.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,27 @@ public function testAutomaticRulesOptionInheritsFromModuleStatus() {
* Test the default options for the IP allow and block lists.
*/
public function testIpListsDefaultOptions() {
$filter_option = function ( $option, $value ) {
return function ( $result, $query ) use ( $option, $value ) {
global $wpdb;

if ( $query === "SELECT option_value FROM $wpdb->options WHERE option_name = '$option' LIMIT 1" ) {
return array(
(object) array(
'option_value' => serialize( $value ),
),
);
}

return $result;
};
};
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
$filter_ip_list_option_true = $filter_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, true );
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
$filter_ip_list_option_false = $filter_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, false );
$filter_ip_allow_list_content = $filter_option( Waf_Rules_Manager::IP_ALLOW_LIST_OPTION_NAME, '1.2.3.4' );

// Enable the WAF module.
Waf_Runner::enable();

Expand All @@ -202,24 +223,20 @@ public function testIpListsDefaultOptions() {
$this->assertFalse( get_option( Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME ) );

// Add content to the allow list.
update_option( Waf_Rules_Manager::IP_ALLOW_LIST_OPTION_NAME, '1.2.3.4' );
add_filter( 'wordbless_wpdb_query_results', $filter_ip_allow_list_content, 10, 2 );

$this->assertTrue( get_option( Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME ) );
$this->assertFalse( get_option( Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME ) );

// Toggle the old generic option from true to false.
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
update_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, true );
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
update_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, false );
add_filter( 'wordbless_wpdb_query_results', $filter_ip_list_option_false, 10, 2 );

// Options default to false when the old generic option is set to false.
$this->assertFalse( get_option( Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME ) );
$this->assertFalse( get_option( Waf_Rules_Manager::IP_BLOCK_LIST_ENABLED_OPTION_NAME ) );

// Set the old generic option to true.
// @phan-suppress-next-line PhanDeprecatedClassConstant -- Needed for backwards compatibility.
update_option( Waf_Rules_Manager::IP_LISTS_ENABLED_OPTION_NAME, true );
remove_filter( 'wordbless_wpdb_query_results', $filter_ip_list_option_false, 10, 2 );
add_filter( 'wordbless_wpdb_query_results', $filter_ip_list_option_true, 10, 2 );

// Options default to true when the old generic option is set to true.
$this->assertTrue( get_option( Waf_Rules_Manager::IP_ALLOW_LIST_ENABLED_OPTION_NAME ) );
Expand Down
Loading