[pulsar] branch branch-2.9 updated: [security] Remove sensitive msg from consumer/producer stats log (#15483)

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mattisonchao pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/branch-2.9 by this push:
     new 6bf9b3130e8 [security] Remove sensitive msg from consumer/producer stats log (#15483)
6bf9b3130e8 is described below

commit 6bf9b3130e8ad44a6de6019a7cbbda33d692a6c2
Author: ZhangJian He <sh...@gmail.com>
AuthorDate: Mon May 9 07:39:31 2022 +0800

    [security] Remove sensitive msg from consumer/producer stats log (#15483)
    
    ### Motivation
    Currently, we are print password field to consumer/producer stats log
    
    ### Modification
    - add missed `@JsonIgnore` on field and getMethod
    - delete unused `withoutAttribute` call
    
    (cherry picked from commit 8b2f3dd095f365fdb22c71078d5a3e0bf6cc9626)
---
 .../client/impl/ConsumerStatsRecorderImpl.java     |  2 +-
 .../client/impl/ProducerStatsRecorderImpl.java     |  2 +-
 .../client/impl/conf/ClientConfigurationData.java  |  9 ++++
 .../impl/conf/ClientConfigurationDataTest.java     | 57 ++++++++++++++++++++++
 4 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerStatsRecorderImpl.java b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerStatsRecorderImpl.java
index fb61a9a8fa3..4fde45bd3b4 100644
--- a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerStatsRecorderImpl.java
+++ b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerStatsRecorderImpl.java
@@ -115,7 +115,7 @@ public class ConsumerStatsRecorderImpl implements ConsumerStatsRecorder {
 
         try {
             log.info("Starting Pulsar consumer status recorder with config: {}", w.writeValueAsString(conf));
-            log.info("Pulsar client config: {}", w.withoutAttribute("authentication").writeValueAsString(pulsarClient.getConfiguration()));
+            log.info("Pulsar client config: {}", w.writeValueAsString(pulsarClient.getConfiguration()));
         } catch (IOException e) {
             log.error("Failed to dump config info", e);
         }
diff --git a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerStatsRecorderImpl.java b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerStatsRecorderImpl.java
index 180d53e4949..3acefa31280 100644
--- a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerStatsRecorderImpl.java
+++ b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerStatsRecorderImpl.java
@@ -99,7 +99,7 @@ public class ProducerStatsRecorderImpl implements ProducerStatsRecorder {
 
         try {
             log.info("Starting Pulsar producer perf with config: {}", w.writeValueAsString(conf));
-            log.info("Pulsar client config: {}", w.withoutAttribute("authentication").writeValueAsString(pulsarClient.getConfiguration()));
+            log.info("Pulsar client config: {}", w.writeValueAsString(pulsarClient.getConfiguration()));
         } catch (IOException e) {
             log.error("Failed to dump config info", e);
         }
diff --git a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java
index 9765cc484cb..093e3e19883 100644
--- a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java
+++ b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java
@@ -30,6 +30,7 @@ import java.util.Optional;
 import java.util.Set;
 import lombok.AllArgsConstructor;
 import lombok.Data;
+import lombok.Getter;
 import lombok.NoArgsConstructor;
 import org.apache.pulsar.client.api.Authentication;
 import org.apache.pulsar.client.api.ProxyProtocol;
@@ -60,6 +61,7 @@ public class ClientConfigurationData implements Serializable, Cloneable {
             value = "The implementation class of ServiceUrlProvider used to generate ServiceUrl."
     )
     @JsonIgnore
+    @Getter(onMethod_ = @JsonIgnore)
     private transient ServiceUrlProvider serviceUrlProvider;
 
     @ApiModelProperty(
@@ -254,6 +256,9 @@ public class ClientConfigurationData implements Serializable, Cloneable {
             name = "tlsTrustStorePassword",
             value = "Password of TLS TrustStore."
     )
+    @Secret
+    @JsonIgnore
+    @Getter(onMethod_ = @JsonIgnore)
     private String tlsTrustStorePassword = null;
 
     @ApiModelProperty(
@@ -312,8 +317,11 @@ public class ClientConfigurationData implements Serializable, Cloneable {
             name = "socks5ProxyUsername",
             value = "Password of SOCKS5 proxy."
     )
+    @Secret
+    @JsonIgnore
     private String socks5ProxyPassword;
 
+    @JsonIgnore
     public Authentication getAuthentication() {
         if (authentication == null) {
             this.authentication = AuthenticationDisabled.INSTANCE;
@@ -369,6 +377,7 @@ public class ClientConfigurationData implements Serializable, Cloneable {
         return Objects.nonNull(socks5ProxyUsername) ? socks5ProxyUsername : System.getProperty("socks5Proxy.username");
     }
 
+    @JsonIgnore
     public String getSocks5ProxyPassword() {
         return Objects.nonNull(socks5ProxyPassword) ? socks5ProxyPassword : System.getProperty("socks5Proxy.password");
     }
diff --git a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/conf/ClientConfigurationDataTest.java b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/conf/ClientConfigurationDataTest.java
new file mode 100644
index 00000000000..b5c30c9a7c6
--- /dev/null
+++ b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/conf/ClientConfigurationDataTest.java
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.client.impl.conf;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectWriter;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import org.apache.pulsar.client.impl.auth.AuthenticationToken;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+/**
+ * Unit test {@link ClientConfigurationData}.
+ */
+public class ClientConfigurationDataTest {
+
+    private final ObjectWriter w;
+
+    {
+        ObjectMapper m = new ObjectMapper();
+        m.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false);
+        w = m.writer();
+    }
+
+
+    @Test
+    public void testDoNotPrintSensitiveInfo() throws JsonProcessingException {
+        ClientConfigurationData clientConfigurationData = new ClientConfigurationData();
+        clientConfigurationData.setTlsTrustStorePassword("xxxx");
+        clientConfigurationData.setSocks5ProxyPassword("yyyy");
+        clientConfigurationData.setAuthentication(new AuthenticationToken("zzzz"));
+        String s = w.writeValueAsString(clientConfigurationData);
+        Assert.assertFalse(s.contains("Password"));
+        Assert.assertFalse(s.contains("xxxx"));
+        Assert.assertFalse(s.contains("yyyy"));
+        Assert.assertFalse(s.contains("zzzz"));
+    }
+
+}