You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2018/03/05 07:49:07 UTC
kudu git commit: java: improve error messages when tokens are not used
Repository: kudu
Updated Branches:
refs/heads/master 9b6034b4e -> cdc73b900
java: improve error messages when tokens are not used
We recently had some trouble debugging Java client connection failures
where we expected it to authenticate by token but it did not. Previously
it just complained that it couldn't authenticate by SASL and didn't give
any clue as to why it didn't use tokens.
This patch makes the error more clear by explaining why a token was not
used to authenticate.
Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Reviewed-on: http://gerrit.cloudera.org:8080/9070
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/cdc73b90
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/cdc73b90
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/cdc73b90
Branch: refs/heads/master
Commit: cdc73b900608000ff2d2f8bc74c05893338454ba
Parents: 9b6034b
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Jan 18 15:34:48 2018 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon Mar 5 07:33:19 2018 +0000
----------------------------------------------------------------------
.../java/org/apache/kudu/client/Negotiator.java | 45 +++++++++++++++++---
.../org/apache/kudu/client/SecurityContext.java | 4 +-
.../org/apache/kudu/client/TestSecurity.java | 45 ++++++++++++++++++--
3 files changed, 83 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/cdc73b90/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
----------------------------------------------------------------------
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 c58ffe6..fd609d3 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
@@ -155,6 +155,19 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
*/
private final SignedTokenPB authnToken;
+ private static enum AuthnTokenNotUsedReason {
+ NONE_AVAILABLE("no token is available"),
+ NO_TRUSTED_CERTS("no TLS certificates are trusted by the client"),
+ FORBIDDEN_BY_POLICY("this connection does not allow authentication by tokens"),
+ NOT_CHOSEN_BY_SERVER("the server chose not to accept token authentication");
+
+ AuthnTokenNotUsedReason(String msg) {
+ this.msg = msg;
+ }
+ final String msg;
+ };
+ private AuthnTokenNotUsedReason authnTokenNotUsedReason = null;
+
private State state = State.INITIAL;
private SaslClient saslClient;
@@ -198,10 +211,20 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
boolean ignoreAuthnToken) {
this.remoteHostname = remoteHostname;
this.securityContext = securityContext;
- if (ignoreAuthnToken) {
- this.authnToken = null;
+ SignedTokenPB token = securityContext.getAuthenticationToken();
+ if (token != null) {
+ if (ignoreAuthnToken) {
+ this.authnToken = null;
+ this.authnTokenNotUsedReason = AuthnTokenNotUsedReason.FORBIDDEN_BY_POLICY;
+ } else if (!securityContext.hasTrustedCerts()) {
+ this.authnToken = null;
+ this.authnTokenNotUsedReason = AuthnTokenNotUsedReason.NO_TRUSTED_CERTS;
+ } else {
+ this.authnToken = token;
+ }
} else {
- this.authnToken = securityContext.getAuthenticationToken();
+ this.authnToken = null;
+ this.authnTokenNotUsedReason = AuthnTokenNotUsedReason.NONE_AVAILABLE;
}
}
@@ -230,7 +253,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
// We may also have a token. But, we can only use the token
// if we are able to use authenticated TLS to authenticate the server.
- if (authnToken != null && securityContext.hasTrustedCerts()) {
+ if (authnToken != null) {
builder.addAuthnTypesBuilder().setToken(
AuthenticationTypePB.Token.getDefaultInstance());
}
@@ -396,7 +419,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
if (clientMech.equals(SaslMechanism.GSSAPI) &&
(securityContext.getSubject() == null ||
securityContext.getSubject().getPrivateCredentials(KerberosTicket.class).isEmpty())) {
- errorsByMech.put(clientMech.name(), "Client does not have Kerberos credentials (tgt)");
+ errorsByMech.put(clientMech.name(), "client does not have Kerberos credentials (tgt)");
continue;
}
@@ -437,13 +460,18 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
if (serverMechs.size() == 1 && serverMechs.contains(SaslMechanism.GSSAPI)) {
// Give a better error diagnostic for common case of an unauthenticated client connecting
// to a secure server.
- message = "server requires authentication, " +
- "but client does not have Kerberos credentials available";
+ message = "server requires authentication, but " +
+ errorsByMech.get(SaslMechanism.GSSAPI.name());
} else {
message = "client/server supported SASL mechanism mismatch: [" +
Joiner.on(", ").withKeyValueSeparator(": ").join(errorsByMech) + "]";
}
+ if (authnTokenNotUsedReason != null) {
+ message += ". Authentication tokens were not used because " +
+ authnTokenNotUsedReason.msg;
+ }
+
// If client has valid secondary authn credentials (such as authn token),
// but it does not have primary authn credentials (such as Kerberos creds),
// throw a recoverable exception. So that the request can be retried as long
@@ -469,6 +497,9 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
AuthenticationTypePB.TypeCase type = response.getAuthnTypes(0).getTypeCase();
switch (type) {
case SASL:
+ if (authnToken != null) {
+ authnTokenNotUsedReason = AuthnTokenNotUsedReason.NOT_CHOSEN_BY_SERVER;
+ }
break;
case TOKEN:
if (authnToken == null) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/cdc73b90/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
index 0340bfd..e9bf0c7 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
@@ -33,6 +33,7 @@ import javax.net.ssl.TrustManagerFactory;
import javax.net.ssl.X509TrustManager;
import javax.security.auth.Subject;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
@@ -75,7 +76,8 @@ class SecurityContext {
/**
* The currently trusted CA certs, in DER format.
*/
- private List<ByteString> trustedCertDers = Collections.emptyList();
+ @VisibleForTesting
+ List<ByteString> trustedCertDers = Collections.emptyList();
/**
* Construct SecurityContext object with the specified JAAS subject.
http://git-wip-us.apache.org/repos/asf/kudu/blob/cdc73b90/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
----------------------------------------------------------------------
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 290526b..793e1f3 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
@@ -20,14 +20,16 @@ import java.util.List;
import com.stumbleupon.async.Deferred;
+import org.apache.kudu.client.Client.AuthenticationCredentialsPB;
import org.apache.kudu.master.Master.ConnectToMasterResponsePB;
import org.apache.kudu.util.AssertHelpers.BooleanExpression;
import org.apache.kudu.util.SecurityUtil;
import org.junit.Assert;
import org.junit.BeforeClass;
-import org.junit.Ignore;
import org.junit.Test;
+import org.hamcrest.CoreMatchers;
+
public class TestSecurity extends BaseKuduTest {
private static final String TABLE_NAME = "TestSecurity-table";
@@ -61,8 +63,10 @@ public class TestSecurity extends BaseKuduTest {
Assert.fail("should not have been able to connect to a secure cluster " +
"with no credentials");
} catch (NonRecoverableException e) {
- Assert.assertTrue(e.getMessage().contains("server requires authentication, " +
- "but client does not have Kerberos credentials available"));
+ Assert.assertThat(e.getMessage(), CoreMatchers.containsString(
+ "server requires authentication, but client does not have " +
+ "Kerberos credentials (tgt). Authentication tokens were not used " +
+ "because no token is available"));
}
// If we import the authentication data from the old authenticated client,
@@ -80,6 +84,41 @@ public class TestSecurity extends BaseKuduTest {
}
/**
+ * Test that if, for some reason, the client has a token but no CA certs, it
+ * will emit an appropriate error message in the exception.
+ */
+ @Test
+ public void testErrorMessageWithNoCaCert() throws Exception {
+ byte[] authnData = client.exportAuthenticationCredentials().join();
+
+ // Remove the CA certs from the credentials.
+ authnData = AuthenticationCredentialsPB.parseFrom(authnData).toBuilder()
+ .clearCaCertDers().build().toByteArray();
+
+ String oldTicketCache = System.getProperty(SecurityUtil.KUDU_TICKETCACHE_PROPERTY);
+ System.clearProperty(SecurityUtil.KUDU_TICKETCACHE_PROPERTY);
+ try {
+ KuduClient newClient = new KuduClient.KuduClientBuilder(masterAddresses).build();
+ newClient.importAuthenticationCredentials(authnData);
+
+ // We shouldn't be able to connect because we have no appropriate CA cert.
+ try {
+ newClient.listTabletServers();
+ Assert.fail("should not have been able to connect to a secure cluster " +
+ "with no credentials");
+ } catch (NonRecoverableException e) {
+ Assert.assertThat(e.getMessage(), CoreMatchers.containsString(
+ "server requires authentication, but client does not have " +
+ "Kerberos credentials (tgt). Authentication tokens were not used " +
+ "because no TLS certificates are trusted by the client"));
+ }
+ } finally {
+ // Restore ticket cache for other test cases.
+ System.setProperty(SecurityUtil.KUDU_TICKETCACHE_PROPERTY, oldTicketCache);
+ }
+ }
+
+ /**
* Regression test for KUDU-2267 and KUDU-2319.
*
* A client with valid token but without valid Kerberos credentials