druid-pac4j: add ability to use custom ssl trust store while talking to auth server#9637
druid-pac4j: add ability to use custom ssl trust store while talking to auth server#9637himanshug merged 2 commits intoapache:masterfrom
Conversation
himanshug
force-pushed
the
pac4j_ssl
branch
3 times, most recently
from
4bf64e1 to
bbd4953
Compare
himanshug
changed the title
| |--------|---------------|-----------|-------|--------| | ||
| |`druid.auth.pac4j.cookiePassphrase`|passphrase for encrypting the cookies used to manage authentication session with browser. It can be provided as plaintext string or The [Password Provider](../../operations/password-provider.md).|none|Yes| | ||
| |`druid.auth.pac4j.readTimeout`|Socket connect and read timeout duration used when communicating with authentication server|PT5S|No| | ||
| |`druid.auth.pac4j.enableCustomSslContext`|Whether to use custom SSLContext setup via [simple-client-sslcontext](simple-client-sslcontext.md) extension which must be added to extensions list when this property is set to true.|false|No| |
There was a problem hiding this comment.
I think reusing simple-client-sslcontext is good for now since the client is the same entity (a Druid server); are there any potential cases where you'd want to use a different truststore or keystore when talking to the auth server here vs. another Druid server?
There was a problem hiding this comment.
thanks for looking, valid question.
I chose to go this route instead of adding separate config for a brand new ssl context to reduce amount of configuration user has. That said, I see that ldap doesn't use druid's common ssl context but builds one separately. I haven't personally seen the use cases for this separation yet but I am not a security expert and could be wrong :)
If we come across such use case, then I think we should add support for another SSLContext inside druid core code .. one for talking to external auth services (oauth server, ldap server etc) so that we don't repeat this thing for all auth extensions that happen to talk to external services.
jon-wei
left a comment
jon-wei
left a comment
There was a problem hiding this comment.
had 1 minor comment, LGTM otherwise
|
|
||
| /** | ||
| * This class exists only to enable use of custom SSLSocketFactory on top of builtin class. This could be removed | ||
| * when same functionality has been added to original class com.nimbusds.jose.util.CustomSSLResourceRetriever. |
There was a problem hiding this comment.
I think the original class in the comment should be com.nimbusds.jose.util.DefaultResourceRetriever.
There was a problem hiding this comment.
duh! :)
Initially CustomSSLResourceRetriever was named DefaultResourceRetriever ... findbugs did not like that I was overriding a class of same name, so I did a "Refactor -> Rename" in IDE which, inadvertently, changed that in comment too.
updated.
…to auth server (apache#9637) * druid-pac4j: add ability for custom ssl trust store for talking to auth server * fix nimbusds DefaultResourceRetriever name in comment
…to auth server (apache#9637) * druid-pac4j: add ability for custom ssl trust store for talking to auth server * fix nimbusds DefaultResourceRetriever name in comment
2 participants
Description
This feature is to enable users who use non-standard or self signed SSL certs on the auth server. Similar feature exists in other Druid features communicating with external LDAP server for example.
It was initially highlighted in #8992 (comment)
This PR has: