You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by rg...@apache.org on 2023/05/09 08:02:55 UTC
[pulsar] branch branch-2.11 updated: [fix][broker] Fix the reason label of authentication metrics (#20030)
This is an automated email from the ASF dual-hosted git repository.
rgao pushed a commit to branch branch-2.11
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.11 by this push:
new 2ab530c6d91 [fix][broker] Fix the reason label of authentication metrics (#20030)
2ab530c6d91 is described below
commit 2ab530c6d91765abaa6a48c3c382415db14d0eb5
Author: ran <rg...@streamnative.io>
AuthorDate: Fri May 5 08:48:59 2023 +0800
[fix][broker] Fix the reason label of authentication metrics (#20030)
(cherry picked from commit 2b515ffb389c4b4fe3cb5a9c5f3d7eb0a4c9ef99)
---
.../AuthenticationProviderAthenz.java | 20 ++++++++++++--
.../authentication/AuthenticationProvider.java | 5 ++++
.../AuthenticationProviderBasic.java | 21 ++++++++++++---
.../authentication/AuthenticationProviderList.java | 16 +++++++----
.../authentication/AuthenticationProviderTls.java | 12 +++++++--
.../AuthenticationProviderToken.java | 31 ++++++++++++++--------
.../metrics/AuthenticationMetrics.java | 16 +++++++++++
.../pulsar/broker/stats/PrometheusMetricsTest.java | 6 ++---
8 files changed, 100 insertions(+), 27 deletions(-)
diff --git a/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java b/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java
index 4a82e549623..5dfeaba9ad7 100644
--- a/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java
+++ b/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java
@@ -43,6 +43,15 @@ public class AuthenticationProviderAthenz implements AuthenticationProvider {
private List<String> domainNameList = null;
private int allowedOffset = 30;
+ public enum ErrorCode {
+ UNKNOWN,
+ NO_CLIENT,
+ NO_TOKEN,
+ NO_PUBLIC_KEY,
+ DOMAIN_MISMATCH,
+ INVALID_TOKEN,
+ }
+
@Override
public void initialize(ServiceConfiguration config) throws IOException {
String domainNames;
@@ -81,11 +90,13 @@ public class AuthenticationProviderAthenz implements AuthenticationProvider {
public String authenticate(AuthenticationDataSource authData) throws AuthenticationException {
SocketAddress clientAddress;
String roleToken;
+ ErrorCode errorCode = ErrorCode.UNKNOWN;
try {
if (authData.hasDataFromPeer()) {
clientAddress = authData.getPeerAddress();
} else {
+ errorCode = ErrorCode.NO_CLIENT;
throw new AuthenticationException("Authentication data source does not have a client address");
}
@@ -94,13 +105,16 @@ public class AuthenticationProviderAthenz implements AuthenticationProvider {
} else if (authData.hasDataFromHttp()) {
roleToken = authData.getHttpHeader(AuthZpeClient.ZPE_TOKEN_HDR);
} else {
+ errorCode = ErrorCode.NO_TOKEN;
throw new AuthenticationException("Authentication data source does not have a role token");
}
if (roleToken == null) {
+ errorCode = ErrorCode.NO_TOKEN;
throw new AuthenticationException("Athenz token is null, can't authenticate");
}
if (roleToken.isEmpty()) {
+ errorCode = ErrorCode.NO_TOKEN;
throw new AuthenticationException("Athenz RoleToken is empty, Server is Using Athenz Authentication");
}
if (log.isDebugEnabled()) {
@@ -110,6 +124,7 @@ public class AuthenticationProviderAthenz implements AuthenticationProvider {
RoleToken token = new RoleToken(roleToken);
if (!domainNameList.contains(token.getDomain())) {
+ errorCode = ErrorCode.DOMAIN_MISMATCH;
throw new AuthenticationException(
String.format("Athenz RoleToken Domain mismatch, Expected: %s, Found: %s",
domainNameList.toString(), token.getDomain()));
@@ -120,6 +135,7 @@ public class AuthenticationProviderAthenz implements AuthenticationProvider {
PublicKey ztsPublicKey = AuthZpeClient.getZtsPublicKey(token.getKeyId());
if (ztsPublicKey == null) {
+ errorCode = ErrorCode.NO_PUBLIC_KEY;
throw new AuthenticationException("Unable to retrieve ZTS Public Key");
}
@@ -128,13 +144,13 @@ public class AuthenticationProviderAthenz implements AuthenticationProvider {
AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(), getAuthMethodName());
return token.getPrincipal();
} else {
+ errorCode = ErrorCode.INVALID_TOKEN;
throw new AuthenticationException(
String.format("Athenz Role Token Not Authenticated from Client: %s", clientAddress));
}
}
} catch (AuthenticationException exception) {
- AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(), getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(errorCode);
throw exception;
}
}
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java
index 840b5cb057d..5f9eae71d56 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java
@@ -27,6 +27,7 @@ import javax.net.ssl.SSLSession;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.pulsar.broker.ServiceConfiguration;
+import org.apache.pulsar.broker.authentication.metrics.AuthenticationMetrics;
import org.apache.pulsar.common.api.AuthData;
import org.apache.pulsar.common.classification.InterfaceStability;
import org.apache.pulsar.common.util.FutureUtil;
@@ -135,6 +136,10 @@ public interface AuthenticationProvider extends Closeable {
}
}
+ default void incrementFailureMetric(Enum<?> errorCode) {
+ AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(), getAuthMethodName(), errorCode);
+ }
+
/**
* Set response, according to passed in request.
* and return whether we should do following chain.doFilter or not.
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
index 9f6bacf7298..bc8ed563d73 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
@@ -47,6 +47,14 @@ public class AuthenticationProviderBasic implements AuthenticationProvider {
private static final String CONF_PULSAR_PROPERTY_KEY = "basicAuthConf";
private Map<String, String> users;
+ private enum ErrorCode {
+ UNKNOWN,
+ EMPTY_AUTH_DATA,
+ INVALID_HEADER,
+ INVALID_AUTH_DATA,
+ INVALID_TOKEN,
+ }
+
@Override
public void close() throws IOException {
// noop
@@ -105,9 +113,10 @@ public class AuthenticationProviderBasic implements AuthenticationProvider {
String userId = authParams.getUserId();
String password = authParams.getPassword();
String msg = "Unknown user or invalid password";
-
+ ErrorCode errorCode = ErrorCode.UNKNOWN;
try {
if (users.get(userId) == null) {
+ errorCode = ErrorCode.INVALID_AUTH_DATA;
throw new AuthenticationException(msg);
}
@@ -118,15 +127,16 @@ public class AuthenticationProviderBasic implements AuthenticationProvider {
List<String> splitEncryptedPassword = Arrays.asList(encryptedPassword.split("\\$"));
if (splitEncryptedPassword.size() != 4 || !encryptedPassword
.equals(Md5Crypt.apr1Crypt(password.getBytes(), splitEncryptedPassword.get(2)))) {
+ errorCode = ErrorCode.INVALID_TOKEN;
throw new AuthenticationException(msg);
}
// For crypt algorithm
} else if (!encryptedPassword.equals(Crypt.crypt(password.getBytes(), encryptedPassword.substring(0, 2)))) {
+ errorCode = ErrorCode.INVALID_TOKEN;
throw new AuthenticationException(msg);
}
} catch (AuthenticationException exception) {
- AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(), getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(errorCode);
throw exception;
}
AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(), getAuthMethodName());
@@ -145,24 +155,29 @@ public class AuthenticationProviderBasic implements AuthenticationProvider {
String rawAuthToken = authData.getHttpHeader(HTTP_HEADER_NAME);
// parsing and validation
if (StringUtils.isBlank(rawAuthToken) || !rawAuthToken.toUpperCase().startsWith("BASIC ")) {
+ incrementFailureMetric(ErrorCode.INVALID_HEADER);
throw new AuthenticationException("Authentication token has to be started with \"Basic \"");
}
String[] splitRawAuthToken = rawAuthToken.split(" ");
if (splitRawAuthToken.length != 2) {
+ incrementFailureMetric(ErrorCode.INVALID_HEADER);
throw new AuthenticationException("Base64 encoded token is not found");
}
try {
authParams = new String(Base64.getDecoder().decode(splitRawAuthToken[1]));
} catch (Exception e) {
+ incrementFailureMetric(ErrorCode.INVALID_HEADER);
throw new AuthenticationException("Base64 decoding is failure: " + e.getMessage());
}
} else {
+ incrementFailureMetric(ErrorCode.EMPTY_AUTH_DATA);
throw new AuthenticationException("Authentication data source does not have data");
}
String[] parsedAuthParams = authParams.split(":");
if (parsedAuthParams.length != 2) {
+ incrementFailureMetric(ErrorCode.INVALID_AUTH_DATA);
throw new AuthenticationException("Base64 decoded params are invalid");
}
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java
index f921a6f1e09..c74d3edb531 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java
@@ -43,9 +43,15 @@ public class AuthenticationProviderList implements AuthenticationProvider {
}
+ private enum ErrorCode {
+ UNKNOWN,
+ AUTH_REQUIRED,
+ }
+
static <T, W> T applyAuthProcessor(List<W> processors, AuthProcessor<T, W> authFunc)
throws AuthenticationException {
AuthenticationException authenticationException = null;
+ String errorCode = ErrorCode.UNKNOWN.name();
for (W ap : processors) {
try {
return authFunc.apply(ap);
@@ -55,19 +61,19 @@ public class AuthenticationProviderList implements AuthenticationProvider {
}
// Store the exception so we can throw it later instead of a generic one
authenticationException = ae;
+ errorCode = ap.getClass().getSimpleName() + "-INVALID-AUTH";
}
}
if (null == authenticationException) {
AuthenticationMetrics.authenticateFailure(
AuthenticationProviderList.class.getSimpleName(),
- "authentication-provider-list", "Authentication required");
+ "authentication-provider-list", ErrorCode.AUTH_REQUIRED);
throw new AuthenticationException("Authentication required");
} else {
- AuthenticationMetrics.authenticateFailure(AuthenticationProviderList.class.getSimpleName(),
- "authentication-provider-list",
- authenticationException.getMessage() != null
- ? authenticationException.getMessage() : "Authentication required");
+ AuthenticationMetrics.authenticateFailure(
+ AuthenticationProviderList.class.getSimpleName(),
+ "authentication-provider-list", errorCode);
throw authenticationException;
}
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
index 4cfcad6a686..7fb93b2e623 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
@@ -27,6 +27,12 @@ import org.apache.pulsar.broker.authentication.metrics.AuthenticationMetrics;
public class AuthenticationProviderTls implements AuthenticationProvider {
+ private enum ErrorCode {
+ UNKNOWN,
+ INVALID_CERTS,
+ INVALID_CN, // invalid common name
+ }
+
@Override
public void close() throws IOException {
// noop
@@ -45,6 +51,7 @@ public class AuthenticationProviderTls implements AuthenticationProvider {
@Override
public String authenticate(AuthenticationDataSource authData) throws AuthenticationException {
String commonName = null;
+ ErrorCode errorCode = ErrorCode.UNKNOWN;
try {
if (authData.hasDataFromTls()) {
/**
@@ -72,6 +79,7 @@ public class AuthenticationProviderTls implements AuthenticationProvider {
// CN=Steve Kille,O=Isode Limited,C=GB
Certificate[] certs = authData.getTlsCertificates();
if (null == certs) {
+ errorCode = ErrorCode.INVALID_CERTS;
throw new AuthenticationException("Failed to get TLS certificates from client");
}
String distinguishedName = ((X509Certificate) certs[0]).getSubjectX500Principal().getName();
@@ -85,12 +93,12 @@ public class AuthenticationProviderTls implements AuthenticationProvider {
}
if (commonName == null) {
+ errorCode = ErrorCode.INVALID_CN;
throw new AuthenticationException("Client unable to authenticate with TLS certificate");
}
AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(), getAuthMethodName());
} catch (AuthenticationException exception) {
- AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(), getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(errorCode);
throw exception;
}
return commonName;
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
index 9cc220aa8c7..ebb28dcd26f 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
@@ -99,6 +99,12 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
private String confTokenAudienceClaimSettingName;
private String confTokenAudienceSettingName;
+ public enum ErrorCode {
+ INVALID_AUTH_DATA,
+ INVALID_TOKEN,
+ INVALID_AUDIENCES,
+ }
+
@Override
public void close() throws IOException {
// noop
@@ -145,19 +151,18 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
@Override
public String authenticate(AuthenticationDataSource authData) throws AuthenticationException {
+ String token;
try {
// Get Token
- String token;
token = getToken(authData);
- // Parse Token by validating
- String role = getPrincipal(authenticateToken(token));
- AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(), getAuthMethodName());
- return role;
} catch (AuthenticationException exception) {
- AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(), getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(ErrorCode.INVALID_AUTH_DATA);
throw exception;
}
+ // Parse Token by validating
+ String role = getPrincipal(authenticateToken(token));
+ AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(), getAuthMethodName());
+ return role;
}
@Override
@@ -214,16 +219,19 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
List<String> audiences = (List<String>) object;
// audience not contains this broker, throw exception.
if (audiences.stream().noneMatch(audienceInToken -> audienceInToken.equals(audience))) {
- throw new AuthenticationException("Audiences in token: [" + String.join(", ", audiences)
- + "] not contains this broker: " + audience);
+ incrementFailureMetric(ErrorCode.INVALID_AUDIENCES);
+ throw new AuthenticationException("Audiences in token: ["
+ + String.join(", ", audiences) + "] not contains this broker: " + audience);
}
} else if (object instanceof String) {
if (!object.equals(audience)) {
- throw new AuthenticationException("Audiences in token: [" + object
- + "] not contains this broker: " + audience);
+ incrementFailureMetric(ErrorCode.INVALID_AUDIENCES);
+ throw new AuthenticationException(
+ "Audiences in token: [" + object + "] not contains this broker: " + audience);
}
} else {
// should not reach here.
+ incrementFailureMetric(ErrorCode.INVALID_AUDIENCES);
throw new AuthenticationException("Audiences in token is not in expected format: " + object);
}
}
@@ -237,6 +245,7 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
if (e instanceof ExpiredJwtException) {
expiredTokenMetrics.inc();
}
+ incrementFailureMetric(ErrorCode.INVALID_TOKEN);
throw new AuthenticationException("Failed to authentication token: " + e.getMessage());
}
}
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java
index b4ae44e200d..00cb01ed08a 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java
@@ -43,11 +43,27 @@ public class AuthenticationMetrics {
/**
* Log authenticate failure event to the authentication metrics.
+ *
+ * This method is deprecated due to the label "reason" is a potential infinite value.
+ * @deprecated See {@link #authenticateFailure(String, String, Enum)} ()}
+ *
* @param providerName The short class name of the provider
* @param authMethod Authentication method name.
* @param reason Failure reason.
*/
+ @Deprecated
public static void authenticateFailure(String providerName, String authMethod, String reason) {
authFailuresMetrics.labels(providerName, authMethod, reason).inc();
}
+
+ /**
+ * Log authenticate failure event to the authentication metrics.
+ * @param providerName The short class name of the provider
+ * @param authMethod Authentication method name.
+ * @param errorCode Error code.
+ */
+ public static void authenticateFailure(String providerName, String authMethod, Enum<?> errorCode) {
+ authFailuresMetrics.labels(providerName, authMethod, errorCode.name()).inc();
+ }
+
}
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java
index 2cbe7486d56..615aaac293d 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java
@@ -1159,15 +1159,12 @@ public class PrometheusMetricsTest extends BrokerTestBase {
conf.setProperties(properties);
provider.initialize(conf);
- String authExceptionMessage = "";
-
try {
provider.authenticate(new AuthenticationDataSource() {
});
fail("Should have failed");
} catch (AuthenticationException e) {
// expected, no credential passed
- authExceptionMessage = e.getMessage();
}
String token = AuthTokenUtils.createToken(secretKey, "subject", Optional.empty());
@@ -1204,7 +1201,8 @@ public class PrometheusMetricsTest extends BrokerTestBase {
boolean haveFailed = false;
for (Metric metric : cm) {
if (Objects.equals(metric.tags.get("auth_method"), "token")
- && Objects.equals(metric.tags.get("reason"), authExceptionMessage)
+ && Objects.equals(metric.tags.get("reason"),
+ AuthenticationProviderToken.ErrorCode.INVALID_AUTH_DATA.name())
&& Objects.equals(metric.tags.get("provider_name"), provider.getClass().getSimpleName())) {
haveFailed = true;
}