pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] sijie commented on a change in pull request #3658: kerberos authentication between client and broker
Date Fri, 22 Feb 2019 06:26:49 GMT
sijie commented on a change in pull request #3658: kerberos authentication between client and
broker
URL: https://github.com/apache/pulsar/pull/3658#discussion_r259224620
 
 

 ##########
 File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
 ##########
 @@ -451,31 +458,80 @@ protected void handleConnect(CommandConnect connect) {
         checkArgument(state == State.Start);
         if (service.isAuthenticationEnabled()) {
             try {
-                String authMethod = "none";
                 if (connect.hasAuthMethodName()) {
                     authMethod = connect.getAuthMethodName();
                 } else if (connect.hasAuthMethod()) {
                     // Legacy client is passing enum
                     authMethod = connect.getAuthMethod().name().substring(10).toLowerCase();
                 }
 
-                String authData = connect.getAuthData().toStringUtf8();
-                ChannelHandler sslHandler = ctx.channel().pipeline().get(PulsarChannelInitializer.TLS_HANDLER);
-                SSLSession sslSession = null;
-                if (sslHandler != null) {
-                    sslSession = ((SslHandler) sslHandler).engine().getSession();
+                // sasl.
+                if (isSaslAuthenticationMethod()) {
 
 Review comment:
   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.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message