You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by mm...@apache.org on 2022/05/10 17:04:01 UTC

[pulsar] branch master updated: [Authenticate] fix Invalid signature error when use Kerberos Authentication (#15121)

This is an automated email from the ASF dual-hosted git repository.

mmarshall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/master by this push:
     new f0b7efa3a16 [Authenticate]  fix Invalid signature error when use Kerberos Authentication (#15121)
f0b7efa3a16 is described below

commit f0b7efa3a16b31b19baa2613ad2e43b3c439bb60
Author: Dezhi LIiu <33...@users.noreply.github.com>
AuthorDate: Wed May 11 01:03:52 2022 +0800

    [Authenticate]  fix Invalid signature error when use Kerberos Authentication (#15121)
    
    * fix SaslRoleTokenSigner Invalid signature when use Kerberos Authentication
    
    * fix checkstyle violation
    
    * add saslJaasServerRoleTokenSignerSecret configuration fields in broker.conf
    
    * add secret in proxy.conf
    
    * supply the secret as a file
    
    * set default value
    
    * fix test error
    
    * fix test error
    
    * saslJaasServerRoleTokenSignerSecretPath must be set
    
    * add secret configuration into the WorkerConfig and the conf/functions_worker.yml
    
    * fix checkstyle error
    
    * chang URL.createURL to  URI.create
    
    Co-authored-by: liudezhi <li...@163.com>
    
    ### Motivation
    When use Kerberos authentication ,  using Pulsar Admin to query topic state, will appear HTTP 401 Unauthorized, becasue
    request redirect url  will use current SaslRoleToken ,but  redirect broker not recognized  the token, per broker secret is not same.
    ```
    WARN  org.apache.pulsar.broker.web.AuthenticationFilter - [10.3.0.4] Failed to authenticate HTTP request: Invalid signature
    ```
    per broker secret is  Random
    ```
    protected String computeSignature(String str) {
            try {
                MessageDigest md = MessageDigest.getInstance("SHA-512");
    
                md.update(str.getBytes());
    
                md.update(secret);
                byte[] digest = md.digest();
                return new Base64(0).encodeToString(digest);
            } catch (NoSuchAlgorithmException ex) {
                throw new RuntimeException("It should not happen, " + ex.getMessage(), ex);
            }
        }
    ```
    
    ### Modifications
    secret can configuration
    `this.signer = new SaslRoleTokenSigner(config.getSaslJaasServerRoleTokenSignerSecret().getBytes());`
    
    
    ### Does this pull request potentially affect one of the following parts:
    
    *If `yes` was chosen, please highlight the changes*
    
      - Dependencies (does it add or upgrade a dependency): (yes / no)
      - The public API: (no)
      - The schema: (no )
      - The default values of configurations: (yes)
      - The wire protocol: (no)
      - The rest endpoints: (no)
      - The admin cli options: ( no)
      - Anything that affects deployment: (no)
    
    ### Documentation
    
    Need to update docs?
    
    - [x]  `doc`
---
 conf/broker.conf                                   | 11 +++++---
 conf/functions_worker.yml                          | 16 +++++++++++
 conf/proxy.conf                                    | 16 +++++++++++
 .../authentication/AuthenticationProviderSasl.java | 31 +++++++++++++++++++---
 .../ProxySaslAuthenticationTest.java               | 14 +++++++++-
 .../authentication/SaslAuthenticateTest.java       |  8 ++++++
 .../apache/pulsar/broker/ServiceConfiguration.java |  8 ++++++
 .../apache/pulsar/common/sasl/SaslConstants.java   |  1 +
 .../pulsar/functions/worker/WorkerConfig.java      | 29 ++++++++++++++++++++
 .../pulsar/proxy/server/ProxyConfiguration.java    |  8 ++++++
 10 files changed, 134 insertions(+), 8 deletions(-)

diff --git a/conf/broker.conf b/conf/broker.conf
index e49ec50e215..2eae6461730 100644
--- a/conf/broker.conf
+++ b/conf/broker.conf
@@ -763,11 +763,16 @@ tokenAudience=
 # This is a regexp, which limits the range of possible ids which can connect to the Broker using SASL.
 # Default value: `SaslConstants.JAAS_CLIENT_ALLOWED_IDS_DEFAULT`, which is ".*pulsar.*",
 # so only clients whose id contains 'pulsar' are allowed to connect.
-saslJaasClientAllowedIds=
+saslJaasClientAllowedIds=.*pulsar.*
 
 # Service Principal, for login context name.
-# Default value `SaslConstants.JAAS_DEFAULT_BROKER_SECTION_NAME`, which is "Broker".
-saslJaasServerSectionName=
+# Default value `SaslConstants.JAAS_DEFAULT_BROKER_SECTION_NAME`, which is "PulsarBroker".
+saslJaasServerSectionName=PulsarBroker
+
+# Path to file containing the secret to be used to SaslRoleTokenSigner
+# The Path can be specified like:
+# saslJaasServerRoleTokenSignerSecretPath=file:///my/saslRoleTokenSignerSecret.key
+saslJaasServerRoleTokenSignerSecretPath=
 
 ### --- HTTP Server config --- ###
 
diff --git a/conf/functions_worker.yml b/conf/functions_worker.yml
index 4a018ceb6c2..4e812519cc6 100644
--- a/conf/functions_worker.yml
+++ b/conf/functions_worker.yml
@@ -361,6 +361,22 @@ webServiceTlsProtocols:
 # Examples:- [TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256]
 webServiceTlsCiphers:
 
+### --- SASL Authentication Provider --- ###
+
+# This is a regexp, which limits the range of possible ids which can connect to the Broker using SASL.
+# Default value: `SaslConstants.JAAS_CLIENT_ALLOWED_IDS_DEFAULT`, which is ".*pulsar.*",
+# so only clients whose id contains 'pulsar' are allowed to connect.
+saslJaasClientAllowedIds: .*pulsar.*
+
+# Service Principal, for login context name.
+# Default value `SaslConstants.JAAS_DEFAULT_BROKER_SECTION_NAME`, which is "PulsarFunction".
+saslJaasServerSectionName: PulsarFunction
+
+# Path to file containing the secret to be used to SaslRoleTokenSigner
+# The Path can be specified like:
+# saslJaasServerRoleTokenSignerSecretPath=file:///my/saslRoleTokenSignerSecret.key
+saslJaasServerRoleTokenSignerSecretPath:
+
 ########################
 # State Management
 ########################
diff --git a/conf/proxy.conf b/conf/proxy.conf
index 653a8a9ca61..838f3a70ba0 100644
--- a/conf/proxy.conf
+++ b/conf/proxy.conf
@@ -288,6 +288,22 @@ tokenAudienceClaim=
 # The token audience stands for this broker. The field `tokenAudienceClaim` of a valid token, need contains this.
 tokenAudience=
 
+### --- SASL Authentication Provider --- ###
+
+# This is a regexp, which limits the range of possible ids which can connect to the Broker using SASL.
+# Default value: `SaslConstants.JAAS_CLIENT_ALLOWED_IDS_DEFAULT`, which is ".*pulsar.*",
+# so only clients whose id contains 'pulsar' are allowed to connect.
+saslJaasClientAllowedIds=.*pulsar.*
+
+# Service Principal, for login context name.
+# Default value `SaslConstants.JAAS_DEFAULT_PROXY_SECTION_NAME`, which is "PulsarProxy".
+saslJaasServerSectionName=PulsarProxy
+
+# Path to file containing the secret to be used to SaslRoleTokenSigner
+# The Path can be specified like:
+# saslJaasServerRoleTokenSignerSecretPath=file:///my/saslRoleTokenSignerSecret.key
+saslJaasServerRoleTokenSignerSecretPath=
+
 ### --- WebSocket config variables --- ###
 
 # Enable or disable the WebSocket servlet.
diff --git a/pulsar-broker-auth-sasl/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderSasl.java b/pulsar-broker-auth-sasl/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderSasl.java
index 5448315b505..0e090c638c1 100644
--- a/pulsar-broker-auth-sasl/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderSasl.java
+++ b/pulsar-broker-auth-sasl/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderSasl.java
@@ -35,10 +35,12 @@ import static org.apache.pulsar.common.sasl.SaslConstants.SASL_STATE_SERVER;
 import static org.apache.pulsar.common.sasl.SaslConstants.SASL_STATE_SERVER_CHECK_TOKEN;
 import java.io.IOException;
 import java.net.SocketAddress;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.Base64;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Random;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
@@ -48,6 +50,7 @@ import javax.security.auth.login.LoginException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.pulsar.broker.ServiceConfiguration;
 import org.apache.pulsar.common.api.AuthData;
 import org.apache.pulsar.common.sasl.JAASCredentialsContainer;
@@ -81,7 +84,7 @@ public class AuthenticationProviderSasl implements AuthenticationProvider {
         try {
             this.allowedIdsPattern = Pattern.compile(allowedIdsPatternRegExp);
         } catch (PatternSyntaxException error) {
-            log.error("Invalid regular expression for id " + allowedIdsPatternRegExp, error);
+            log.error("Invalid regular expression for id {}", allowedIdsPatternRegExp, error);
             throw new IOException(error);
         }
 
@@ -98,8 +101,15 @@ public class AuthenticationProviderSasl implements AuthenticationProvider {
                 throw new IOException(e);
             }
         }
-
-        this.signer = new SaslRoleTokenSigner(Long.toString(new Random().nextLong()).getBytes());
+        String saslJaasServerRoleTokenSignerSecretPath = config.getSaslJaasServerRoleTokenSignerSecretPath();
+        byte[] secret = null;
+        if (StringUtils.isNotBlank(saslJaasServerRoleTokenSignerSecretPath)) {
+            secret = readSecretFromUrl(saslJaasServerRoleTokenSignerSecretPath);
+        } else {
+            String msg = "saslJaasServerRoleTokenSignerSecretPath parameter is empty";
+            throw new IllegalArgumentException(msg);
+        }
+        this.signer = new SaslRoleTokenSigner(secret);
     }
 
     @Override
@@ -175,6 +185,19 @@ public class AuthenticationProviderSasl implements AuthenticationProvider {
         return signed;
     }
 
+    private byte[] readSecretFromUrl(String secretConfUrl) throws IOException {
+        if (secretConfUrl.startsWith("file:")) {
+            URI filePath = URI.create(secretConfUrl);
+            return Files.readAllBytes(Paths.get(filePath));
+        } else if (Files.exists(Paths.get(secretConfUrl))) {
+            // Assume the key content was passed in a valid file path
+            return Files.readAllBytes(Paths.get(secretConfUrl));
+        } else {
+            String msg = "Role token signer secret file " + secretConfUrl + " doesn't exist";
+            throw new IllegalArgumentException(msg);
+        }
+    }
+
     private ConcurrentHashMap<Long, AuthenticationState> authStates = new ConcurrentHashMap<>();
 
     // return authState if it is in cache.
diff --git a/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/ProxySaslAuthenticationTest.java b/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/ProxySaslAuthenticationTest.java
index d9d0544d4d0..66737c80de8 100644
--- a/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/ProxySaslAuthenticationTest.java
+++ b/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/ProxySaslAuthenticationTest.java
@@ -24,6 +24,7 @@ import java.io.File;
 import java.io.FileWriter;
 import java.net.URI;
 import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
@@ -64,6 +65,8 @@ public class ProxySaslAuthenticationTest extends ProducerConsumerBase {
 
 	public static File kdcDir;
 	public static File kerberosWorkDir;
+	public static File brokerSecretKeyFile;
+	public static File proxySecretKeyFile;
 
 	private static MiniKdc kdc;
 	private static Properties properties;
@@ -182,6 +185,9 @@ public class ProxySaslAuthenticationTest extends ProducerConsumerBase {
 		conf.setAuthenticationEnabled(true);
 		conf.setSaslJaasClientAllowedIds(".*" + localHostname + ".*");
 		conf.setSaslJaasServerSectionName("PulsarBroker");
+		brokerSecretKeyFile = File.createTempFile("saslRoleTokenSignerSecret", ".key");
+		Files.write(Paths.get(brokerSecretKeyFile.toString()), "PulsarSecret".getBytes());
+		conf.setSaslJaasServerRoleTokenSignerSecretPath(brokerSecretKeyFile.toString());
 		Set<String> providers = new HashSet<>();
 		providers.add(AuthenticationProviderSasl.class.getName());
 		conf.setAuthenticationProviders(providers);
@@ -208,6 +214,10 @@ public class ProxySaslAuthenticationTest extends ProducerConsumerBase {
 	@Override
 	@AfterMethod(alwaysRun = true)
 	protected void cleanup() throws Exception {
+		FileUtils.deleteQuietly(brokerSecretKeyFile);
+		Assert.assertFalse(brokerSecretKeyFile.exists());
+		FileUtils.deleteQuietly(proxySecretKeyFile);
+		Assert.assertFalse(proxySecretKeyFile.exists());
 		super.internalCleanup();
 	}
 
@@ -234,7 +244,9 @@ public class ProxySaslAuthenticationTest extends ProducerConsumerBase {
 		proxyConfig.setBrokerClientAuthenticationParameters(
 			"{\"saslJaasClientSectionName\": " + "\"PulsarProxy\"," +
 				"\"serverType\": " + "\"broker\"}");
-
+		proxySecretKeyFile = File.createTempFile("saslRoleTokenSignerSecret", ".key");
+		Files.write(Paths.get(proxySecretKeyFile.toString()), "PulsarSecret".getBytes());
+		proxyConfig.setSaslJaasServerRoleTokenSignerSecretPath(proxySecretKeyFile.toString());
 		// proxy as a server, it will use sasl to authn
 		Set<String> providers = new HashSet<>();
 		providers.add(AuthenticationProviderSasl.class.getName());
diff --git a/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/SaslAuthenticateTest.java b/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/SaslAuthenticateTest.java
index 679ff2b9937..f7fc4abaf80 100644
--- a/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/SaslAuthenticateTest.java
+++ b/pulsar-broker-auth-sasl/src/test/java/org/apache/pulsar/broker/authentication/SaslAuthenticateTest.java
@@ -28,6 +28,7 @@ import java.io.File;
 import java.io.FileWriter;
 import java.net.URI;
 import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Properties;
@@ -53,6 +54,7 @@ import org.apache.pulsar.client.api.PulsarClient;
 import org.apache.pulsar.client.impl.auth.AuthenticationSasl;
 import org.apache.pulsar.common.api.AuthData;
 import org.apache.pulsar.common.sasl.SaslConstants;
+import org.testng.Assert;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeClass;
@@ -63,6 +65,7 @@ import org.testng.annotations.Test;
 public class SaslAuthenticateTest extends ProducerConsumerBase {
     public static File kdcDir;
     public static File kerberosWorkDir;
+    public static File secretKeyFile;
 
     private static MiniKdc kdc;
     private static Properties properties;
@@ -170,6 +173,9 @@ public class SaslAuthenticateTest extends ProducerConsumerBase {
         conf.setAuthenticationEnabled(true);
         conf.setSaslJaasClientAllowedIds(".*" + "client" + ".*");
         conf.setSaslJaasServerSectionName("PulsarBroker");
+        secretKeyFile = File.createTempFile("saslRoleTokenSignerSecret", ".key");
+        Files.write(Paths.get(secretKeyFile.toString()), "PulsarSecret".getBytes());
+        conf.setSaslJaasServerRoleTokenSignerSecretPath(secretKeyFile.toString());
         Set<String> providers = new HashSet<>();
         providers.add(AuthenticationProviderSasl.class.getName());
         conf.setAuthenticationProviders(providers);
@@ -202,6 +208,8 @@ public class SaslAuthenticateTest extends ProducerConsumerBase {
     @AfterMethod(alwaysRun = true)
     @Override
     protected void cleanup() throws Exception {
+        FileUtils.deleteQuietly(secretKeyFile);
+        Assert.assertFalse(secretKeyFile.exists());
         super.internalCleanup();
     }
 
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
index f877bfa086e..8b037c677d7 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
@@ -1462,6 +1462,14 @@ public class ServiceConfiguration implements PulsarConfiguration {
     )
     private String saslJaasServerSectionName = SaslConstants.JAAS_DEFAULT_BROKER_SECTION_NAME;
 
+    @FieldContext(
+            category = CATEGORY_SASL_AUTH,
+            doc = "Path to file containing the secret to be used to SaslRoleTokenSigner\n"
+                    + "The secret can be specified like:\n"
+                    + "saslJaasServerRoleTokenSignerSecretPath=file:///my/saslRoleTokenSignerSecret.key."
+    )
+    private String saslJaasServerRoleTokenSignerSecretPath;
+
     @FieldContext(
         category = CATEGORY_SASL_AUTH,
         doc = "kerberos kinit command."
diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/sasl/SaslConstants.java b/pulsar-common/src/main/java/org/apache/pulsar/common/sasl/SaslConstants.java
index 14e3b351ad5..2ed857c5954 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/sasl/SaslConstants.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/sasl/SaslConstants.java
@@ -33,6 +33,7 @@ public class SaslConstants {
     public static final String JAAS_SERVER_SECTION_NAME = "saslJaasServerSectionName";
     public static final String JAAS_DEFAULT_BROKER_SECTION_NAME = "PulsarBroker";
     public static final String JAAS_DEFAULT_PROXY_SECTION_NAME = "PulsarProxy";
+    public static final String JAAS_DEFAULT_FUNCTION_SECTION_NAME = "PulsarFunction";
 
     // Client principal
     public static final String JAAS_CLIENT_SECTION_NAME = "saslJaasClientSectionName";
diff --git a/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java b/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java
index dd694b6b36d..01683369d3e 100644
--- a/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java
+++ b/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java
@@ -49,6 +49,7 @@ import org.apache.pulsar.common.configuration.FieldContext;
 import org.apache.pulsar.common.configuration.PulsarConfiguration;
 import org.apache.pulsar.common.functions.Resources;
 import org.apache.pulsar.common.nar.NarClassLoader;
+import org.apache.pulsar.common.sasl.SaslConstants;
 import org.apache.pulsar.functions.auth.KubernetesSecretsTokenAuthProvider;
 import org.apache.pulsar.functions.instance.state.BKStateStoreProviderImpl;
 import org.apache.pulsar.functions.runtime.kubernetes.KubernetesRuntimeFactory;
@@ -548,6 +549,34 @@ public class WorkerConfig implements Serializable, PulsarConfiguration {
     )
     private Set<String> superUserRoles = Sets.newTreeSet();
 
+    @FieldContext(
+            category = CATEGORY_WORKER_SECURITY,
+            doc = "This is a regexp, which limits the range of possible ids which can connect to the Broker using SASL."
+                    + "\n Default value is: \".*pulsar.*\", so only clients whose id contains 'pulsar' are allowed to"
+                    + " connect."
+    )
+    private String saslJaasClientAllowedIds = SaslConstants.JAAS_CLIENT_ALLOWED_IDS_DEFAULT;
+
+    @FieldContext(
+            category = CATEGORY_WORKER_SECURITY,
+            doc = "Service Principal, for login context name. Default value is \"PulsarFunction\"."
+    )
+    private String saslJaasServerSectionName = SaslConstants.JAAS_DEFAULT_FUNCTION_SECTION_NAME;
+
+    @FieldContext(
+            category = CATEGORY_WORKER_SECURITY,
+            doc = "Path to file containing the secret to be used to SaslRoleTokenSigner\n"
+                    + "The secret can be specified like:\n"
+                    + "saslJaasServerRoleTokenSignerSecretPath=file:///my/saslRoleTokenSignerSecret.key."
+    )
+    private String saslJaasServerRoleTokenSignerSecretPath;
+
+    @FieldContext(
+            category = CATEGORY_WORKER_SECURITY,
+            doc = "kerberos kinit command."
+    )
+    private String kinitCommand = "/usr/bin/kinit";
+
     private Properties properties = new Properties();
 
     public boolean getTlsEnabled() {
diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java
index b6371c55f93..e1102e186f2 100644
--- a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java
+++ b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java
@@ -357,6 +357,14 @@ public class ProxyConfiguration implements PulsarConfiguration {
     )
     private String saslJaasServerSectionName = SaslConstants.JAAS_DEFAULT_PROXY_SECTION_NAME;
 
+    @FieldContext(
+            category = CATEGORY_SASL_AUTH,
+            doc = "Path to file containing the secret to be used to SaslRoleTokenSigner\n"
+                    + "The secret can be specified like:\n"
+                    + "saslJaasServerRoleTokenSignerSecretPath=file:///my/saslRoleTokenSignerSecret.key."
+    )
+    private String saslJaasServerRoleTokenSignerSecretPath;
+
     @FieldContext(
         category = CATEGORY_SASL_AUTH,
         doc = "kerberos kinit command."