This is an automated email from the ASF dual-hosted git repository. abukor pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit e69b3c652d0c21393015aad8e5acd5ece2098573 Author: Attila Bukor <[email protected]> AuthorDate: Wed Apr 7 14:45:22 2021 +0200 [java] KUDU-1884 Make SASL proto name configurable The server-side Kerberos service principal name must match the client-side SASL protocol name. This patch makes this configurable through the client builder API. Change-Id: I4a881f2cf125651277927b43a5b31afc173a9bab Reviewed-on: http://gerrit.cloudera.org:8080/17280 Reviewed-by: Alexey Serbin <[email protected]> Tested-by: Attila Bukor <[email protected]> Reviewed-by: Grant Henke <[email protected]> --- .../org/apache/kudu/client/AsyncKuduClient.java | 17 +++++++++++++- .../java/org/apache/kudu/client/Connection.java | 11 ++++++--- .../org/apache/kudu/client/ConnectionCache.java | 9 ++++++-- .../java/org/apache/kudu/client/KuduClient.java | 14 +++++++++++ .../java/org/apache/kudu/client/Negotiator.java | 8 +++++-- .../org/apache/kudu/client/TestNegotiator.java | 2 +- .../java/org/apache/kudu/client/TestSecurity.java | 27 ++++++++++++++++++++++ .../apache/kudu/test/cluster/MiniKuduCluster.java | 16 ++++++++++--- 8 files changed, 92 insertions(+), 12 deletions(-) diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java index d8ba883..0710db8 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java @@ -425,7 +425,7 @@ public class AsyncKuduClient implements AutoCloseable { this.requestTracker = new RequestTracker(clientId); this.securityContext = new SecurityContext(); - this.connectionCache = new ConnectionCache(securityContext, bootstrap); + this.connectionCache = new ConnectionCache(securityContext, bootstrap, b.saslProtocolName); this.tokenReacquirer = new AuthnTokenReacquirer(this); this.authzTokenCache = new AuthzTokenCache(this); } @@ -2731,6 +2731,7 @@ public class AsyncKuduClient implements AutoCloseable { private Executor workerExecutor; private int workerCount = DEFAULT_WORKER_COUNT; private boolean statisticsDisabled = false; + private String saslProtocolName = "kudu"; /** * Creates a new builder for a client that will connect to the specified masters. @@ -2892,6 +2893,20 @@ public class AsyncKuduClient implements AutoCloseable { } /** + * Set the SASL protocol name. + * SASL protocol name is used when connecting to a secure (Kerberos-enabled) + * cluster. It must match the servers' service principal name (SPN). + * + * Optional. + * If not provided, it will use the default SASL protocol name ("kudu"). + * @return this builder + */ + public AsyncKuduClientBuilder saslProtocolName(String saslProtocolName) { + this.saslProtocolName = saslProtocolName; + return this; + } + + /** * Creates a new client that connects to the masters. * Doesn't block and won't throw an exception if the masters don't exist. * @return a new asynchronous Kudu client diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java index e712475..74445ef 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java @@ -108,6 +108,8 @@ class Connection extends SimpleChannelInboundHandler<Object> { /** The Netty client bootstrap used to configure and initialize a connected channel. */ private final Bootstrap bootstrap; + private final String saslProtocolName; + /** The underlying Netty's socket channel. */ private SocketChannel channel; @@ -178,14 +180,17 @@ class Connection extends SimpleChannelInboundHandler<Object> { * @param credentialsPolicy policy controlling which credentials to use while negotiating on the * connection to the target server: * if {@link CredentialsPolicy#PRIMARY_CREDENTIALS}, the authentication - * token from the security context is ignored + * @param saslProtocolName SASL protocol name used when connecting to secure + * clusters. Must match the servers' service principal name. */ Connection(ServerInfo serverInfo, SecurityContext securityContext, Bootstrap bootstrap, - CredentialsPolicy credentialsPolicy) { + CredentialsPolicy credentialsPolicy, + String saslProtocolName) { this.serverInfo = serverInfo; this.securityContext = securityContext; + this.saslProtocolName = saslProtocolName; this.state = State.NEW; this.credentialsPolicy = credentialsPolicy; this.bootstrap = bootstrap.clone(); @@ -208,7 +213,7 @@ class Connection extends SimpleChannelInboundHandler<Object> { } ctx.writeAndFlush(Unpooled.wrappedBuffer(CONNECTION_HEADER), ctx.voidPromise()); Negotiator negotiator = new Negotiator(serverInfo.getAndCanonicalizeHostname(), securityContext, - (credentialsPolicy == CredentialsPolicy.PRIMARY_CREDENTIALS)); + (credentialsPolicy == CredentialsPolicy.PRIMARY_CREDENTIALS), saslProtocolName); ctx.pipeline().addBefore(ctx.name(), "negotiation", negotiator); negotiator.sendHello(ctx); } diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java index 83bff27..705f962 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java @@ -56,6 +56,8 @@ class ConnectionCache { /** Netty's bootstrap to use by connections. */ private final Bootstrap bootstrap; + private final String saslProtocolName; + /** * Container mapping server IP/port into the established connection from the client to the * server. It may be up to two connections per server: one established with secondary @@ -67,9 +69,11 @@ class ConnectionCache { /** Create a new empty ConnectionCache given the specified parameters. */ ConnectionCache(SecurityContext securityContext, - Bootstrap bootstrap) { + Bootstrap bootstrap, + String saslProtocolName) { this.securityContext = securityContext; this.bootstrap = bootstrap; + this.saslProtocolName = saslProtocolName; } /** @@ -122,7 +126,8 @@ class ConnectionCache { result = new Connection(serverInfo, securityContext, bootstrap, - credentialsPolicy); + credentialsPolicy, + saslProtocolName); connections.add(result); // There can be at most 2 connections to the same destination: one with primary and another // with secondary credentials. diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java index a77689b..ad518a6 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java @@ -609,6 +609,20 @@ public class KuduClient implements AutoCloseable { } /** + * Set the SASL protocol name. + * SASL protocol name is used when connecting to a secure (Kerberos-enabled) + * cluster. It must match the servers' service principal name (SPN). + * + * Optional. + * If not provided, it will use the default SASL protocol name ("kudu"). + * @return this builder + */ + public KuduClientBuilder saslProtocolName(String saslProtocolName) { + clientBuilder.saslProtocolName(saslProtocolName); + return this; + } + + /** * Creates a new client that connects to the masters. * Doesn't block and won't throw an exception if the masters don't exist. * @return a new asynchronous Kudu client diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java index e570665..234984b 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java @@ -222,14 +222,18 @@ public class Negotiator extends SimpleChannelInboundHandler<CallResponse> { private Certificate peerCert; + private String saslProtocolName; + @InterfaceAudience.LimitedPrivate("Test") boolean overrideLoopbackForTests; public Negotiator(String remoteHostname, SecurityContext securityContext, - boolean ignoreAuthnToken) { + boolean ignoreAuthnToken, + String saslProtocolName) { this.remoteHostname = remoteHostname; this.securityContext = securityContext; + this.saslProtocolName = saslProtocolName; SignedTokenPB token = securityContext.getAuthenticationToken(); if (token != null) { if (ignoreAuthnToken) { @@ -456,7 +460,7 @@ public class Negotiator extends SimpleChannelInboundHandler<CallResponse> { try { saslClient = Sasl.createSaslClient(new String[]{ clientMech.name() }, null, - "kudu", + saslProtocolName, remoteHostname, props, saslCallback); diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java index 4d4dce6..0da8368 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java @@ -117,7 +117,7 @@ public class TestNegotiator { } private void startNegotiation(boolean fakeLoopback) { - Negotiator negotiator = new Negotiator("127.0.0.1", secContext, false); + Negotiator negotiator = new Negotiator("127.0.0.1", secContext, false, "kudu"); negotiator.overrideLoopbackForTests = fakeLoopback; embedder = new EmbeddedChannel(negotiator); negotiator.sendHello(embedder.pipeline().firstContext()); diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java index d4f993d..88037a08 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java @@ -55,6 +55,7 @@ public class TestSecurity { private static final String TABLE_NAME = "TestSecurity-table"; private static final int TICKET_LIFETIME_SECS = 10; private static final int RENEWABLE_LIFETIME_SECS = 20; + public static final String CUSTOM_PRINCIPAL = "oryx"; private CapturingLogAppender cla; private MiniKuduCluster miniCluster; @@ -64,6 +65,7 @@ public class TestSecurity { LONG_LEADER_ELECTION, SHORT_TOKENS_AND_TICKETS, START_TSERVERS, + CUSTOM_PRINCIPAL, } private static class KeyValueMessage { @@ -89,6 +91,9 @@ public class TestSecurity { .kdcRenewLifetime(RENEWABLE_LIFETIME_SECS + "s") .kdcTicketLifetime(TICKET_LIFETIME_SECS + "s"); } + if (opts.contains(Option.CUSTOM_PRINCIPAL)) { + mcb.principal(CUSTOM_PRINCIPAL); + } miniCluster = mcb.numMasterServers(3) .numTabletServers(opts.contains(Option.START_TSERVERS) ? 3 : 0) .build(); @@ -491,4 +496,26 @@ public class TestSecurity { } } } + + @Test(timeout = 60000) + public void testNonDefaultPrincipal() throws Exception { + startCluster(ImmutableSet.of(Option.CUSTOM_PRINCIPAL, Option.START_TSERVERS)); + try { + this.client.createTable("TestSecurity-nondefault-principal-1", + getBasicSchema(), + getBasicCreateTableOptions()); + Assert.fail("default client shouldn't be able to connect to the cluster."); + } catch (NonRecoverableException e) { + Assert.assertThat(e.getMessage(), CoreMatchers.containsString( + "this client is not authenticated" + )); + } + KuduClient client = new KuduClient.KuduClientBuilder(miniCluster.getMasterAddressesAsString()) + .saslProtocolName(CUSTOM_PRINCIPAL) + .build(); + Assert.assertNotNull(client.createTable( "TestSecurity-nondefault-principal-2", + getBasicSchema(), + getBasicCreateTableOptions())); + } + } diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java index 09ac649..2dcd781 100644 --- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java +++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java @@ -106,6 +106,7 @@ public final class MiniKuduCluster implements AutoCloseable { private final ImmutableList<String> extraMasterFlags; private final ImmutableList<String> locationInfo; private final String clusterRoot; + private final String principal; private MiniKdcOptionsPB kdcOptionsPb; private final Common.HmsMode hmsMode; @@ -118,7 +119,8 @@ public final class MiniKuduCluster implements AutoCloseable { List<String> locationInfo, MiniKdcOptionsPB kdcOptionsPb, String clusterRoot, - Common.HmsMode hmsMode) { + Common.HmsMode hmsMode, + String principal) { this.enableKerberos = enableKerberos; this.numMasters = numMasters; this.numTservers = numTservers; @@ -126,6 +128,7 @@ public final class MiniKuduCluster implements AutoCloseable { this.extraMasterFlags = ImmutableList.copyOf(extraMasterFlags); this.locationInfo = ImmutableList.copyOf(locationInfo); this.kdcOptionsPb = kdcOptionsPb; + this.principal = principal; this.hmsMode = hmsMode; if (clusterRoot == null) { @@ -220,7 +223,8 @@ public final class MiniKuduCluster implements AutoCloseable { .addAllExtraMasterFlags(extraMasterFlags) .addAllExtraTserverFlags(extraTserverFlags) .setMiniKdcOptions(kdcOptionsPb) - .setClusterRoot(clusterRoot); + .setClusterRoot(clusterRoot) + .setPrincipal(principal); // Set up the location mapping command flag if there is location info. if (!locationInfo.isEmpty()) { @@ -610,6 +614,7 @@ public final class MiniKuduCluster implements AutoCloseable { private final List<String> extraMasterServerFlags = new ArrayList<>(); private final List<String> locationInfo = new ArrayList<>(); private String clusterRoot = null; + private String principal = "kudu"; private MiniKdcOptionsPB.Builder kdcOptionsPb = MiniKdcOptionsPB.newBuilder(); private Common.HmsMode hmsMode = Common.HmsMode.NONE; @@ -690,6 +695,11 @@ public final class MiniKuduCluster implements AutoCloseable { return this; } + public MiniKuduClusterBuilder principal(String principal) { + this.principal = principal; + return this; + } + /** * Builds and starts a new {@link MiniKuduCluster} using builder state. * @return the newly started {@link MiniKuduCluster} @@ -700,7 +710,7 @@ public final class MiniKuduCluster implements AutoCloseable { new MiniKuduCluster(enableKerberos, numMasterServers, numTabletServers, extraTabletServerFlags, extraMasterServerFlags, locationInfo, - kdcOptionsPb.build(), clusterRoot, hmsMode); + kdcOptionsPb.build(), clusterRoot, hmsMode, principal); try { cluster.start(); } catch (IOException e) {
