Druid Extension to enable Authentication using Kerberos. #3853

nishantmonu51 · 2017-01-17T12:29:30Z

Druid Extension to enable Authentication for Druid Nodes using Kerberos.
It uses the simple and protected GSSAPI negotiation mechanism, SPNEGO(https://en.wikipedia.org/wiki/SPNEGO) for authentication via HTTP.
For internal node communication it also adds a wrapper around existing HTTP client to add GSSAPI authentication headers.

b-slim · 2017-01-20T23:36:16Z

distribution/pom.xml

@@ -105,6 +105,8 @@
                                <argument>-c</argument>
                                <argument>io.druid.extensions:postgresql-metadata-storage</argument>
                                <argument>${druid.distribution.pulldeps.opts}</argument>
+				                        <argument>-c</argument>


b-slim · 2017-01-20T23:39:55Z

docs/content/development/extensions-core/druid-kerberos.md

+|`druid.hadoop.security.spnego.authToLocal`||It allows you to set a general rule for mapping principal names to local user names. It will be used if there is not an explicit mapping for the principal name that is being translated.|DEFAULT|
+|`druid.hadoop.security.spnego.excludedPaths`|`['/status','/health']`| Array of HTTP paths which which does NOT need to be authenticated.|\["/status"]|
+
+As a note, it is required that the SPNego principal in use by the druid nodes must have a primary of HTTP (where Kerberos principals are of the form primary\[/instance]@REALM).


not sure i am getting what does this mean ?

b-slim · 2017-01-20T23:41:55Z

extensions-core/druid-kerberos/pom.xml

+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>


duplicate line 54

- This PR adds an extension for supporting druid authentication via Kerberos. - Working on the docs.

nishantmonu51 · 2017-01-23T16:27:22Z

@b-slim Thanks for the review. handled review comments and resolved conflicts.
@himanshug can you also take a look at it ?

b-slim · 2017-01-23T17:33:11Z

docs/content/development/extensions-core/druid-kerberos.md

+# Druid-Kerberos
+
+Druid Extension to enable Authentication for Druid Nodes using Kerberos.
+This extension adds AuthenticationFilter which is used to proect HTTP Endpoints using the simple and protected GSSAPI negotiation mechanism [SPNEGO](https://en.wikipedia.org/wiki/SPNEGO). 


proect -> protect

b-slim · 2017-01-23T18:01:36Z

docs/content/development/extensions-core/druid-kerberos.md

+
+|Property|Possible Values|Description|Default|
+|--------|---------------|-----------|-------|
+|`druid.authentication.type`|kerberos||Must be set to 'kerberos' to enable kerberos authetication.|empty|


do we expect to have another type withing the same module ? not sure if this is needed i propose to turn it into a switch to turn it on/off druid.authentication.kerberos=true by default.

b-slim · 2017-01-23T18:02:24Z

docs/content/development/extensions-core/druid-kerberos.md

+|Property|Possible Values|Description|Default|
+|--------|---------------|-----------|-------|
+|`druid.authentication.type`|kerberos||Must be set to 'kerberos' to enable kerberos authetication.|empty|
+|`druid.hadoop.security.kerberos.principal`|`[email protected]`| Principal user name, used for internal node communication|empty|


can we add a column to clear out what is required and not ?

b-slim · 2017-01-23T18:03:53Z

docs/content/development/extensions-core/druid-kerberos.md

+|`druid.hadoop.security.kerberos.keytab`|`/etc/security/keytabs/druid.headlessUser.keytab`|Path to keytab file used for internal node communication|empty|
+|`druid.hadoop.security.spnego.principal`|`HTTP/[email protected]`| SPNego service principal used by druid nodes|empty|
+|`druid.hadoop.security.spnego.keytab`|`/etc/security/keytabs/spnego.service.keytab`|SPNego service keytab|empty|
+|`druid.hadoop.security.spnego.authToLocal`||It allows you to set a general rule for mapping principal names to local user names. It will be used if there is not an explicit mapping for the principal name that is being translated.|DEFAULT|


does this take a map of object ? can you give example please ?

b-slim · 2017-01-23T18:05:17Z

docs/content/development/extensions-core/druid-kerberos.md

+|`druid.hadoop.security.spnego.principal`|`HTTP/[email protected]`| SPNego service principal used by druid nodes|empty|
+|`druid.hadoop.security.spnego.keytab`|`/etc/security/keytabs/spnego.service.keytab`|SPNego service keytab|empty|
+|`druid.hadoop.security.spnego.authToLocal`||It allows you to set a general rule for mapping principal names to local user names. It will be used if there is not an explicit mapping for the principal name that is being translated.|DEFAULT|
+|`druid.hadoop.security.spnego.excludedPaths`|`['/status','/health']`| Array of HTTP paths which which does NOT need to be authenticated.|\["/status"]|


not sure about this, usually the security guys like to block every thing by default. but it is ok if you like to have it open

removed, by default all paths are authenticated now.

b-slim · 2017-01-23T18:07:12Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/DruidKerberosModule.java

+  private static final String PROPERTY_AUTH_TYPE = "druid.authentication.type";
+  private static final String KERBEROS = "kerberos";
+
+  @Inject


can we avoid injecting the properties ?

I could not find a trivial way to avoid it, FWIW, I referenced this from SqlModule, there also we inject props, so maybe its ok ?
If not can you suggest an alternative ?

b-slim · 2017-01-23T18:10:57Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/DruidKerberosModule.java

+      binder.bind(HttpClient.class)
+            .annotatedWith(Client.class)
+            .toProvider(new KerberosHttpClientProvider(new HttpClientModule.HttpClientProvider(Client.class)))
+            .in(LazySingleton.class);


did you tested on Router i guess it is missing annotation @Router https://github.com/druid-io/druid/blob/35160e55953a476ca17b8676bf984a68a2a8f192/server/src/main/java/io/druid/server/AsyncQueryForwardingServlet.java#L115-L115

made changes for Router, since it uses Jetty Http client instead of MMX, had to write another class for that,

b-slim · 2017-01-23T18:11:35Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/DruidKerberosModule.java

+  private boolean isKerberosSecurityEnabled()
+  {
+    Preconditions.checkNotNull(props, "props");
+    return props.getProperty(PROPERTY_AUTH_TYPE, "").equalsIgnoreCase(KERBEROS);


as i said above this is more like true/false IMO.

changed to boolean flag

b-slim · 2017-01-23T18:14:35Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/DruidKerberosModule.java

+ * under the License.
+ */
+
+package io.druid.kerberos;


nit i would call it io.druid.security.kerberos

b-slim · 2017-01-23T18:29:00Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/KerberosHttpClient.java

+        @Override
+        public String run() throws Exception
+        {
+          return kerberosChallenge(host);


not sure how this actually works but the question is it blocking ? can we cache the challenge key to avoid RPC comms for every call ?

if you have multiple calls will this generate multiple tokens ? shouldn't make sure that we have unique token in case of concurrent requests ?

@nishantmonu51 please this is an important one.

this should not be blocking as user is already logged in using this realm.
Also checked how other projects implement this e.g -
https://github.com/prongs/apache-hive/blob/master/jdbc/src/java/org/apache/hive/jdbc/HttpKerberosRequestInterceptor.java
which has similar logic underneath.
as far as multi threading goes, Since the credentials are read from the Subject which is initiating the context. it should be safe to call this concurrently from multiple threads.

b-slim · 2017-01-23T19:17:56Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/SpnegoFilterHolder.java

+  @Override
+  public Map<String, String> getInitParameters()
+  {
+    System.out.println(config);


not needed probably.

b-slim · 2017-01-23T19:52:09Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/KerberosHttpClient.java

+  public static String kerberosChallenge(String server) throws GSSException
+  {
+    // This Oid for Kerberos GSS-API mechanism.
+    Oid mechOid = new Oid("1.2.840.113554.1.2.2");


looking at Hadoop project looks like this is JVM dependent https://github.com/apache/hadoop/blob/916140604ffef59466ba30832478311d3e6249bd/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosUtil.java#L54-L54

@nishantmonu51 this still the same.

b-slim · 2017-01-23T19:53:46Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/KerberosHttpClient.java

+          return kerberosChallenge(host);
+        }
+      });
+      request.setHeader(HttpHeaders.Names.AUTHORIZATION, "Negotiate " + challenge);


looks like there is no Negotiation which means that there is no spenago in the picture at all ? i am i missing something ?

himanshug · 2017-01-23T21:03:48Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/DruidKerberosModule.java

+      Multibinder.newSetBinder(binder, ServletFilterHolder.class)
+                 .addBinding()
+                 .to(SpnegoFilterHolder.class);
+      JsonConfigProvider.bind(binder, "druid.global.http", DruidHttpClientConfig.class, Global.class);


is this needed?

not needed, removed.

himanshug · 2017-01-23T21:03:55Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/DruidKerberosModule.java

+            .toProvider(new KerberosHttpClientProvider(new HttpClientModule.HttpClientProvider(Global.class)))
+            .in(LazySingleton.class);
+
+      JsonConfigProvider.bind(binder, "druid.broker.http", DruidHttpClientConfig.class, Client.class);


is this needed?

himanshug · 2017-01-23T21:05:56Z

looks good to me besides existing comments.

b-slim · 2017-01-23T21:50:57Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/SpnegoFilterHolder.java

+      {
+        ClassLoader prevLoader = Thread.currentThread().getContextClassLoader();
+        try {
+          Thread.currentThread().setContextClassLoader(AuthenticationFilter.class.getClassLoader());


do we need this ? can we add a comments why we need to switch the classloader ?

added comment

b-slim · 2017-01-23T21:59:13Z

extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/KerberosHttpClient.java

+      Configuration conf = new Configuration();
+      conf.set(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION, "kerberos");
+      UserGroupInformation.setConfiguration(conf);
+      if (UserGroupInformation.isSecurityEnabled()) {


this seems to me it is using hadoop conf files to check of kerberos is enabled. would this still work in case we are not using hadoop config with druid ?

So after digging into the hadoop code seems like you are setting conf.set(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION, "kerberos"); and then testing UserGroupInformation.isSecurityEnabled()) seems like this test will always return true. ??

removed the check.

@nishantmonu51t this seems to me it is using hadoop conf files to check of kerberos is enabled. would this still work in case we are not using hadoop config with druid ?

b-slim · 2017-01-24T15:59:07Z

...nsions-core/druid-kerberos/src/main/java/io/druid/security/kerberos/DruidKerberosModule.java

+  private static final String KERBEROS_ENABLED = "druid.authentication.kerberos.enabled";
+
+  @Inject
+  private Properties props;


If i am not wrong the only reason this is needed is to read the value of druid.authentication.kerberos.enabled.
IMO this is not need since if the user chose to add this extension to the list of loaded module it means they want it activated and if they don't all they have to do is to remove it from the list. So i guess it is fair to assume that there is no need for this flag and turn it on by adding it to the list.

The extensions can also be loaded from classpath, so if someone adds the jar in classpath without wanting to enable authentication, It will be useful for such cases.

b-slim · 2017-01-24T16:01:54Z

docs/content/development/extensions-core/druid-kerberos.md

+|`druid.hadoop.security.spnego.principal`|`HTTP/[email protected]`| SPNego service principal used by druid nodes|empty|Yes|
+|`druid.hadoop.security.spnego.keytab`|`/etc/security/keytabs/spnego.service.keytab`|SPNego service keytab|empty|Yes|
+|`druid.hadoop.security.spnego.authToLocal`||It allows you to set a general rule for mapping principal names to local user names. It will be used if there is not an explicit mapping for the principal name that is being translated.|DEFAULT|No|
+|`druid.hadoop.security.spnego.excludedPaths`|`['/status','/health']`| Array of HTTP paths which which does NOT need to be authenticated.|\["/status"]|No|


thought you said everything is blocked ?

b-slim · 2017-01-25T15:00:22Z

Have you seen the lock used ? https://github.com/prongs/apache-hive/blob/master/jdbc/src/java/org/apache/hive/jdbc/HttpKerberosRequestInterceptor.java#L61 -- B-Slim _______/\/\/\_______/\/\/\_______/\/\/\_______/\/\/\_______/\/\/\_______

…

On Jan 25, 2017, at 2:21 AM, Nishant Bangarwa ***@***.***> wrote: @nishantmonu51 commented on this pull request. In extensions-core/druid-kerberos/src/main/java/io/druid/kerberos/KerberosHttpClient.java <#3853>: > + + @OverRide + public <Intermediate, Final> ListenableFuture<Final> go( + Request request, HttpResponseHandler<Intermediate, Final> httpResponseHandler, Duration duration + ) + { + try { + final String host = request.getUrl().getHost(); + authenticateIfRequired(); + UserGroupInformation currentUser = UserGroupInformation.getCurrentUser(); + String challenge = currentUser.doAs(new PrivilegedExceptionAction<String>() + { + @OverRide + public String run() throws Exception + { + return kerberosChallenge(host); this should not be blocking as user is already logged in using this realm. Also checked how other projects implement this e.g - https://github.com/prongs/apache-hive/blob/master/jdbc/src/java/org/apache/hive/jdbc/HttpKerberosRequestInterceptor.java <https://github.com/prongs/apache-hive/blob/master/jdbc/src/java/org/apache/hive/jdbc/HttpKerberosRequestInterceptor.java> which has similar logic underneath. as far as multi threading goes, Since the credentials are read from the Subject which is initiating the context. it should be safe to call this concurrently from multiple threads. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3853>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHCODK5Jh5w1whPHnn-GRxbgbAAUBLz8ks5rVyITgaJpZM4Lllrf>.

nishantmonu51 · 2017-01-25T15:24:58Z

@b-slim: Nice catch, fixed the locking.

nishantmonu51 · 2017-01-30T14:53:51Z

@b-slim: Have added cookie handling and added more detailed docs to include curl commands and browser configs required to access coordinator and overlord console.

b-slim · 2017-02-01T16:15:54Z

docs/content/development/extensions-core/druid-kerberos.md

+3. Now you can access druid HTTP endpoints using curl command as follows - 
+
+    ```
+    curl --negotiate -u:anyUser -b ~/cookies.txt -c ~/cookies.txt -X POST -H'Content-Type: application/json' <HTTP_END_POINT>


can we mention that negotiate is needed only once ?

IMO, user should always specify negotiate. Fwiw, adding this will not mean that authentication handshake will be always done.
Above curl command works like this -

sends request to that to the server with the cookies for that domain if any.

If the server accepts the cookie it will return a 200 OK

If the cookie is found to be invalid (multiple reasons - someone restarted the server before the cookie expired, the cookie expired) It will send a response otherwise 401 Unauthorized.

The client on receiving 401 now will perform the SPNego negotiate mechanism.

Will add comment about the use of cookies.

b-slim · 2017-02-01T16:20:59Z

...nsions-core/druid-kerberos/src/main/java/io/druid/security/kerberos/DruidKerberosModule.java

+  @Override
+  public void configure(Binder binder)
+  {
+    JsonConfigProvider.bind(binder, "druid.hadoop.security.kerberos", DruidKerberosConfig.class);


nit: so fare we have HdfsKerberosConfig and HadoopKerberosConfig can we call this one AuthKerberosConfig or any other name that reflects the scope ?

b-slim · 2017-02-01T16:29:18Z

docs/content/development/extensions-core/druid-kerberos.md

+|`druid.hadoop.security.spnego.keytab`|`/etc/security/keytabs/spnego.service.keytab`|SPNego service keytab used by druid nodes|empty|Yes|
+|`druid.hadoop.security.spnego.authToLocal`|`RULE:[1:$1@$0]([email protected])s/.*/druid DEFAULT`|It allows you to set a general rule for mapping principal names to local user names. It will be used if there is not an explicit mapping for the principal name that is being translated.|DEFAULT|No|
+|`druid.hadoop.security.spnego.excludedPaths`|`['/status','/health']`| Array of HTTP paths which which does NOT need to be authenticated.|None|No|
+|`druid.hadoop.security.spnego.cookieSignatureSecret`|`secretString`| Secret used to sign authentication cookies|<Random value>|No|


wondering : if we can get away with a random String why we need this config ?

main reason is that the cookie specs does not guarantee isolation against port numbers.
e.g This is a sample cookie content from my host which does not have any port info-

druid-spnego-1.openstacklocal FALSE /druid/v2/ FALSE 0 hadoop.auth u=druid&[email protected]&t=kerberos&e=1486006717274&s=/pK2gDY6htUnS73Bo2DrwDSTh2Y=

so if you have multiple druid nodes running on same host using same paths and different signature (generated randomly instead of being explictly set ), user will keep doing token negotiation as cookie generated by one node will not be accepted by other node. setting the cookie signature ensures that this does not occur.
Have added more clarification in docs.

@nishantmonu51 This is a very important information to add to the docs thought looks like it is mandetory to have it random if you have multiple process in the same machine.

b-slim · 2017-02-01T18:57:05Z

extensions-core/druid-kerberos/src/main/java/io/druid/security/kerberos/SpnegoFilterHolder.java

+      params.put(AuthenticationFilter.AUTH_TYPE, "kerberos");
+      params.put("kerberos.name.rules", config.getAuthToLocal());
+      if (config.getCookieSignatureSecret() != null) {
+        params.put("signature.secret", config.getCookieSignatureSecret());


@nishantmonu51 not sure where the random string will be generated ?

got Offline answer from @nishantmonu51 https://github.com/hanborq/hadoop/blob/master/src/core/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java#L156

b-slim · 2017-02-01T19:27:35Z

👍

nishantmonu51 · 2017-02-01T19:33:48Z

@himanshug: Handled review comments from slim, Please check again.

nishantmonu51 assigned himanshug and b-slim Jan 17, 2017

nishantmonu51 added this to the 0.10.0 milestone Jan 17, 2017

b-slim reviewed Jan 20, 2017

View reviewed changes

nishantmonu51 added 3 commits January 23, 2017 21:55

Add extension for supporting kerberos security

dfbca62

- This PR adds an extension for supporting druid authentication via Kerberos. - Working on the docs.

Add docs

83464e7

review comments

c33e9c2

nishantmonu51 force-pushed the kerberos-security branch from d14997a to c33e9c2 Compare January 23, 2017 16:26

b-slim reviewed Jan 23, 2017

View reviewed changes

himanshug reviewed Jan 23, 2017

View reviewed changes

b-slim reviewed Jan 23, 2017

View reviewed changes

more review comments

fb93050

b-slim reviewed Jan 24, 2017

View reviewed changes

nishantmonu51 added 3 commits January 24, 2017 21:57

Block all paths by default

af20be5

more review comments - use proper Oid

d581b2f

Allow extensions to override httpclient for integration tests

af685bf

Add kerberos lock to prevent multithreaded issues.

063e6d7

nishantmonu51 added 2 commits January 25, 2017 22:52

review comment - remove enabled flag and fix router injection

83edb98

Add Cookie Handling and more detailed docs

76ae1b6

b-slim reviewed Feb 1, 2017

View reviewed changes

nishantmonu51 added 2 commits February 1, 2017 22:23

review comment - rename DruidKerberosConfig -> AuthKerberosConfig

3df3bce

review comments

56d7cb0

b-slim reviewed Feb 1, 2017

View reviewed changes

b-slim closed this Feb 1, 2017

b-slim reopened this Feb 1, 2017

fix travis failure on jdk7

7a79d5e

himanshug merged commit a457cde into apache:master Feb 2, 2017

gianm added the Release Notes label Feb 23, 2017

gianm mentioned this pull request Feb 28, 2017

Druid 0.10.0 release notes #3944

Closed

omkreddy mentioned this pull request Apr 12, 2017

Add Kerberos support to Tranquility druid-io/tranquility#219

Open

clambertus unassigned himanshug and b-slim Jul 6, 2018

Druid Extension to enable Authentication using Kerberos. #3853

Druid Extension to enable Authentication using Kerberos. #3853

Conversation

nishantmonu51 commented Jan 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nishantmonu51 commented Jan 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himanshug commented Jan 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-slim Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

b-slim Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-slim commented Jan 25, 2017 via email

nishantmonu51 commented Jan 25, 2017

nishantmonu51 commented Jan 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-slim commented Feb 1, 2017

nishantmonu51 commented Feb 1, 2017

b-slim Jan 23, 2017 •

edited

Loading

b-slim Jan 23, 2017 •

edited

Loading