[GitHub] [pulsar] codelipenghui commented on a change in pull request #12355: [pulsar-broker] Additional mTLS Logging and Metrics

[GitBox <gi...@apache.org>]

codelipenghui commented on a change in pull request #12355:
URL: https://github.com/apache/pulsar/pull/12355#discussion_r742930955



##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
##########
@@ -36,7 +87,50 @@ public void close() throws IOException {
 
     @Override
     public void initialize(ServiceConfiguration config) throws IOException {
-        // noop
+        this.logEntireCertificateChain = config.isTlsLogEntireCertificateChain();
+
+        this.printWarnOnSelfSignedCertificate = config.isTlsPrintWarnOnSelfSignedCertificate();
+        if(this.printWarnOnSelfSignedCertificate) {
+            LOG.info("Broker will emit warnings when a self-signed client cert is encountered");
+        }
+
+        this.printWarnIfRsaKeySizeLessThanBits = config.getTlsPrintWarnOnRsaKeySizeLessThanBits();
+        if(0 != this.printWarnIfRsaKeySizeLessThanBits) {

Review comment:
       ```suggestion
           if(0 < this.printWarnIfRsaKeySizeLessThanBits) {
   ```

##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1074,6 +1074,52 @@
         doc = "Specify whether Client certificates are required for TLS Reject.\n"
             + "the Connection if the Client Certificate is not trusted")
     private boolean tlsRequireTrustedClientCertOnConnect = false;
+    @FieldContext(
+            category = CATEGORY_TLS,
+            doc = "Print the entire trust chain for the client's certificate in\n"
+                + "the application logs when authenticating\n"
+                + "Default value is 0 (off)")
+    private boolean tlsLogEntireCertificateChain = false;
+    @FieldContext(
+            category = CATEGORY_TLS,
+            doc = "If set to a positive non-zero value, the broker will print a warning in\n"
+                + "the application logs and increment a Prometheus counter if the client's\n"
+                + "certificate is within this many milliseconds of expiration.\n"
+                + "Default value is 0 (off)")
+    private long tlsPrintWarnOnClientCertNearingExpirationMillis = 0;

Review comment:
       Please check the provided value is not a negative value.

##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
##########
@@ -19,15 +19,66 @@
 package org.apache.pulsar.broker.authentication;
 
 import java.io.IOException;
+import java.security.PublicKey;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
+import java.security.interfaces.RSAPublicKey;
+import java.time.*;
+import java.time.temporal.ChronoUnit;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.stream.Collectors;
 
 import javax.naming.AuthenticationException;
+import javax.security.auth.x500.X500Principal;
 
+import io.prometheus.client.Counter;
+import lombok.NonNull;
 import org.apache.pulsar.broker.ServiceConfiguration;
 import org.apache.pulsar.broker.authentication.metrics.AuthenticationMetrics;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class AuthenticationProviderTls implements AuthenticationProvider {
+    private static final Logger LOG = LoggerFactory.getLogger(AuthenticationProviderTls.class);
+
+    static final Counter clientCertSelfSignedMetrics = Counter.build()

Review comment:
       Could you please move to `broker/stats` dir? all of the metrics components are maintained under the package `org.apache.pulsar.broker.stats`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org