diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java index 3baaf57990a9a..05f146e895388 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java @@ -19,10 +19,10 @@ package org.apache.pulsar.broker.authorization; import static java.util.concurrent.TimeUnit.SECONDS; +import java.net.SocketAddress; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; -import javax.ws.rs.core.Response; import org.apache.commons.lang3.StringUtils; import org.apache.pulsar.broker.PulsarServerException; import org.apache.pulsar.broker.ServiceConfiguration; @@ -37,7 +37,6 @@ import org.apache.pulsar.common.policies.data.TenantInfo; import org.apache.pulsar.common.policies.data.TenantOperation; import org.apache.pulsar.common.policies.data.TopicOperation; -import org.apache.pulsar.common.util.FutureUtil; import org.apache.pulsar.common.util.RestException; import org.apache.pulsar.metadata.api.MetadataStoreException; import org.slf4j.Logger; @@ -293,19 +292,39 @@ public CompletableFuture allowSinkOpsAsync(NamespaceName namespaceName, return provider.allowSinkOpsAsync(namespaceName, role, authenticationData); } - private static void validateOriginalPrincipal(Set proxyRoles, String authenticatedPrincipal, - String originalPrincipal) { - if (proxyRoles.contains(authenticatedPrincipal)) { - // Request has come from a proxy + public boolean isValidOriginalPrincipal(String authenticatedPrincipal, + String originalPrincipal, + AuthenticationDataSource authDataSource) { + SocketAddress remoteAddress = authDataSource != null ? authDataSource.getPeerAddress() : null; + return isValidOriginalPrincipal(authenticatedPrincipal, originalPrincipal, remoteAddress); + } + + /** + * Validates that the authenticatedPrincipal and the originalPrincipal are a valid combination. + * Valid combinations fulfill the following rule: the authenticatedPrincipal is in + * {@link ServiceConfiguration#getProxyRoles()}, if, and only if, the originalPrincipal is set to a role + * that is not also in {@link ServiceConfiguration#getProxyRoles()}. + * @return true when roles are a valid combination and false when roles are an invalid combination + */ + public boolean isValidOriginalPrincipal(String authenticatedPrincipal, + String originalPrincipal, + SocketAddress remoteAddress) { + String errorMsg = null; + if (conf.getProxyRoles().contains(authenticatedPrincipal)) { if (StringUtils.isBlank(originalPrincipal)) { - log.warn("Original principal empty in request authenticated as {}", authenticatedPrincipal); - throw new RestException(Response.Status.UNAUTHORIZED, "Original principal cannot be empty if the " - + "request is via proxy."); - } - if (proxyRoles.contains(originalPrincipal)) { - log.warn("Original principal {} cannot be a proxy role ({})", originalPrincipal, proxyRoles); - throw new RestException(Response.Status.UNAUTHORIZED, "Original principal cannot be a proxy role"); + errorMsg = "originalPrincipal must be provided when connecting with a proxy role."; + } else if (conf.getProxyRoles().contains(originalPrincipal)) { + errorMsg = "originalPrincipal cannot be a proxy role."; } + } else if (StringUtils.isNotBlank(originalPrincipal)) { + errorMsg = "cannot specify originalPrincipal when connecting without valid proxy role."; + } + if (errorMsg != null) { + log.warn("[{}] Illegal combination of role [{}] and originalPrincipal [{}]: {}", remoteAddress, + authenticatedPrincipal, originalPrincipal, errorMsg); + return false; + } else { + return true; } } @@ -340,7 +359,9 @@ public CompletableFuture allowTenantOperationAsync(String tenantName, String originalRole, String role, AuthenticationDataSource authData) { - validateOriginalPrincipal(conf.getProxyRoles(), role, originalRole); + if (!isValidOriginalPrincipal(role, originalRole, authData)) { + return CompletableFuture.completedFuture(false); + } if (isProxyRole(role)) { CompletableFuture isRoleAuthorizedFuture = allowTenantOperationAsync( tenantName, operation, role, authData); @@ -396,7 +417,9 @@ public CompletableFuture allowNamespaceOperationAsync(NamespaceName nam String originalRole, String role, AuthenticationDataSource authData) { - validateOriginalPrincipal(conf.getProxyRoles(), role, originalRole); + if (!isValidOriginalPrincipal(role, originalRole, authData)) { + return CompletableFuture.completedFuture(false); + } if (isProxyRole(role)) { CompletableFuture isRoleAuthorizedFuture = allowNamespaceOperationAsync( namespaceName, operation, role, authData); @@ -438,7 +461,9 @@ public CompletableFuture allowNamespacePolicyOperationAsync(NamespaceNa String originalRole, String role, AuthenticationDataSource authData) { - validateOriginalPrincipal(conf.getProxyRoles(), role, originalRole); + if (!isValidOriginalPrincipal(role, originalRole, authData)) { + return CompletableFuture.completedFuture(false); + } if (isProxyRole(role)) { CompletableFuture isRoleAuthorizedFuture = allowNamespacePolicyOperationAsync( namespaceName, policy, operation, role, authData); @@ -495,10 +520,8 @@ public CompletableFuture allowTopicPolicyOperationAsync(TopicName topic String originalRole, String role, AuthenticationDataSource authData) { - try { - validateOriginalPrincipal(conf.getProxyRoles(), role, originalRole); - } catch (RestException e) { - return FutureUtil.failedFuture(e); + if (!isValidOriginalPrincipal(role, originalRole, authData)) { + return CompletableFuture.completedFuture(false); } if (isProxyRole(role)) { CompletableFuture isRoleAuthorizedFuture = allowTopicPolicyOperationAsync( @@ -582,7 +605,9 @@ public CompletableFuture allowTopicOperationAsync(TopicName topicName, String originalRole, String role, AuthenticationDataSource authData) { - validateOriginalPrincipal(conf.getProxyRoles(), role, originalRole); + if (!isValidOriginalPrincipal(role, originalRole, authData)) { + return CompletableFuture.completedFuture(false); + } if (isProxyRole(role)) { CompletableFuture isRoleAuthorizedFuture = allowTopicOperationAsync( topicName, operation, role, authData); diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java index 1ed27d4a757e1..bd55067b563a9 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java @@ -3991,7 +3991,7 @@ protected void internalOffloadStatus(AsyncResponse asyncResponse, boolean author }); } - public static CompletableFuture getPartitionedTopicMetadata( + public CompletableFuture getPartitionedTopicMetadata( PulsarService pulsar, String clientAppId, String originalPrincipal, AuthenticationDataSource authenticationData, TopicName topicName) { CompletableFuture metadataFuture = new CompletableFuture<>(); diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java index 899bdd496268d..1e7ccfb298a65 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java @@ -188,7 +188,6 @@ public class ServerCnx extends PulsarHandler implements TransportCnx { private int nonPersistentPendingMessages = 0; private final int maxNonPersistentPendingMessages; private String originalPrincipal = null; - private Set proxyRoles; private boolean authenticateOriginalAuthData; private final boolean schemaValidationEnforced; private String authMethod = "none"; @@ -261,7 +260,6 @@ public ServerCnx(PulsarService pulsar, String listenerName) { this.recentlyClosedProducers = new HashMap<>(); this.replicatorPrefix = conf.getReplicatorPrefix(); this.maxNonPersistentPendingMessages = conf.getMaxConcurrentNonPersistentMessagePerConnection(); - this.proxyRoles = conf.getProxyRoles(); this.authenticateOriginalAuthData = conf.isAuthenticateOriginalAuthData(); this.schemaValidationEnforced = conf.isSchemaValidationEnforced(); this.maxMessageSize = conf.getMaxMessageSize(); @@ -367,32 +365,6 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E ctx.close(); } - /** - * When transitioning from Connecting to Connected, this method validates the roles. - * If the authRole is one of proxyRoles, the following must be true: - * - the originalPrincipal is given while connecting - * - originalPrincipal is not blank - * - originalPrincipal is not a proxy principal. - * @return true when roles are valid and false when roles are invalid - */ - private boolean isValidRoleAndOriginalPrincipal() { - String errorMsg = null; - if (proxyRoles.contains(authRole)) { - if (StringUtils.isBlank(originalPrincipal)) { - errorMsg = "originalPrincipal must be provided when connecting with a proxy role."; - } else if (proxyRoles.contains(originalPrincipal)) { - errorMsg = "originalPrincipal cannot be a proxy role."; - } - } - if (errorMsg != null) { - log.warn("[{}] Illegal combination of role [{}] and originalPrincipal [{}]: {}", remoteAddress, authRole, - originalPrincipal, errorMsg); - return false; - } else { - return true; - } - } - // //// // // Incoming commands handling // //// @@ -685,7 +657,8 @@ private void doAuthentication(AuthData clientData, if (state != State.Connected) { // First time authentication is done if (service.isAuthenticationEnabled() && service.isAuthorizationEnabled()) { - if (!isValidRoleAndOriginalPrincipal()) { + if (!service.getAuthorizationService() + .isValidOriginalPrincipal(this.authRole, originalPrincipal, remoteAddress)) { state = State.Failed; service.getPulsarStats().recordConnectionCreateFail(); final ByteBuf msg = Commands.newError(-1, ServerError.AuthorizationError, "Invalid roles."); diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java index 39211aca7ad9a..67904b5ff563a 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java @@ -149,19 +149,11 @@ public static boolean isClientAuthenticated(String appId) { return appId != null; } - private static void validateOriginalPrincipal(Set proxyRoles, String authenticatedPrincipal, - String originalPrincipal) { - if (proxyRoles.contains(authenticatedPrincipal)) { - // Request has come from a proxy - if (StringUtils.isBlank(originalPrincipal)) { - log.warn("Original principal empty in request authenticated as {}", authenticatedPrincipal); - throw new RestException(Status.UNAUTHORIZED, - "Original principal cannot be empty if the request is via proxy."); - } - if (proxyRoles.contains(originalPrincipal)) { - log.warn("Original principal {} cannot be a proxy role ({})", originalPrincipal, proxyRoles); - throw new RestException(Status.UNAUTHORIZED, "Original principal cannot be a proxy role"); - } + private void validateOriginalPrincipal(String authenticatedPrincipal, String originalPrincipal) { + if (!pulsar.getBrokerService().getAuthorizationService() + .isValidOriginalPrincipal(authenticatedPrincipal, originalPrincipal, clientAuthData())) { + throw new RestException(Status.UNAUTHORIZED, + "Invalid combination of Original principal cannot be empty if the request is via proxy."); } } @@ -184,7 +176,7 @@ public CompletableFuture validateSuperUserAccessAsync(){ isClientAuthenticated(appId), appId); } String originalPrincipal = originalPrincipal(); - validateOriginalPrincipal(pulsar.getConfiguration().getProxyRoles(), appId, originalPrincipal); + validateOriginalPrincipal(appId, originalPrincipal); if (pulsar.getConfiguration().getProxyRoles().contains(appId)) { BrokerService brokerService = pulsar.getBrokerService(); @@ -259,7 +251,7 @@ protected void validateAdminAccessForTenant(String tenant) { } } - protected static void validateAdminAccessForTenant(PulsarService pulsar, String clientAppId, + protected void validateAdminAccessForTenant(PulsarService pulsar, String clientAppId, String originalPrincipal, String tenant, AuthenticationDataSource authenticationData, long timeout, TimeUnit unit) { @@ -286,7 +278,7 @@ protected CompletableFuture validateAdminAccessForTenantAsync(String tenan clientAuthData()); } - protected static CompletableFuture validateAdminAccessForTenantAsync( + protected CompletableFuture validateAdminAccessForTenantAsync( PulsarService pulsar, String clientAppId, String originalPrincipal, String tenant, AuthenticationDataSource authenticationData) { @@ -305,8 +297,7 @@ protected static CompletableFuture validateAdminAccessForTenantAsync( if (!isClientAuthenticated(clientAppId)) { throw new RestException(Status.FORBIDDEN, "Need to authenticate to perform the request"); } - validateOriginalPrincipal(pulsar.getConfiguration().getProxyRoles(), clientAppId, - originalPrincipal); + validateOriginalPrincipal(clientAppId, originalPrincipal); if (pulsar.getConfiguration().getProxyRoles().contains(clientAppId)) { AuthorizationService authorizationService = pulsar.getBrokerService().getAuthorizationService(); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java index 39a91f72dc742..ad69180b236dc 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java @@ -22,8 +22,14 @@ import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; +import com.google.common.collect.Sets; +import java.net.SocketAddress; +import java.util.Collections; import java.util.EnumSet; +import org.apache.pulsar.broker.ServiceConfiguration; +import org.apache.pulsar.broker.authentication.AuthenticationDataSource; import org.apache.pulsar.broker.authorization.AuthorizationService; +import org.apache.pulsar.broker.resources.PulsarResources; import org.apache.pulsar.client.admin.PulsarAdmin; import org.apache.pulsar.client.admin.PulsarAdminBuilder; import org.apache.pulsar.common.naming.TopicDomain; @@ -32,11 +38,11 @@ import org.apache.pulsar.common.policies.data.ClusterData; import org.apache.pulsar.common.policies.data.TenantInfoImpl; import org.apache.pulsar.common.policies.data.SubscriptionAuthMode; +import org.mockito.Mockito; import org.testng.Assert; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; -import com.google.common.collect.Sets; @Test(groups = "flaky") public class AuthorizationTest extends MockedPulsarServiceBaseTest { @@ -229,6 +235,39 @@ public void simple() throws Exception { admin.clusters().deleteCluster("c1"); } + @Test + public void testOriginalRoleValidation() throws Exception { + ServiceConfiguration conf = new ServiceConfiguration(); + conf.setProxyRoles(Collections.singleton("proxy")); + AuthorizationService auth = new AuthorizationService(conf, Mockito.mock(PulsarResources.class)); + + // Original principal should be supplied when authenticatedPrincipal is proxy role + assertTrue(auth.isValidOriginalPrincipal("proxy", "client", (SocketAddress) null)); + + // Non proxy role should not supply originalPrincipal + assertTrue(auth.isValidOriginalPrincipal("client", "", (SocketAddress) null)); + assertTrue(auth.isValidOriginalPrincipal("client", null, (SocketAddress) null)); + + // Only likely in cases when authentication is disabled, but we still define these to be valid. + assertTrue(auth.isValidOriginalPrincipal(null, null, (SocketAddress) null)); + assertTrue(auth.isValidOriginalPrincipal(null, "", (SocketAddress) null)); + assertTrue(auth.isValidOriginalPrincipal("", null, (SocketAddress) null)); + assertTrue(auth.isValidOriginalPrincipal("", "", (SocketAddress) null)); + + // Proxy role must supply an original principal + assertFalse(auth.isValidOriginalPrincipal("proxy", "", (SocketAddress) null)); + assertFalse(auth.isValidOriginalPrincipal("proxy", null, (SocketAddress) null)); + + // OriginalPrincipal cannot be proxy role + assertFalse(auth.isValidOriginalPrincipal("proxy", "proxy", (SocketAddress) null)); + assertFalse(auth.isValidOriginalPrincipal("client", "proxy", (SocketAddress) null)); + assertFalse(auth.isValidOriginalPrincipal("", "proxy", (SocketAddress) null)); + assertFalse(auth.isValidOriginalPrincipal(null, "proxy", (SocketAddress) null)); + + // Must gracefully handle a missing AuthenticationDataSource + assertTrue(auth.isValidOriginalPrincipal("proxy", "client", (AuthenticationDataSource) null)); + } + @Test public void testGetListWithGetBundleOp() throws Exception { String tenant = "p1"; diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java index 34cc3ba2232d4..f55cc387705d4 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java @@ -416,6 +416,10 @@ public void testConnectCommandWithInvalidRoleCombinations() throws Exception { verifyAuthRoleAndOriginalPrincipalBehavior(authMethodName, "pass.proxy", "pass.proxy"); verifyAuthRoleAndOriginalPrincipalBehavior(authMethodName, "pass.proxy", ""); verifyAuthRoleAndOriginalPrincipalBehavior(authMethodName, "pass.proxy", null); + // Invalid combinations where original principal is set to a pass.proxy role + verifyAuthRoleAndOriginalPrincipalBehavior(authMethodName, "pass.client", "pass.proxy"); + // Invalid combinations where the original principal is set to a non-proxy role + verifyAuthRoleAndOriginalPrincipalBehavior(authMethodName, "pass.client1", "pass.client"); } private void verifyAuthRoleAndOriginalPrincipalBehavior(String authMethodName, String authData, diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/AdminApiKeyStoreTlsAuthTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/AdminApiKeyStoreTlsAuthTest.java index 77533949b8722..fe734e06008bd 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/AdminApiKeyStoreTlsAuthTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/AdminApiKeyStoreTlsAuthTest.java @@ -47,7 +47,6 @@ import org.apache.pulsar.client.api.ProducerConsumerBase; import org.apache.pulsar.client.impl.auth.AuthenticationKeyStoreTls; import org.apache.pulsar.common.policies.data.ClusterData; -import org.apache.pulsar.common.tls.NoopHostnameVerifier; import org.apache.pulsar.common.policies.data.TenantInfoImpl; import org.apache.pulsar.common.util.keystoretls.KeyStoreSSLContext; import org.glassfish.jersey.client.ClientConfig; @@ -74,8 +73,14 @@ public class AdminApiKeyStoreTlsAuthTest extends ProducerConsumerBase { "./src/test/resources/authentication/keystoretls/client.keystore.jks"; protected final String CLIENT_TRUSTSTORE_FILE_PATH = "./src/test/resources/authentication/keystoretls/client.truststore.jks"; + protected final String PROXY_KEYSTORE_FILE_PATH = + "./src/test/resources/authentication/keystoretls/proxy.keystore.jks"; + protected final String PROXY_AND_CLIENT_TRUSTSTORE_FILE_PATH = + "./src/test/resources/authentication/keystoretls/proxy-and-client.truststore.jks"; protected final String CLIENT_KEYSTORE_PW = "111111"; protected final String CLIENT_TRUSTSTORE_PW = "111111"; + protected final String PROXY_KEYSTORE_PW = "111111"; + protected final String PROXY_AND_CLIENT_TRUSTSTORE_PW = "111111"; protected final String CLIENT_KEYSTORE_CN = "clientuser"; protected final String KEYSTORE_TYPE = "JKS"; @@ -96,8 +101,8 @@ public void setup() throws Exception { conf.setTlsKeyStorePassword(BROKER_KEYSTORE_PW); conf.setTlsTrustStoreType(KEYSTORE_TYPE); - conf.setTlsTrustStore(CLIENT_TRUSTSTORE_FILE_PATH); - conf.setTlsTrustStorePassword(CLIENT_TRUSTSTORE_PW); + conf.setTlsTrustStore(PROXY_AND_CLIENT_TRUSTSTORE_FILE_PATH); + conf.setTlsTrustStorePassword(PROXY_AND_CLIENT_TRUSTSTORE_PW); conf.setClusterName(clusterName); conf.setTlsRequireTrustedClientCertOnConnect(true); @@ -107,6 +112,7 @@ public void setup() throws Exception { // config for authentication and authorization. conf.setSuperUserRoles(Sets.newHashSet(CLIENT_KEYSTORE_CN)); + conf.setProxyRoles(Sets.newHashSet("proxy")); conf.setAuthenticationEnabled(true); conf.setAuthorizationEnabled(true); Set providers = new HashSet<>(); @@ -147,13 +153,13 @@ WebTarget buildWebClient() throws Exception { SSLContext sslCtx = KeyStoreSSLContext.createClientSslContext( KEYSTORE_TYPE, - CLIENT_KEYSTORE_FILE_PATH, - CLIENT_KEYSTORE_PW, + PROXY_KEYSTORE_FILE_PATH, + PROXY_KEYSTORE_PW, KEYSTORE_TYPE, BROKER_TRUSTSTORE_FILE_PATH, BROKER_TRUSTSTORE_PW); - clientBuilder.sslContext(sslCtx).hostnameVerifier(NoopHostnameVerifier.INSTANCE); + clientBuilder.sslContext(sslCtx); Client client = clientBuilder.build(); return client.target(brokerUrlTls.toString()); @@ -186,11 +192,11 @@ public void testSuperUserCanListTenants() throws Exception { } @Test - public void testSuperUserCantListNamespaces() throws Exception { + public void testSuperUserCanListNamespaces() throws Exception { try (PulsarAdmin admin = buildAdminClient()) { admin.clusters().createCluster("test", ClusterData.builder().serviceUrl(brokerUrl.toString()).build()); admin.tenants().createTenant("tenant1", - new TenantInfoImpl(ImmutableSet.of("proxy"), + new TenantInfoImpl(ImmutableSet.of(""), ImmutableSet.of("test"))); admin.namespaces().createNamespace("tenant1/ns1"); Assert.assertTrue(admin.namespaces().getNamespaces("tenant1").contains("tenant1/ns1")); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeyStoreTlsTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeyStoreTlsTest.java index 2e839b93f194f..425e08cb91fab 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeyStoreTlsTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeyStoreTlsTest.java @@ -31,23 +31,23 @@ public class KeyStoreTlsTest { protected final String BROKER_KEYSTORE_FILE_PATH = "./src/test/resources/authentication/keystoretls/broker.keystore.jks"; protected final String BROKER_TRUSTSTORE_FILE_PATH = - "./src/test/resources/authentication/keystoretls/broker.truststore.jks"; + "./src/test/resources/authentication/keystoretls/client.truststore.jks"; protected final String BROKER_KEYSTORE_PW = "111111"; protected final String BROKER_TRUSTSTORE_PW = "111111"; protected final String CLIENT_KEYSTORE_FILE_PATH = "./src/test/resources/authentication/keystoretls/client.keystore.jks"; protected final String CLIENT_TRUSTSTORE_FILE_PATH = - "./src/test/resources/authentication/keystoretls/client.truststore.jks"; + "./src/test/resources/authentication/keystoretls/broker.truststore.jks"; protected final String CLIENT_KEYSTORE_PW = "111111"; protected final String CLIENT_TRUSTSTORE_PW = "111111"; protected final String KEYSTORE_TYPE = "JKS"; protected final String BROKER_TRUSTSTORE_FILE_NPD_PATH = - "./src/test/resources/authentication/keystoretls/pulsar_server_trust_npd.jks"; + "./src/test/resources/authentication/keystoretls/client.truststore.nopassword.jks"; protected final String CLIENT_TRUSTSTORE_FILE_NPD_PATH = - "./src/test/resources/authentication/keystoretls/pulsar_client_trust_npd.jks"; + "./src/test/resources/authentication/keystoretls/broker.truststore.nopassword.jks"; public static final Provider BC_PROVIDER = getProvider(); diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/broker.keystore.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/broker.keystore.jks index 8ef2c6c60ba81..6f2df055f26ad 100644 Binary files a/pulsar-broker/src/test/resources/authentication/keystoretls/broker.keystore.jks and b/pulsar-broker/src/test/resources/authentication/keystoretls/broker.keystore.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/broker.truststore.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/broker.truststore.jks index 96f12a38a7227..9c35356c54021 100644 Binary files a/pulsar-broker/src/test/resources/authentication/keystoretls/broker.truststore.jks and b/pulsar-broker/src/test/resources/authentication/keystoretls/broker.truststore.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/broker.truststore.nopassword.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/broker.truststore.nopassword.jks new file mode 100644 index 0000000000000..75c3fd8012f96 Binary files /dev/null and b/pulsar-broker/src/test/resources/authentication/keystoretls/broker.truststore.nopassword.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/client.keystore.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/client.keystore.jks index 375e2e00bb410..0c9d33408e1c1 100644 Binary files a/pulsar-broker/src/test/resources/authentication/keystoretls/client.keystore.jks and b/pulsar-broker/src/test/resources/authentication/keystoretls/client.keystore.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/client.truststore.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/client.truststore.jks index 210e423145de9..ac59bd92541c5 100644 Binary files a/pulsar-broker/src/test/resources/authentication/keystoretls/client.truststore.jks and b/pulsar-broker/src/test/resources/authentication/keystoretls/client.truststore.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/client.truststore.nopassword.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/client.truststore.nopassword.jks new file mode 100644 index 0000000000000..363fafab6be7a Binary files /dev/null and b/pulsar-broker/src/test/resources/authentication/keystoretls/client.truststore.nopassword.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/proxy-and-client.truststore.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/proxy-and-client.truststore.jks new file mode 100644 index 0000000000000..45a49018d8da3 Binary files /dev/null and b/pulsar-broker/src/test/resources/authentication/keystoretls/proxy-and-client.truststore.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/proxy.keystore.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/proxy.keystore.jks new file mode 100644 index 0000000000000..d3b977560f190 Binary files /dev/null and b/pulsar-broker/src/test/resources/authentication/keystoretls/proxy.keystore.jks differ diff --git a/pulsar-broker/src/test/resources/authentication/keystoretls/proxy.truststore.jks b/pulsar-broker/src/test/resources/authentication/keystoretls/proxy.truststore.jks new file mode 100644 index 0000000000000..0e13895b1c21d Binary files /dev/null and b/pulsar-broker/src/test/resources/authentication/keystoretls/proxy.truststore.jks differ diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyAuthenticatedProducerConsumerTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyAuthenticatedProducerConsumerTest.java index 622cc7dc35f59..b6985400c81a1 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyAuthenticatedProducerConsumerTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyAuthenticatedProducerConsumerTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.Sets; +import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Optional; @@ -57,9 +58,17 @@ public class ProxyAuthenticatedProducerConsumerTest extends ProducerConsumerBase { private static final Logger log = LoggerFactory.getLogger(ProxyAuthenticatedProducerConsumerTest.class); + // Root for both proxy and client certificates private final String TLS_TRUST_CERT_FILE_PATH = "./src/test/resources/authentication/tls/cacert.pem"; - private final String TLS_SERVER_CERT_FILE_PATH = "./src/test/resources/authentication/tls/server-cert.pem"; - private final String TLS_SERVER_KEY_FILE_PATH = "./src/test/resources/authentication/tls/server-key.pem"; + + // Borrow certs for broker and proxy from other test + private final String TLS_PROXY_CERT_FILE_PATH = "./src/test/resources/authentication/tls/ProxyWithAuthorizationTest/proxy-cert.pem"; + private final String TLS_PROXY_KEY_FILE_PATH = "./src/test/resources/authentication/tls/ProxyWithAuthorizationTest/proxy-key.pem"; + private final String TLS_BROKER_TRUST_CERT_FILE_PATH = "./src/test/resources/authentication/tls/ProxyWithAuthorizationTest/broker-cacert.pem"; + private final String TLS_BROKER_CERT_FILE_PATH = "./src/test/resources/authentication/tls/ProxyWithAuthorizationTest/broker-cert.pem"; + private final String TLS_BROKER_KEY_FILE_PATH = "./src/test/resources/authentication/tls/ProxyWithAuthorizationTest/broker-key.pem"; + + // This client cert is a superUser, so use that one private final String TLS_CLIENT_CERT_FILE_PATH = "./src/test/resources/authentication/tls/client-cert.pem"; private final String TLS_CLIENT_KEY_FILE_PATH = "./src/test/resources/authentication/tls/client-key.pem"; @@ -78,20 +87,23 @@ protected void setup() throws Exception { conf.setBrokerServicePortTls(Optional.of(0)); conf.setWebServicePortTls(Optional.of(0)); conf.setTlsTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH); - conf.setTlsCertificateFilePath(TLS_SERVER_CERT_FILE_PATH); - conf.setTlsKeyFilePath(TLS_SERVER_KEY_FILE_PATH); - conf.setTlsAllowInsecureConnection(true); + conf.setTlsCertificateFilePath(TLS_BROKER_CERT_FILE_PATH); + conf.setTlsKeyFilePath(TLS_BROKER_KEY_FILE_PATH); + conf.setTlsAllowInsecureConnection(false); conf.setNumExecutorThreadPoolSize(5); Set superUserRoles = new HashSet<>(); superUserRoles.add("localhost"); superUserRoles.add("superUser"); + superUserRoles.add("Proxy"); conf.setSuperUserRoles(superUserRoles); + conf.setProxyRoles(Collections.singleton("Proxy")); conf.setBrokerClientAuthenticationPlugin(AuthenticationTls.class.getName()); conf.setBrokerClientAuthenticationParameters( - "tlsCertFile:" + TLS_CLIENT_CERT_FILE_PATH + "," + "tlsKeyFile:" + TLS_SERVER_KEY_FILE_PATH); - conf.setBrokerClientTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH); + "tlsCertFile:" + TLS_CLIENT_CERT_FILE_PATH + "," + "tlsKeyFile:" + TLS_CLIENT_KEY_FILE_PATH); + conf.setBrokerClientTrustCertsFilePath(TLS_BROKER_TRUST_CERT_FILE_PATH); + conf.setBrokerClientTlsEnabled(true); Set providers = new HashSet<>(); providers.add(AuthenticationProviderTls.class.getName()); conf.setAuthenticationProviders(providers); @@ -102,7 +114,6 @@ protected void setup() throws Exception { // start proxy service proxyConfig.setAuthenticationEnabled(true); - proxyConfig.setAuthenticationEnabled(true); proxyConfig.setServicePort(Optional.of(0)); proxyConfig.setBrokerProxyAllowedTargetPorts("*"); @@ -110,16 +121,18 @@ protected void setup() throws Exception { proxyConfig.setWebServicePort(Optional.of(0)); proxyConfig.setWebServicePortTls(Optional.of(0)); proxyConfig.setTlsEnabledWithBroker(true); + // Setting advertised address to localhost to avoid hostname verification failure + proxyConfig.setAdvertisedAddress("localhost"); // enable tls and auth&auth at proxy - proxyConfig.setTlsCertificateFilePath(TLS_SERVER_CERT_FILE_PATH); - proxyConfig.setTlsKeyFilePath(TLS_SERVER_KEY_FILE_PATH); + proxyConfig.setTlsCertificateFilePath(TLS_PROXY_CERT_FILE_PATH); + proxyConfig.setTlsKeyFilePath(TLS_PROXY_KEY_FILE_PATH); proxyConfig.setTlsTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH); proxyConfig.setBrokerClientAuthenticationPlugin(AuthenticationTls.class.getName()); proxyConfig.setBrokerClientAuthenticationParameters( - "tlsCertFile:" + TLS_CLIENT_CERT_FILE_PATH + "," + "tlsKeyFile:" + TLS_CLIENT_KEY_FILE_PATH); - proxyConfig.setBrokerClientTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH); + "tlsCertFile:" + TLS_PROXY_CERT_FILE_PATH + "," + "tlsKeyFile:" + TLS_PROXY_KEY_FILE_PATH); + proxyConfig.setBrokerClientTrustCertsFilePath(TLS_BROKER_TRUST_CERT_FILE_PATH); proxyConfig.setAuthenticationProviders(providers); proxyConfig.setMetadataStoreUrl(DUMMY_VALUE); @@ -207,10 +220,11 @@ public void testTlsSyncProducerAndConsumer() throws Exception { } protected final PulsarClient createPulsarClient(Authentication auth, String lookupUrl) throws Exception { - admin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrlTls.toString()).tlsTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH) - .allowTlsInsecureConnection(true).authentication(auth).build()); + admin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrlTls.toString()) + .tlsTrustCertsFilePath(TLS_BROKER_TRUST_CERT_FILE_PATH) + .enableTlsHostnameVerification(true).authentication(auth).build()); return PulsarClient.builder().serviceUrl(lookupUrl).statsInterval(0, TimeUnit.SECONDS) - .tlsTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH).allowTlsInsecureConnection(true).authentication(auth) + .tlsTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH).enableTlsHostnameVerification(true).authentication(auth) .enableTls(true).build(); } diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationNegTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationNegTest.java index 0fad961ba2110..23138929c84b5 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationNegTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationNegTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.Sets; +import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Optional; @@ -91,6 +92,7 @@ protected void setup() throws Exception { Set superUserRoles = new HashSet<>(); superUserRoles.add("superUser"); conf.setSuperUserRoles(superUserRoles); + conf.setProxyRoles(Collections.singleton("Proxy")); conf.setBrokerClientAuthenticationPlugin(AuthenticationTls.class.getName()); conf.setBrokerClientAuthenticationParameters( diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java index 07b71530b53a6..0775722d504e0 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java @@ -22,11 +22,15 @@ import com.google.common.collect.Sets; -import java.util.*; -import java.util.concurrent.TimeUnit; - import io.jsonwebtoken.Jwts; import io.jsonwebtoken.SignatureAlgorithm; +import java.util.Base64; +import java.util.Collections; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import javax.crypto.SecretKey; import org.apache.pulsar.broker.authentication.AuthenticationProviderToken; import org.apache.pulsar.broker.authentication.AuthenticationService; import org.apache.pulsar.broker.authentication.utils.AuthTokenUtils; @@ -49,7 +53,7 @@ import javax.crypto.SecretKey; public class ProxyWithJwtAuthorizationTest extends ProducerConsumerBase { - private static final Logger log = LoggerFactory.getLogger(ProxyWithAuthorizationTest.class); + private static final Logger log = LoggerFactory.getLogger(ProxyWithJwtAuthorizationTest.class); private final String ADMIN_ROLE = "admin"; private final String PROXY_ROLE = "proxy"; @@ -78,6 +82,7 @@ protected void setup() throws Exception { superUserRoles.add(PROXY_ROLE); superUserRoles.add(BROKER_ROLE); conf.setSuperUserRoles(superUserRoles); + conf.setProxyRoles(Collections.singleton(PROXY_ROLE)); conf.setBrokerClientAuthenticationPlugin(AuthenticationToken.class.getName()); conf.setBrokerClientAuthenticationParameters(BROKER_TOKEN);