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.

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

Ensure the default state for new installs is more secure #290

Merged
merged 5 commits into from
Dec 13, 2023
Merged

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Dec 5, 2023

Description of the Change

In #198 we introduced some new filters that can be used to make RSA more secure. These filters do two things:

  1. Allow you to set which HTTP headers to trust
  2. Allow you to set which proxy IP addresses to trust

The idea being that if you want to trust any HTTP headers besides REMOTE_ADDR, more than likely you're doing this because your site is behind a proxy. In this situation, you should only trust those additional headers if the request is coming from your proxy IP address, which can be set with that second filter.

But all of this is disabled by default and has to be enabled through the use of these filters. This means for sites that use IP restriction, there is a chance of an IP spoofing attack being used.

As such, we've decided to make the core behavior more secure and allow individual sites to opt-in to additional HTTP headers using the filter, instead of the opposite approach we have now.

That said, we also want to maintain backwards compatibility so for any sites that are currently configured to use RSA, they will continue to use the existing list of HTTP headers. They can use the filter to change that list (and are recommended to do so), for instance removing those headers entirely by doing:

add_filter( 'rsa_trusted_headers', '__return_empty_array' );

But any new installs or newly configured installs will have to use that filter to set additional headers as needed. And as mentioned above, if trusting any additional headers, ideally you should be using the rsa_trusted_proxies filter to set a list of trusted proxy IP addresses, so the additional headers will only be used if one of those IP addresses matches.

Closes #195

How to test the Change

  1. WIth the latest released version of RSA, configure it to restrict access based on IP addresses
  2. Enter at least one IP address that is allowed but ensure this isn't your IP address
  3. Try to access the site (without being logged in) and notice access is restricted
  4. In your HTTP request client of choice (Postman or direct curl requests for instance) make a request to your site but make sure you set one of the default HTTP headers to match your IP address. For instance: curl --location --request POST 'https://rsa.test' --header 'CF-Connecting-Ip: 127.0.0.2'
  5. Note you should gain access to the site even though your actual IP address isn't allowed
  6. Now checkout the changes in this PR
  7. To simulate a new instance, set the option rsa_activation_version to 7.5.0 on whatever site you're testing on
  8. Run through the steps above again and notice you'll no longer be able to access the site in either scenario
  9. Add the following code, replacing the HTTP header name with whatever header you're testing
add_filter(
	'rsa_trusted_headers',
	function( $trusted_headers ) {
		return array_merge( array( 'HTTP_CF_CONNECTING_IP' ), $trusted_headers );
	}
);
  1. Run through the steps again and notice you should have access now when spoofing
  2. Remove the rsa_activation_version option and and the code added above and run through the steps again. This simulates an existing install. Note you should be able to access the site when spoofing
  3. Add the following code:
add_filter(
	'rsa_trusted_headers',
	'__return_empty_array'
);
  1. Note you should no longer have access when spoofing

Changelog Entry

Security - For new installs, ensure we only trust the REMOTE_ADDR HTTP header by default. Existing installs will still utilize the old list of approved headers but can modify this (and are recommended to) by using the rsa_trusted_headers filter

Credits

Props @dkotter, @peterwilsoncc, @dustinrue, @mikhail-net

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

…hy this is the case and how to utilize filters to get around this
…on't have that stored yet and also aren't configured yet. Use this value to determine what our default set of trusted headers is. This allows us to keep backwards compatibility with existing sites that are configured
@dkotter dkotter added this to the 7.5.0 milestone Dec 5, 2023
@dkotter dkotter self-assigned this Dec 5, 2023
@dkotter dkotter requested a review from Sidsector9 as a code owner December 5, 2023 22:00
@github-actions github-actions bot added the needs:code-review This requires code review. label Dec 5, 2023
@dkotter dkotter removed the request for review from Sidsector9 December 5, 2023 22:02
@dkotter dkotter requested a review from peterwilsoncc December 5, 2023 22:12
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

This looks good to me.

Testing notes:

  1. Activate on develop branch
  2. Using the mod header extension, set an X-Forwarded-For header to another, faked, IP address
  3. In RSA settings, allow unrestricted access to the fake IP address
  4. In a private window, confirm I was able to access the site.
  5. Update the activation option via wp cli: wp option update rsa_activation_version 7.5.0
  6. Switch to this branch
  7. Confirm I was restricted from accessing the site in the private window.

@dkotter dkotter mentioned this pull request Dec 6, 2023
17 tasks
@dkotter dkotter merged commit a4ae27a into develop Dec 13, 2023
@dkotter dkotter deleted the fix/195-v2 branch December 13, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:code-review This requires code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin is open to IP address spoofing
2 participants