This is an automated email from the ASF dual-hosted git repository.
rsivaram pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new 7cef37c KAFKA-7324: NPE due to lack of SASLExtensions in
SASL/OAUTHBEARER (#5552)
7cef37c is described below
commit 7cef37cf55353f542db3562157547d26c992e782
Author: Ron Dagostino <[email protected]>
AuthorDate: Wed Aug 29 06:16:29 2018 -0400
KAFKA-7324: NPE due to lack of SASLExtensions in SASL/OAUTHBEARER (#5552)
Set empty extensions if null is passed in.
Reviewers: Satish Duggana <[email protected]>, Stanislav Kozlovski
<[email protected]>, Rajini Sivaram <[email protected]>
---
.../kafka/common/security/auth/SaslExtensions.java | 4 ++
.../security/auth/SaslExtensionsCallback.java | 16 ++++--
.../OAuthBearerClientInitialResponse.java | 63 ++++++++++++++++++++--
.../OAuthBearerClientInitialResponseTest.java | 22 ++++++++
.../internals/OAuthBearerSaslClientTest.java | 35 ++++++++++--
5 files changed, 127 insertions(+), 13 deletions(-)
diff --git
a/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensions.java
b/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensions.java
index 75cac05..c129f1e 100644
---
a/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensions.java
+++
b/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensions.java
@@ -24,6 +24,10 @@ import java.util.Map;
* A simple immutable value object class holding customizable SASL extensions
*/
public class SaslExtensions {
+ /**
+ * An "empty" instance indicating no SASL extensions
+ */
+ public static final SaslExtensions NO_SASL_EXTENSIONS = new
SaslExtensions(Collections.emptyMap());
private final Map<String, String> extensionsMap;
public SaslExtensions(Map<String, String> extensionsMap) {
diff --git
a/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensionsCallback.java
b/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensionsCallback.java
index d07be32..c5bd449 100644
---
a/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensionsCallback.java
+++
b/clients/src/main/java/org/apache/kafka/common/security/auth/SaslExtensionsCallback.java
@@ -17,6 +17,8 @@
package org.apache.kafka.common.security.auth;
+import java.util.Objects;
+
import javax.security.auth.callback.Callback;
/**
@@ -24,11 +26,14 @@ import javax.security.auth.callback.Callback;
* in the SASL exchange.
*/
public class SaslExtensionsCallback implements Callback {
- private SaslExtensions extensions;
+ private SaslExtensions extensions = SaslExtensions.NO_SASL_EXTENSIONS;
/**
- * Returns a {@link SaslExtensions} consisting of the extension names and
values that are sent by the client to
- * the server in the initial client SASL authentication message.
+ * Returns always non-null {@link SaslExtensions} consisting of the
extension
+ * names and values that are sent by the client to the server in the
initial
+ * client SASL authentication message. The default value is
+ * {@link SaslExtensions#NO_SASL_EXTENSIONS} so that if this callback is
+ * unhandled the client will see a non-null value.
*/
public SaslExtensions extensions() {
return extensions;
@@ -36,8 +41,11 @@ public class SaslExtensionsCallback implements Callback {
/**
* Sets the SASL extensions on this callback.
+ *
+ * @param extensions
+ * the mandatory extensions to set
*/
public void extensions(SaslExtensions extensions) {
- this.extensions = extensions;
+ this.extensions = Objects.requireNonNull(extensions, "extensions must
not be null");
}
}
diff --git
a/clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponse.java
b/clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponse.java
index ef16ea2..a356f0d 100644
---
a/clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponse.java
+++
b/clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponse.java
@@ -22,6 +22,7 @@ import org.apache.kafka.common.utils.Utils;
import javax.security.sasl.SaslException;
import java.nio.charset.StandardCharsets;
import java.util.Map;
+import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -58,7 +59,9 @@ public class OAuthBearerClientInitialResponse {
if (auth == null)
throw new SaslException("Invalid OAUTHBEARER client first message:
'auth' not specified");
properties.remove(AUTH_KEY);
- this.saslExtensions = validateExtensions(new
SaslExtensions(properties));
+ SaslExtensions extensions = new SaslExtensions(properties);
+ validateExtensions(extensions);
+ this.saslExtensions = extensions;
Matcher authMatcher = AUTH_PATTERN.matcher(auth);
if (!authMatcher.matches())
@@ -71,16 +74,48 @@ public class OAuthBearerClientInitialResponse {
this.tokenValue = authMatcher.group("token");
}
+ /**
+ * Constructor
+ *
+ * @param tokenValue
+ * the mandatory token value
+ * @param extensions
+ * the optional extensions
+ * @throws SaslException
+ * if any extension name or value fails to conform to the
required
+ * regular expression as defined by the specification, or if
the
+ * reserved {@code auth} appears as a key
+ */
public OAuthBearerClientInitialResponse(String tokenValue, SaslExtensions
extensions) throws SaslException {
this(tokenValue, "", extensions);
}
+ /**
+ * Constructor
+ *
+ * @param tokenValue
+ * the mandatory token value
+ * @param authorizationId
+ * the optional authorization ID
+ * @param extensions
+ * the optional extensions
+ * @throws SaslException
+ * if any extension name or value fails to conform to the
required
+ * regular expression as defined by the specification, or if
the
+ * reserved {@code auth} appears as a key
+ */
public OAuthBearerClientInitialResponse(String tokenValue, String
authorizationId, SaslExtensions extensions) throws SaslException {
- this.tokenValue = tokenValue;
+ this.tokenValue = Objects.requireNonNull(tokenValue, "token value must
not be null");
this.authorizationId = authorizationId == null ? "" : authorizationId;
- this.saslExtensions = validateExtensions(extensions);
+ validateExtensions(extensions);
+ this.saslExtensions = extensions != null ? extensions :
SaslExtensions.NO_SASL_EXTENSIONS;
}
+ /**
+ * Return the always non-null extensions
+ *
+ * @return the always non-null extensions
+ */
public SaslExtensions extensions() {
return saslExtensions;
}
@@ -97,10 +132,20 @@ public class OAuthBearerClientInitialResponse {
return message.getBytes(StandardCharsets.UTF_8);
}
+ /**
+ * Return the always non-null token value
+ *
+ * @return the always non-null toklen value
+ */
public String tokenValue() {
return tokenValue;
}
+ /**
+ * Return the always non-null authorization ID
+ *
+ * @return the always non-null authorization ID
+ */
public String authorizationId() {
return authorizationId;
}
@@ -108,10 +153,19 @@ public class OAuthBearerClientInitialResponse {
/**
* Validates that the given extensions conform to the standard. They
should also not contain the reserve key name {@link
OAuthBearerClientInitialResponse#AUTH_KEY}
*
+ * @param extensions
+ * optional extensions to validate
+ * @throws SaslException
+ * if any extension name or value fails to conform to the
required
+ * regular expression as defined by the specification, or if
the
+ * reserved {@code auth} appears as a key
+ *
* @see <a href="https://tools.ietf.org/html/rfc7628#section-3.1";>RFC 7628,
* Section 3.1</a>
*/
- public static SaslExtensions validateExtensions(SaslExtensions extensions)
throws SaslException {
+ public static void validateExtensions(SaslExtensions extensions) throws
SaslException {
+ if (extensions == null)
+ return;
if
(extensions.map().containsKey(OAuthBearerClientInitialResponse.AUTH_KEY))
throw new SaslException("Extension name " +
OAuthBearerClientInitialResponse.AUTH_KEY + " is invalid");
@@ -124,7 +178,6 @@ public class OAuthBearerClientInitialResponse {
if (!EXTENSION_VALUE_PATTERN.matcher(extensionValue).matches())
throw new SaslException("Extension value (" + extensionValue +
") for extension " + extensionName + " is invalid");
}
- return extensions;
}
/**
diff --git
a/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponseTest.java
b/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponseTest.java
index 3de6408..0ba9565 100644
---
a/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponseTest.java
+++
b/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerClientInitialResponseTest.java
@@ -17,6 +17,7 @@
package org.apache.kafka.common.security.oauthbearer.internals;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import org.apache.kafka.common.security.auth.SaslExtensions;
import org.junit.Test;
@@ -99,4 +100,25 @@ public class OAuthBearerClientInitialResponseTest {
assertEquals("server.example.com",
response.extensions().map().get("host"));
assertEquals("143", response.extensions().map().get("port"));
}
+
+ @Test
+ public void testNoExtensionsFromByteArray() throws Exception {
+ String message = "n,[email protected],\u0001" +
+ "auth=Bearer
vF9dft4qmTc2Nvb3RlckBhbHRhdmlzdGEuY29tCg\u0001\u0001";
+ OAuthBearerClientInitialResponse response = new
OAuthBearerClientInitialResponse(message.getBytes(StandardCharsets.UTF_8));
+ assertEquals("vF9dft4qmTc2Nvb3RlckBhbHRhdmlzdGEuY29tCg",
response.tokenValue());
+ assertEquals("[email protected]", response.authorizationId());
+ assertTrue(response.extensions().map().isEmpty());
+ }
+
+ @Test
+ public void testNoExtensionsFromTokenAndNullExtensions() throws Exception {
+ OAuthBearerClientInitialResponse response = new
OAuthBearerClientInitialResponse("token", null);
+ assertTrue(response.extensions().map().isEmpty());
+ }
+
+ @Test
+ public void testValidateNullExtensions() throws Exception {
+ OAuthBearerClientInitialResponse.validateExtensions(null);
+ }
}
diff --git
a/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerSaslClientTest.java
b/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerSaslClientTest.java
index 55a8624..fad7431 100644
---
a/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerSaslClientTest.java
+++
b/clients/src/test/java/org/apache/kafka/common/security/oauthbearer/internals/OAuthBearerSaslClientTest.java
@@ -20,8 +20,8 @@ import org.apache.kafka.common.config.ConfigException;
import org.apache.kafka.common.security.auth.SaslExtensionsCallback;
import org.apache.kafka.common.security.auth.AuthenticateCallbackHandler;
import org.apache.kafka.common.security.auth.SaslExtensions;
+import org.apache.kafka.common.security.oauthbearer.OAuthBearerToken;
import org.apache.kafka.common.security.oauthbearer.OAuthBearerTokenCallback;
-import
org.apache.kafka.common.security.oauthbearer.internals.unsecured.OAuthBearerUnsecuredJws;
import org.easymock.EasyMockSupport;
import org.junit.Test;
@@ -30,9 +30,11 @@ import
javax.security.auth.callback.UnsupportedCallbackException;
import javax.security.auth.login.AppConfigurationEntry;
import javax.security.sasl.SaslException;
import java.nio.charset.StandardCharsets;
+import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@@ -70,7 +72,32 @@ public class OAuthBearerSaslClientTest extends
EasyMockSupport {
public void handle(Callback[] callbacks) throws
UnsupportedCallbackException {
for (Callback callback : callbacks) {
if (callback instanceof OAuthBearerTokenCallback)
- ((OAuthBearerTokenCallback)
callback).token(createMock(OAuthBearerUnsecuredJws.class));
+ ((OAuthBearerTokenCallback) callback).token(new
OAuthBearerToken() {
+ @Override
+ public String value() {
+ return "";
+ }
+
+ @Override
+ public Set<String> scope() {
+ return Collections.emptySet();
+ }
+
+ @Override
+ public long lifetimeMs() {
+ return 100;
+ }
+
+ @Override
+ public String principalName() {
+ return "principalName";
+ }
+
+ @Override
+ public Long startTimeMs() {
+ return null;
+ }
+ });
else if (callback instanceof SaslExtensionsCallback) {
if (toThrow)
throw new ConfigException(errorMessage);
@@ -88,7 +115,7 @@ public class OAuthBearerSaslClientTest extends
EasyMockSupport {
@Test
public void testAttachesExtensionsToFirstClientMessage() throws Exception {
- String expectedToken = new String(new
OAuthBearerClientInitialResponse(null, testExtensions).toBytes(),
StandardCharsets.UTF_8);
+ String expectedToken = new String(new
OAuthBearerClientInitialResponse("", testExtensions).toBytes(),
StandardCharsets.UTF_8);
OAuthBearerSaslClient client = new OAuthBearerSaslClient(new
ExtensionsCallbackHandler(false));
@@ -101,7 +128,7 @@ public class OAuthBearerSaslClientTest extends
EasyMockSupport {
public void testNoExtensionsDoesNotAttachAnythingToFirstClientMessage()
throws Exception {
TEST_PROPERTIES.clear();
testExtensions = new SaslExtensions(TEST_PROPERTIES);
- String expectedToken = new String(new
OAuthBearerClientInitialResponse(null, new
SaslExtensions(TEST_PROPERTIES)).toBytes(), StandardCharsets.UTF_8);
+ String expectedToken = new String(new
OAuthBearerClientInitialResponse("", new
SaslExtensions(TEST_PROPERTIES)).toBytes(), StandardCharsets.UTF_8);
OAuthBearerSaslClient client = new OAuthBearerSaslClient(new
ExtensionsCallbackHandler(false));
String message = new String(client.evaluateChallenge("".getBytes()),
StandardCharsets.UTF_8);
