[WIP]kerberos authentication between client and broker #3658

jiazhai · 2019-02-22T05:06:46Z

Master Issue: #3491

Motivation

Currently both Zookeeper and BookKeeper could be secured by using Kerberos, It would be good to support Kerberos in Pulsar Broker and Client.
This is the first PR for issue in #3491 to support Kerberos in Pulsar Broker and Client.
Will add proxy and web resource support in following prs.
The Kerberos authentication is similar to that in Zookeeper and BookKeeper, which leverage SASL and GSSAPI, so reused some of the code there.

Changes.

changed connect command to support mutual auth between client and server;
provide both client and broker side support for authentication api;
add unit test.

jiazhai · 2019-02-22T05:41:41Z

@eolivelli Since you are familiar with BookKeeper Kerberos auth, Would you please help review this?
Thanks.

merlimat · 2019-02-22T05:42:00Z

conf/broker.conf

+### --- SASL Authentication Provider --- ###
+
+# Whether Use SASL Authentication or not.
+isSaslAuthentication=


This should be enabled through the authentication plugins list instead of an ad hoc flag

Thanks, @merlimat @sijie, right, this is a temp change, and will not be exist after the implementation of supporting for web resource.
As in the current code, this is only used in web resource checking for authentication.
Will add a TODO and link with the issue.

merlimat · 2019-02-22T05:45:12Z

...r-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSource.java

+
+    /**
+     *
+     * @return authentication data which will be stored in a command


This is changing the current authentication framework in non-trivial ways. This of course is needed to support Kerberos but It needs to be done in a separated PR. Also we should have a PIP to outline the design and have more people to get eyes on it.

merlimat · 2019-02-22T05:46:57Z

...r-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSource.java

+
+    /**
+     * set data which will be stored in a command to send to client
+     * used for sasl.


The interface should not be tied to a specific implementation (eg sasl), but rather abstract the generic needs of multi-step authentication.

sijie · 2019-02-22T05:21:10Z

conf/broker.conf

+### --- SASL Authentication Provider --- ###
+
+# Whether Use SASL Authentication or not.
+isSaslAuthentication=


do we really need another flag for checking whether it is sasl authentication?

Shall we just check if the authentication provider is an instance of sasl provider?

Thanks, @merlimat @sijie, right, this is a temp change, and will not be exist after the implementation of supporting for web resource.
As in the current code, this is only used in web resource checking for authentication.
Will add a TODO and link with the issue.

...h-sasl/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderSasl.java

sijie · 2019-02-22T05:30:14Z

...h-sasl/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderSasl.java

+        }
+    }
+
+    // will get authenticated client id like: "client/[email protected]"


is the comment needed?

will remove it.

...broker-auth-sasl/src/main/java/org/apache/pulsar/broker/authentication/PulsarSaslServer.java

sijie · 2019-02-22T05:50:17Z

...r-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSource.java

+     *
+     * @return authentication data which will be stored in a command
+     */
+    default byte[] getCommandDataBytes() {


shall we just standardize to use byte[]?

We should do this. since String convert meet some error in this pr

In other auth plugins we're base64 encoding the credential. That would not need any change in the plugin interfaces

Thanks. It is Ok for me. If byte[] will involve too many changes, will try and use base64 later.

@merlimat, How about we keep the old methods to use String, and new methods use byte[]?

one concern here is: Is it worth to convert between String and Byte using base64?
As in PulsarApi.proto, we store auth data as bytes in CommandConnected, and in SaslServer and SaslClient, it do the valuate and challenge also use bytes, like this:

byte[] evaluateResponse(byte[] response)

This means, we need to convert
bytes(proto cmd) <-> String(pulsar api) <-> bytes(sasl api)
each time.
The benefit to use String is the alignment with old api, while the opposite is useless converting.

...r-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSource.java

sijie · 2019-02-22T05:54:51Z

...r-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSource.java

+     * After the authentication between client and broker completed.
+     * Get authenticationId represent for the client
+     */
+    default String getAuthorizationID() {


is this only required by SASL?

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java

sijie · 2019-02-22T06:02:45Z

...r-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSource.java

     * @return authentication data which is stored in a command
     */
    default String getCommandData() {
        return null;
    }

+
+    /*


I think the newly introduced methods can just be part of SaslAuthenticationDataSource. Since they are only used by SaslAuthenticationProvider and SaslAuthenticationProvider will check if a data source is a sasl data source, it can cast the data source and access those methods.

We don't need to put the methods here.

sijie · 2019-02-22T06:26:35Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java

-                if (sslHandler != null) {
-                    sslSession = ((SslHandler) sslHandler).engine().getSession();
+                // sasl.
+                if (isSaslAuthenticationMethod()) {


I think this part of code can be refactored by introducing an AuthState interface. so we can have one stage AuthState for existing authentication providers, and a multi-stage authentication implementation for Kerberos (or any future implementation).

An AuthState basically is holding the authentication state, tell broker whether the authentication is completed or not, if completed, what is the AuthRole.

interface AuthState { boolean isComplete(); AuthenticationData getAuthData(); String getAuthRole(); /** * Returns null if authentication has completed, and no auth data is required to send back to client. * Returns the auth data back to client, if authentication has not completed. */ byte[] authenticate(byte[] authData); }

Then add a newAuthState in the AuthenticationProvider. The default implementation can be the OneStageAuthState (described at the end) to be shared across existing authentication providers. The kerberos plugin can return its own AuthState.

So the implementation in broker can be very simple:

AuthState authState = authenticationProvider.newAuthState(); byte[] clientAuthData = connect.getAuthData().toByteArray(); byte[] brokerAuthData = authState.authenticate(clientAuthData); if (null == brokerAuthData) { // authentication has completed. authData = authState.getAuthenticateData(); authRole = authState.getAuthRole(); // we are done here } else { // it is a multi-stage authentication mechanism, send the auth data back ctx.writeAndFlush(Commands.newConnecting(authMethod, data)); }

for the current existing authentication providers, implement a common OneStageAuthState (basically the logic from line 515 to line 534).

for the kerberos authenciation provider, implement a KeberosAuthState (basically the logic from line 469 to line 513).

This would produce a clean interface between AuthenticationProvider and Brokers and avoiding add Sasl specific logic in ServerCnx.

jiazhai · 2019-02-22T08:17:17Z

@sijie @merlimat Thanks for the comments. will prepare a pip for the mutual auth api changes, then back to this PR again.

merlimat · 2019-02-22T17:33:21Z

...broker-auth-sasl/src/main/java/org/apache/pulsar/broker/authentication/PulsarSaslServer.java

@@ -0,0 +1,198 @@
+/*
+ Licensed to Diennea S.r.l. under one


This will fail the License check. Either we add an exclusion or we ask the copyirght holder to donate this code to ASF.

@merlimat, @eolivelli right, it is copy from bookkeeper before this change. and It is there in BookKeeper.

I will send a patch for Bookkeeper

eolivelli · 2019-02-22T19:17:21Z

Was it on Bookkeeper? Its my company

eolivelli

What about using PulsarBroker and PulsarClient for jaas entries?

eolivelli · 2019-02-22T06:20:31Z

...broker-auth-sasl/src/main/java/org/apache/pulsar/broker/authentication/PulsarSaslServer.java

+                    serviceHostname = null;
+                }
+
+                log.info("serviceHostname is '{}', servicePrincipalName is '{}', SASL mechanism(mech) is '{}'.",


This will be logged at every new connection

will change it into debug

eolivelli · 2019-02-22T06:22:51Z

...er-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/SaslAuthenticateTest.java

+
+    @BeforeClass
+    public static void startMiniKdc() throws Exception {
+        kdcDir = Files.createTempDirectory("test-kdc-dir").toFile();


Aren't we using TemporaryFolder rule ?

Thanks. TemporaryFolder is from junit, but pulsar mainly use testng. will try to handle the delete of these folder in cleanup.

…/authentication/PulsarSaslServer.java Co-Authored-By: jiazhai <[email protected]>

jiazhai · 2019-03-14T07:25:45Z

Since changed a lot in PR #3677 , would like to close this pr, and will send a new one. Thanks for the comments.

Fixes #3652 **Motivation** Currently both Zookeeper and BookKeeper could be secured by using Kerberos, It would be good to support Kerberos in Pulsar Broker and Client. This is the sub-task for issue in #3491 to support Kerberos in Pulsar Broker and Client. Will add proxy and web resource support in following prs. The Kerberos authentication is similar to that in Zookeeper and BookKeeper, which leverage SASL and GSSAPI, so reused some of the code there. PR #3658 is the first version of PR before #3677 . **Changes** provide both client and broker side support for authentication api; add unit test.

jiazhai self-assigned this Feb 22, 2019

sijie added type/feature The PR added a new feature or issue requested a new feature area/security labels Feb 22, 2019

sijie added this to the 2.4.0 milestone Feb 22, 2019

jiazhai requested review from merlimat and sijie and removed request for merlimat February 22, 2019 05:40

merlimat reviewed Feb 22, 2019

View reviewed changes

sijie reviewed Feb 22, 2019

View reviewed changes

jiazhai force-pushed the issue_3652 branch from 0ce6c6d to 4606ef3 Compare February 22, 2019 08:20

jiazhai added 3 commits February 22, 2019 16:38

kerberos client <-> broker

d941189

fix header check

fb13549

change following comments

71439d6

merlimat reviewed Feb 22, 2019

View reviewed changes

eolivelli requested changes Feb 22, 2019

View reviewed changes

sijie and others added 2 commits February 23, 2019 11:53

Update pulsar-broker-auth-sasl/src/main/java/org/apache/pulsar/broker…

848caf6

…/authentication/PulsarSaslServer.java Co-Authored-By: jiazhai <[email protected]>

change following comments

0c1a120

jiazhai force-pushed the issue_3652 branch from 9bd5f64 to 0c1a120 Compare February 23, 2019 05:15

add AuthenticationState in broker side

ae888bf

jiazhai changed the title ~~kerberos authentication between client and broker~~ [WIP]kerberos authentication between client and broker Feb 23, 2019

jiazhai closed this Mar 14, 2019

jiazhai mentioned this pull request Mar 14, 2019

kerberos: authentication between client and broker #3821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]kerberos authentication between client and broker #3658

[WIP]kerberos authentication between client and broker #3658

jiazhai commented Feb 22, 2019 •

edited

Loading

jiazhai commented Feb 22, 2019

merlimat Feb 22, 2019

jiazhai Feb 22, 2019

merlimat Feb 22, 2019

merlimat Feb 22, 2019

sijie Feb 22, 2019

jiazhai Feb 22, 2019

sijie Feb 22, 2019

jiazhai Feb 22, 2019

sijie Feb 22, 2019

jiazhai Feb 22, 2019

merlimat Feb 22, 2019

jiazhai Feb 23, 2019

jiazhai Feb 23, 2019 •

edited

Loading

sijie Feb 22, 2019

sijie Feb 22, 2019

sijie Feb 22, 2019

jiazhai commented Feb 22, 2019

merlimat Feb 22, 2019

jiazhai Feb 23, 2019

eolivelli Feb 23, 2019

eolivelli commented Feb 22, 2019 •

edited

Loading

eolivelli left a comment

eolivelli Feb 22, 2019

jiazhai Feb 23, 2019

eolivelli Feb 22, 2019

jiazhai Feb 23, 2019

jiazhai commented Mar 14, 2019

[WIP]kerberos authentication between client and broker #3658

[WIP]kerberos authentication between client and broker #3658

Conversation

jiazhai commented Feb 22, 2019 • edited Loading

Motivation

Changes.

jiazhai commented Feb 22, 2019

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

jiazhai Feb 23, 2019 • 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

jiazhai commented Feb 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eolivelli commented Feb 22, 2019 • edited Loading

eolivelli left a 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

jiazhai commented Mar 14, 2019

jiazhai commented Feb 22, 2019 •

edited

Loading

jiazhai Feb 23, 2019 •

edited

Loading

eolivelli commented Feb 22, 2019 •

edited

Loading