Security: Bypassing the IP authentication is easy?

Problem/Motivation

The module does not properly authenticate or protect REST endpoints.

If you have no patches applied: If you don't pass the expected Accept header, you immediately bypass IP-based authentication, because the IPConsumerAuth::applies function returns FALSE.

If you have a patch applied from #2942049: Prevents normal website access by restricting all routes by default or #3367992: Apply changes to apply security coverage: the IPConsumerAuth::applies improperly returns FALSE on the first call after sending a request, because the request doesn't find a matching route, and so the _format parameter won't be found or the header won't be checked, depending on which version you have.

Basically, if IPConsumerAuth::applies returns FALSE at all, then the IP address is not checked, meaning anyone can access the endpoint.

Steps to reproduce

1. composer require drupal/restui
2. drush en rest restui ip_consumer_auth
3. Either apply the changes at #3367992: Apply changes to apply security coverage, or modify the cURL commands below to use an Accept header instead of a _format param.
4. Create a custom REST endpoint, or enable a built-in one. I made a custom one. See attached CustomResource.php file.
5. Enable GET method, JSON response, IP Consumer Auth for authentication provider. Save.
6. Go to /admin/config/services/ip_consumer_auth
7. Enable for all formats. Use blacklist. Enter a bogus IP in the Blacklist, so that it should be enabled for all other IPs. Save.
8. Use xdebug and set a breakpoint in IPConsumerAuth::authenticate, or put something else so you know when it's called or not.
9. Clear cache with drush cr
10. Make sure the route works over cURL from the CLI.
11. curl -o - -I -X "GET" http://my_site.ddev.site/api/custom-resource?_format=json
12. Go to /admin/config/services/ip_consumer_auth
13. Change to use Whitelist. Keep the bogus IP, or try leaving the IP whitelist blank. So your IP should not be allowed. Save.
14. curl -o - -I -X "GET" http://my_site.ddev.site/api/custom-resource?_format=json
15. Are you still able to access it? You shouldn't be.

Also, if you applied the patch that lets you use _format in the URL query params, try this (leave _format blank):
curl -o - -I -X "GET" http://my_site.ddev.site/api/custom-resource?_format=
Doing this bypasses IP auth altogether, and IPConsumerAuth::authenticate is never called.

During all the steps above, make sure IPConsumerAuth::authenticate is called each time you run a cURL command.

It's super obvious if you have no patches installed, just look at the function:

  public function applies(Request $request) {
    // Only apply this validation if request has a valid accept value.
    $formats = $this->config->get('format');
    foreach ($formats as $format) {
      if (strstr($request->headers->get('Accept'), $format)) {
        return TRUE;
      }
    }
    return FALSE;
  }

Only apply this validation if request has a valid accept value.

All you have to do to bypass the IP-based authentication is to not include a valid accept value. That's no security at all.

Proposed resolution

Honestly, after a couple of days trying to get the module to work as-is or with patches, it just doesn't work. It doesn't properly prevent IPs from accessing the endpoint. Basically, the module has no tests included in the code, and it seems it hasn't been coded to work properly in all cases.

I think the proper solution is to have the module on for all "formats", putting return TRUE; in the IPConsumerAuth::applies function, and remove the settings to select the format.

  public function applies(Request $request) {
    return TRUE;
  }

And in ip_consumer_auth.services.yml, set priority to -100 (See #2942049: Prevents normal website access by restricting all routes by default):

    tags:
      - { name: authentication_provider, provider_id: ip_consumer_auth, priority: -100 }

If you're restricting an IP address's access to an endpoint, it's probably not because you care about the format; it's because you don't want anyone else to have access to the data at all. If an IP can access data in CSV but not JSON, they can just convert the data to JSON after accessing it.

Remaining tasks

1. Figure out everything that isn't working.
2. Determine whether the format checking code should be in the authenticate function, or removed completely.
3. Secure the module.
4. Write tests.

Add tests to the module in a follow-up issue, making sure that functionality works as described, blocking IPs and allowing others. Blocking for formats, allowing others. Making sure that access cannot be bypassed by using an empty format or some other trick. An authentication/access control module should be robust and tested and secure.

I don't have experience writing tests, so this would probably require some other users to step up.

User interface changes

API changes

Data model changes