bookkeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From si...@apache.org
Subject [bookkeeper] branch master updated: Dont log ledgermetadata which contains password.
Date Thu, 01 Feb 2018 19:19:37 GMT
This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 67018bd  Dont log ledgermetadata which contains password.
67018bd is described below

commit 67018bd4b1e006f4d957a1f70a1c0e01b9ebb70b
Author: cguttapalem <cguttapalem@salesforce.com>
AuthorDate: Thu Feb 1 11:19:29 2018 -0800

    Dont log ledgermetadata which contains password.
    
    Descriptions of the changes in this PR:
    
    Dont log ledgermetadata contains password used to create
    the ledger. Logs are not supposed to reveal such sensitive info.
    
    Author: cguttapalem <cguttapalem@salesforce.com>
    
    Reviewers: Ivan Kelly <ivank@apache.org>, Jia Zhai <None>, Sijie Guo <sijie@apache.org>
    
    This closes #1092 from reddycharan/fixpasswordlog
---
 .../apache/bookkeeper/client/LedgerMetadata.java   | 33 ++++++++++++++++++++--
 .../bookkeeper/client/ReadOnlyLedgerHandle.java    |  2 +-
 .../bookkeeper/client/LedgerMetadataTest.java      | 19 ++++++++++++-
 3 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
index d37e7fc..753eebd 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
@@ -327,6 +327,10 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
     }
 
     LedgerMetadataFormat buildProtoFormat() {
+        return buildProtoFormat(true);
+    }
+
+    LedgerMetadataFormat buildProtoFormat(boolean withPassword) {
         LedgerMetadataFormat.Builder builder = LedgerMetadataFormat.newBuilder();
         builder.setQuorumSize(writeQuorumSize).setAckQuorumSize(ackQuorumSize)
             .setEnsembleSize(ensembleSize).setLength(length)
@@ -337,7 +341,10 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
         }
 
         if (hasPassword) {
-            builder.setDigestType(digestType).setPassword(ByteString.copyFrom(password));
+            builder.setDigestType(digestType);
+            if (withPassword) {
+                builder.setPassword(ByteString.copyFrom(password));
+            }
         }
 
         if (customMetadata != null) {
@@ -366,13 +373,17 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
      * @return the metadata serialized into a byte array
      */
     public byte[] serialize() {
+        return serialize(true);
+    }
+
+    public byte[] serialize(boolean withPassword) {
         if (metadataFormatVersion == 1) {
             return serializeVersion1();
         }
 
         StringBuilder s = new StringBuilder();
         s.append(VERSION_KEY).append(tSplitter).append(CURRENT_METADATA_FORMAT_VERSION).append(lSplitter);
-        s.append(TextFormat.printToString(buildProtoFormat()));
+        s.append(TextFormat.printToString(buildProtoFormat(withPassword)));
         if (LOG.isDebugEnabled()) {
             LOG.debug("Serialized config: {}", s);
         }
@@ -671,8 +682,24 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
 
     @Override
     public String toString() {
+        return toStringRepresentation(true);
+    }
+
+    /**
+     * Returns a string representation of this LedgerMetadata object by
+     * filtering out the password field.
+     *
+     * @return a string representation of the object without password field in
+     *         it.
+     */
+    public String toSafeString() {
+        return toStringRepresentation(false);
+    }
+
+    private String toStringRepresentation(boolean withPassword) {
         StringBuilder sb = new StringBuilder();
-        sb.append("(meta:").append(new String(serialize(), UTF_8)).append(", version:").append(version).append(")");
+        sb.append("(meta:").append(new String(serialize(withPassword), UTF_8)).append(",
version:").append(version)
+                .append(")");
         return sb.toString();
     }
 
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java
index c940d27..d1ab615 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java
@@ -55,7 +55,7 @@ class ReadOnlyLedgerHandle extends LedgerHandle implements LedgerMetadataListene
             Version.Occurred occurred =
                     ReadOnlyLedgerHandle.this.metadata.getVersion().compare(this.m.getVersion());
             if (Version.Occurred.BEFORE == occurred) {
-                LOG.info("Updated ledger metadata for ledger {} to {}.", ledgerId, this.m);
+                LOG.info("Updated ledger metadata for ledger {} to {}.", ledgerId, this.m.toSafeString());
                 synchronized (ReadOnlyLedgerHandle.this) {
                     if (this.m.isClosed()) {
                             ReadOnlyLedgerHandle.this.lastAddConfirmed = this.m.getLastEntryId();
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java
index 9570c58..dc947dd 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java
@@ -36,7 +36,8 @@ import org.junit.Test;
  */
 public class LedgerMetadataTest {
 
-    private static final byte[] passwd = "testPasswd".getBytes(UTF_8);
+    private static final String passwdStr = "testPasswd";
+    private static final byte[] passwd = passwdStr.getBytes(UTF_8);
 
     @Test
     public void testGetters() {
@@ -154,4 +155,20 @@ public class LedgerMetadataTest {
         assertTrue(lm1.isConflictWith(lm2));
     }
 
+    @Test
+    public void testToString() {
+        LedgerMetadata lm1 = new LedgerMetadata(
+            3,
+            3,
+            2,
+            DigestType.CRC32,
+            passwd,
+            Collections.emptyMap(),
+            true);
+
+        assertTrue("toString should contain 'password' field", lm1.toString().contains("password"));
+        assertTrue("toString should contain password value", lm1.toString().contains(passwdStr));
+        assertFalse("toSafeString should not contain 'password' field", lm1.toSafeString().contains("password"));
+        assertFalse("toSafeString should not contain password value", lm1.toSafeString().contains(passwdStr));
+    }
 }

-- 
To stop receiving notification emails like this one, please contact
sijie@apache.org.

Mime
View raw message