Fix HTTP OPTIONS request auth handling#5615
Conversation
jon-wei
force-pushed
the
http_options_auth
branch
from
46b3b85 to
e8465d5
Compare
jon-wei
force-pushed
the
http_options_auth
branch
from
e8465d5 to
b43dcc7
Compare
| |`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.requireHttpOptionsAuthentication`|Boolean|If false, skip authentication checks for HTTP OPTIONS requests. Note that disabling authentication checks for OPTIONS requests will allow unauthenticated users to determine what Druid endpoints are valid, and this may leak sensitive information (for example, the `/druid/indexer/v1//task/{taskid}` endpoint on the Overlord and `/druid/coordinator/v1/datasources` endpoints on the Coordinator contain resource names), so the authentication checks should not be disabled unless truly necessary. |true|no| |
There was a problem hiding this comment.
Property name disableHttpOptionsAuthentication (by default set false due to the potential information leak) seems to be better IMHO. If someone wants to change the property to true it will mean that some kind of authentication will be disabled (dangerous in general).
There was a problem hiding this comment.
sounds good, I flipped the boolean there
|
Thanks for the investigation and fix! Mentioned links are really good lecture. |
|
@jon-wei is this 0.12.1 or 0.13? |
|
@fjy let's do 0.12.1 |
| |`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.disableHttpOptionsAuthentication`|Boolean|If true, skip authentication checks for HTTP OPTIONS requests. Note that disabling authentication checks for OPTIONS requests will allow unauthenticated users to determine what Druid endpoints are valid, and this may leak sensitive information (for example, the `/druid/indexer/v1//task/{taskid}` endpoint on the Overlord and `/druid/coordinator/v1/datasources` endpoints on the Coordinator contain resource names), so the authentication checks should not be disabled unless truly necessary. |false|no| |
There was a problem hiding this comment.
- Should be
/druid/indexer/v1/task(typo with the slashes). - The docs should explain when you might want to set this to true: for example, if you want to use CORS.
druid.auth.allowUnauthenticatedHttpOptionsmight be a better name.- I'd remove the "truly", it sounds a bit too scary.
I'd replace the parenthetical,
(for example, the
/druid/indexer/v1//task/{taskid}endpoint on the Overlord and/druid/coordinator/v1/datasourcesendpoints on the Coordinator contain resource names)
With,
(for example, callers can attempt to enumerate valid datasources via OPTIONS requests to
/druid/coordinator/v1/datasources/{dataSource}/intervalsendpoints on the Coordinator)
Although is this true? Will anything be leaked? It seems strange to me that Jetty would be clairvoyant enough to know whether a dataSource embedded in the resource is valid or not, at the time it serves the OPTIONS request.
There was a problem hiding this comment.
Yeah, you're right, at the time I wrote that section I was checking to see what authenticated but unauthorized users could see (authenticated users can determine if a resource exists with non-OPTIONS requests in some cases by checking for access denied vs. resource not found), and incorrectly made that assumption for unauthenticated OPTIONS requests.
I changed the doc there to mention a more general and less serious caveat about determining valid endpoints by checking for 200 vs 404 status, which you can do with the unauthenticated OPTIONS requests.
* Fix HTTP OPTIONS request auth handling * Flip configuration boolean
4 participants
Fixes #5588
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
disableHttpOptionsAuthenticationconfig is added to disable authentication checks on OPTIONS requests (authentication is required by default) to support CORS use cases.