Fix HTTP OPTIONS request auth handling by jon-wei · Pull Request #5638 · apache/druid

jon-wei · 2018-04-13T00:08:59Z

This PR sets the AUTHORIZATION_CHECKED flag to true for all HTTP OPTIONS requests. This is handled in a new servlet filter (Druid has no explicit OPTIONS method handlers on its endpoints).

It seems that CORS pre-flight OPTIONS requests do not generally contain credentials headers:

So a disableHttpOptionsAuthentication config is added to disable authentication checks on OPTIONS requests (authentication is required by default) to support CORS use cases.

jon-wei · 2018-04-13T00:09:46Z

I merged #5615 too early, so reopening in this PR.

This addresses the comments left on the last PR.

gianm · 2018-04-13T00:35:17Z

 |`druid.escalator.type`|String|Type of the Escalator that should be used for internal Druid communications. This Escalator must use an authentication scheme that is supported by an Authenticator in `druid.auth.authenticationChain`.|"noop"|no|
 |`druid.auth.authorizers`|JSON List of Strings|List of Authorizer type names |["allowAll"]|no|
 |`druid.auth.unsecuredPaths`| List of Strings|List of paths for which security checks will not be performed. All requests to these paths will be allowed.|[]|no|
+|`druid.auth.allowUnauthenticatedHttpOptions`|Boolean|If true, skip authentication checks for HTTP OPTIONS requests. This is needed for certain use cases, such as supporting CORS pre-flight requests. Note that disabling authentication checks for OPTIONS requests will allow unauthenticated users to determine what Druid endpoints are valid (by checking if the OPTIONS request returns a 200 instead of 404), so the authentication checks should not be disabled unless necessary. |false|no|


Will this leak any info beyond what is available in the docs?

Is there any difference in the response of an OPTIONS request for e.g. /druid/coordinator/v1/datasources/{dataSource}/intervals for a real datasource vs a not-real datasource? Or other endpoints like this that include dataSource names in the resource?

The "does resource that appears in a URL exist?" leakage won't happen, the responses for the OPTIONS requests are the same:

Valid datasource:

curl -i -X OPTIONS http://localhost:8081/druid/coordinator/v1/datasources/wikiticker/intervals HTTP/1.1 200 OK Date: Fri, 13 Apr 2018 00:45:34 GMT Content-Type: application/vnd.sun.wadl+xml Allow: OPTIONS,HEAD,GET Last-Modified: Fri, 13 Apr 2018 00:21:06 UTC Vary: Accept Content-Length: 711 Server: Jetty(9.3.19.v20170502) <?xml version="1.0" encoding="UTF-8" standalone="yes"?><application xmlns="http://wadl.dev.java.net/2009/02"><doc xmlns:jersey="http://jersey.java.net/" jersey:generatedBy="Jersey: 1.19.3 10/24/2016 03:43 PM"/><grammars/><resources base="http://localhost:8081/"><resource path="druid/coordinator/v1/datasources/wikiticker/intervals"><method id="getSegmentDataSourceIntervals" name="GET"><request><param xmlns:xs="http://www.w3.org/2001/XMLSchema" name="simple" style="query" type="xs:string"/><param xmlns:xs="http://www.w3.org/2001/XMLSchema" name="full" style="query" type="xs:string"/></request><response><representation mediaType="application/json"/></response></method></resource></resources></application>

Invalid datasource:

curl -i -X OPTIONS http://localhost:8081/druid/coordinator/v1/datasources/FAKE_DATASOURCES/intervals HTTP/1.1 200 OK Date: Fri, 13 Apr 2018 00:45:42 GMT Content-Type: application/vnd.sun.wadl+xml Allow: OPTIONS,HEAD,GET Last-Modified: Fri, 13 Apr 2018 00:21:06 UTC Vary: Accept Content-Length: 717 Server: Jetty(9.3.19.v20170502) <?xml version="1.0" encoding="UTF-8" standalone="yes"?><application xmlns="http://wadl.dev.java.net/2009/02"><doc xmlns:jersey="http://jersey.java.net/" jersey:generatedBy="Jersey: 1.19.3 10/24/2016 03:43 PM"/><grammars/><resources base="http://localhost:8081/"><resource path="druid/coordinator/v1/datasources/FAKE_DATASOURCES/intervals"><method id="getSegmentDataSourceIntervals" name="GET"><request><param xmlns:xs="http://www.w3.org/2001/XMLSchema" name="simple" style="query" type="xs:string"/><param xmlns:xs="http://www.w3.org/2001/XMLSchema" name="full" style="query" type="xs:string"/></request><response><representation mediaType="application/json"/></response></method></resource></resources></application>

The unauthenticated OPTIONS requests would leak information about what features are enabled in some cases.

For example, it's possible to determine whether JSON SQL queries are enabled:

Enabled:

curl -i -X OPTIONS http://localhost:8082/druid/v2/sql HTTP/1.1 200 OK Date: Fri, 13 Apr 2018 01:20:29 GMT Content-Type: application/vnd.sun.wadl+xml Allow: OPTIONS,POST Last-Modified: Fri, 13 Apr 2018 01:16:58 UTC Vary: Accept Content-Length: 502 Server: Jetty(9.3.19.v20170502)

Disabled (No POST method, shows the query DELETE method from /druid/v2/{id} instead)

curl -i -X OPTIONS http://localhost:8082/druid/v2/sql HTTP/1.1 200 OK Date: Fri, 13 Apr 2018 01:21:08 GMT Content-Type: application/vnd.sun.wadl+xml Allow: OPTIONS,DELETE Last-Modified: Fri, 13 Apr 2018 01:21:07 UTC Vary: Accept Content-Length: 444 Server: Jetty(9.3.19.v20170502)

Other examples would be extensions that expose some endpoints when enabled, e.g. druid-basic-security:

With extension enabled:

curl -i -X OPTIONS http://localhost:8081/druid-ext/basic-security/authorization/ HTTP/1.1 200 OK Date: Fri, 13 Apr 2018 01:15:49 GMT Content-Type: application/vnd.sun.wadl+xml Allow: OPTIONS Last-Modified: Fri, 13 Apr 2018 01:13:22 UTC Vary: Accept Content-Length: 4440 Server: Jetty(9.3.19.v20170502) <?xml version="1.0" encoding="UTF-8" standalone="yes"?><application xmlns="http://wadl.dev.java.net/2009/02"><doc xmlns:jersey="http://jersey.java.net/" jersey:generatedBy="Jersey: 1.19.3 10/24/2016 03:43 PM"/><grammars/><resources base="http://localhost:8081/"><resource path="druid-ext/basic-security/authorization/"><resource path="/db/{authorizerName}/users/{userName}"> ...

With extension disabled:

curl -i -X OPTIONS http://localhost:8081/druid-ext/basic-security/authorization/ HTTP/1.1 404 Not Found Date: Fri, 13 Apr 2018 01:17:30 GMT Content-Length: 0 Server: Jetty(9.3.19.v20170502)

In that case I'd suggest wording like this,

If true, skip authentication checks for HTTP OPTIONS requests. This is needed for certain use cases, such as supporting CORS pre-flight requests. Note that disabling authentication checks for OPTIONS requests will allow unauthenticated users to determine what Druid endpoints are valid (by checking if the OPTIONS request returns a 200 instead of 404), so enabling this option may reveal information about server configuration, including information about what extensions are loaded (if those extensions add endpoints).

Sounds good, updated to use that wording

gianm · 2018-04-13T19:16:25Z

+    // so this filter catches all OPTIONS requests and authorizes them.
+    if (HttpMethod.OPTIONS.equals(httpReq.getMethod())) {
+      if (allowUnauthenticatedHttpOptions) {
+        httpReq.setAttribute(


This will override authentication information if it is present in the request. But ideally we'd retain that if it's present. Is there a way to do that?

Yes, I moved AllowOptionsResourceFilter so that it runs after all the authenticators, and had it check for existing authentication results

gianm · 2018-04-13T19:19:16Z

+            AuthConfig.DRUID_AUTHENTICATION_RESULT,
+            new AuthenticationResult(AuthConfig.ALLOW_ALL_NAME, AuthConfig.ALLOW_ALL_NAME, null)
+        );
+      }


I think it would be be better to else deny-this-request here rather than doing nothing and letting the catch-all filter catch it.

Added an error response here

gianm · 2018-04-13T19:19:58Z

    this.authenticatorChain = authenticationChain;
    this.authorizers = authorizers;
    this.unsecuredPaths = unsecuredPaths == null ? Collections.emptyList() : unsecuredPaths;
+    this.allowUnauthenticatedHttpOptions = allowUnauthenticatedHttpOptions == null


This can be simplified by making the type boolean instead of Boolean. Jackson defaults to filling those in as false.

made this boolean

gianm

LGTM

gianm · 2018-04-17T01:10:07Z

@jon-wei are you planning to do the backport?

* Fix HTTP OPTIONS request auth handling * PR comment * More PR comments * Fix * PR comment

* Fix HTTP OPTIONS request auth handling (#5638) * Fix HTTP OPTIONS request auth handling * PR comment * More PR comments * Fix * PR comment * Compile fixes * Compile

…he#5654) * Fix HTTP OPTIONS request auth handling (apache#5638) * Fix HTTP OPTIONS request auth handling * PR comment * More PR comments * Fix * PR comment * Compile fixes * Compile

* Fix HTTP OPTIONS request auth handling * PR comment * More PR comments * Fix * PR comment

Fix HTTP OPTIONS request auth handling

93ac36c

jon-wei added Bug Security labels Apr 13, 2018

jon-wei added this to the 0.12.1 milestone Apr 13, 2018

gianm reviewed Apr 13, 2018

View reviewed changes

PR comment

b6a6509

gianm reviewed Apr 13, 2018

View reviewed changes

jon-wei added 2 commits April 13, 2018 19:04

More PR comments

8ec0b06

Fix

3ee07aa

jon-wei force-pushed the http_options_auth branch from 54707dc to 3ee07aa Compare April 16, 2018 19:48

PR comment

c93e616

gianm approved these changes Apr 17, 2018

View reviewed changes

gianm merged commit d0b66a6 into apache:master Apr 17, 2018

jon-wei added a commit to jon-wei/druid that referenced this pull request Apr 17, 2018

Fix HTTP OPTIONS request auth handling (apache#5638)

88be9f6

* Fix HTTP OPTIONS request auth handling * PR comment * More PR comments * Fix * PR comment

jon-wei mentioned this pull request Apr 17, 2018

[Backport] Fix HTTP OPTIONS request auth handling (#5638) #5654

Merged

sathishsri88 pushed a commit to sathishs/druid that referenced this pull request May 8, 2018

Fix HTTP OPTIONS request auth handling (apache#5638)

9579368

* Fix HTTP OPTIONS request auth handling * PR comment * More PR comments * Fix * PR comment

Conversation

jon-wei commented Apr 13, 2018

Uh oh!

jon-wei commented Apr 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jon-wei Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gianm left a comment

Choose a reason for hiding this comment

Uh oh!

gianm commented Apr 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jon-wei Apr 16, 2018 •

edited

Loading