[
https://issues.apache.org/jira/browse/KAFKA-7324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16596172#comment-16596172
]
ASF GitHub Bot commented on KAFKA-7324:
---------------------------------------
rajinisivaram closed pull request #5552: KAFKA-7324: NPE due to lack of
SASLExtensions in SASL/OAUTHBEARER
URL: https://github.com/apache/kafka/pull/5552
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 75cac0533ea..c129f1ec400 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 @@
* 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 d07be320625..c5bd449e0cc 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 @@
* 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 SaslExtensions extensions() {
/**
* 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 ef16ea237d4..a356f0da3dd 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 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 OAuthBearerClientInitialResponse(byte[] response)
throws SaslException {
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 OAuthBearerClientInitialResponse(byte[] response)
throws SaslException {
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 SaslExtensions extensions() {
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 String authorizationId() {
/**
* 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 static SaslExtensions
validateExtensions(SaslExtensions extensions) throw
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 3de6408accd..0ba956561df 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 void testRfc7688Example() throws Exception {
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 55a86245da4..fad743136f3 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.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.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 void configure(Map<String, ?> configs, String
saslMechanism, List<AppConf
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 void close() {
@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 void testAttachesExtensionsToFirstClientMessage()
throws Exception {
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);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> NPE due to lack of SASLExtensions in SASL/OAUTHBEARER
> -----------------------------------------------------
>
> Key: KAFKA-7324
> URL: https://issues.apache.org/jira/browse/KAFKA-7324
> Project: Kafka
> Issue Type: Bug
> Components: clients
> Affects Versions: 2.1.0
> Reporter: Ron Dagostino
> Assignee: Ron Dagostino
> Priority: Major
> Fix For: 2.1.0
>
>
> When there are no SASL extensions in an OAUTHBEARER request (or the callback
> handler does not support SaslExtensionsCallback) the
> OAuthBearerSaslClient.retrieveCustomExtensions() method returns null. This
> null value is then passed to the OAuthBearerClientInitialResponse
> constructor, and that results in an NPE:
> java.lang.NullPointerException
> at
> org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerClientInitialResponse.validateExtensions(OAuthBearerClientInitialResponse.java:115)
> at
> org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerClientInitialResponse.<init>(OAuthBearerClientInitialResponse.java:81)
> at
> org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerClientInitialResponse.<init>(OAuthBearerClientInitialResponse.java:75)
> at
> org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerSaslClient.evaluateChallenge(OAuthBearerSaslClient.java:99)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
