SQOOP-3257: Sqoop must not log database passwords

(Szabolcs Vasas via Boglarka Egyed)

src/docs/user/saved-jobs.txt

@@ -90,7 +90,7 @@ Available jobs:
   myjob
 ----
 
-We can inspect the configuration of a job with the +show+ action:
+We can inspect the configuration of a job with the +show+ action. As you can see in the below example even if the password is stored in the metastore the +show+ action will redact its value in the output:
 
 ----
  $ sqoop job --show myjob
@@ -102,6 +102,7 @@ We can inspect the configuration of a job with the +show+ action:
  codegen.input.delimiters.record = 0
  hdfs.append.dir = false
  db.table = mytable
+ db.password = ********
  ...
 ----
 

src/java/org/apache/sqoop/SqoopJobDataPublisher.java

@@ -22,6 +22,7 @@
 import org.apache.sqoop.mapreduce.ExportJobBase;
 import org.apache.sqoop.mapreduce.ImportJobBase;
 import org.apache.sqoop.mapreduce.hcat.SqoopHCatUtilities;
+import org.apache.sqoop.metastore.PasswordRedactor;
 
 import java.util.Properties;
 
@@ -145,7 +146,7 @@ public String toString() {
             return  "Operation=" + operation + ", Url=" + url + ", User=" + user + ", StoreType=" + storeType
                     + ", StoreTable=" + storeTable + ", StoreQuery=" + storeQuery + ", HiveDB=" + hiveDB
                     + ", HiveTable=" + hiveTable + ", StartTime=" + startTime + ", EndTime=" + endTime
-                    + ", CmdLineArgs=" + commandLineOpts;
+                    + ", CmdLineArgs=" + PasswordRedactor.redactValues(commandLineOpts);
         }
     }
 

src/java/org/apache/sqoop/SqoopOptions.java

@@ -81,6 +81,8 @@ public class SqoopOptions implements Cloneable {
   public static final String DEF_HCAT_HOME_OLD = "/usr/lib/hcatalog";
 
   public static final boolean METASTORE_PASSWORD_DEFAULT = false;
+  public static final String DB_PASSWORD_KEY = "db.password";
+
   /**
    * Thrown when invalid cmdline options are given.
    */
@@ -748,7 +750,7 @@ private void loadPasswordProperty(Properties props) {
       // Require that the user enter it now.
       setPasswordFromConsole();
     } else {
-      this.password = props.getProperty("db.password", this.password);
+      this.password = props.getProperty(DB_PASSWORD_KEY, this.password);
     }
   }
 
@@ -832,7 +834,7 @@ private void writePasswordProperty(Properties props) {
     if (this.getConf().getBoolean(
       METASTORE_PASSWORD_KEY, METASTORE_PASSWORD_DEFAULT)) {
       // If the user specifies, we may store the password in the metastore.
-      putProperty(props, "db.password", this.password);
+      putProperty(props, DB_PASSWORD_KEY, this.password);
       putProperty(props, "db.require.password", "false");
     } else if (this.password != null) {
       // Otherwise, if the user has set a password, we just record

src/java/org/apache/sqoop/metastore/GenericJobStorage.java

@@ -723,7 +723,7 @@ private void checkForOldRootProperties() throws SQLException {
   private void setV0Property(String jobName, String propClass,
       String propName, String propVal) throws SQLException {
     LOG.debug("Job: " + jobName + "; Setting property "
-        + propName + " with class " + propClass + " => " + propVal);
+        + propName + " with class " + propClass + " => " + PasswordRedactor.redactValue(propName, propVal));
 
     PreparedStatement s = null;
     try {

src/java/org/apache/sqoop/metastore/PasswordRedactor.java

@@ -0,0 +1,52 @@
+/**
+ * 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.sqoop.metastore;
+
+import static org.apache.sqoop.SqoopOptions.DB_PASSWORD_KEY;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+public class PasswordRedactor {
+
+    static final String REDACTED_PASSWORD_STRING = "********";
+
+    private static final Collection<String> REDACTION_KEYS = Arrays.asList(DB_PASSWORD_KEY);
+
+    public static String redactValue(String key, String value) {
+        if (REDACTION_KEYS.contains(key)) {
+            return REDACTED_PASSWORD_STRING;
+        } else {
+            return value;
+        }
+    }
+
+    public static Map<String, String> redactValues(Map<?, ?> values) {
+        Map<String, String> result = new HashMap<>();
+
+        for (Map.Entry<?, ?> entry : values.entrySet()) {
+            String key = entry.getKey().toString();
+            result.put(key, redactValue(key, entry.getValue().toString()));
+        }
+
+        return result;
+    }
+
+}

src/java/org/apache/sqoop/tool/JobTool.java

@@ -52,6 +52,7 @@
 import com.cloudera.sqoop.metastore.JobStorage;
 import com.cloudera.sqoop.metastore.JobStorageFactory;
 import org.apache.sqoop.manager.JdbcDrivers;
+import org.apache.sqoop.metastore.PasswordRedactor;
 import org.apache.sqoop.util.LoggingUtils;
 
 /**
@@ -270,9 +271,9 @@ private int showJob(SqoopOptions opts) throws IOException {
 
     System.out.println("Options:");
     System.out.println("----------------------------");
-    Properties props = childOpts.writeProperties();
+    Map<String, String> props = PasswordRedactor.redactValues(childOpts.writeProperties());
-    for (Map.Entry<Object, Object> entry : props.entrySet()) {
+    for (Map.Entry<String, String> entry : props.entrySet()) {
-      System.out.println(entry.getKey().toString() + " = " + entry.getValue());
+      System.out.println(entry.getKey() + " = " + entry.getValue());
     }
 
     // TODO: This does not show entries in the Configuration

src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java

@@ -51,6 +51,7 @@
 import java.util.Collections;
 import java.util.Properties;
 
+import static org.apache.sqoop.SqoopOptions.DB_PASSWORD_KEY;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -242,7 +243,7 @@ public void testPasswordInMetastoreWithRecordEnabledAndSecureOption()
     // this is what is used to record password into the metastore
     Properties propertiesIntoMetastore = out.writeProperties();
 
-    assertNull(propertiesIntoMetastore.getProperty("db.password"));
+    assertNull(propertiesIntoMetastore.getProperty(DB_PASSWORD_KEY));
     // password-file should NOT be null as it'll be sued to retrieve password
     assertNotNull(propertiesIntoMetastore.getProperty("db.password.file"));
 
@@ -279,7 +280,7 @@ public void testPasswordInMetastoreWithRecordDisabledAndSecureOption()
     // this is what is used to record password into the metastore
     Properties propertiesIntoMetastore = out.writeProperties();
 
-    assertNull(propertiesIntoMetastore.getProperty("db.password"));
+    assertNull(propertiesIntoMetastore.getProperty(DB_PASSWORD_KEY));
     assertNotNull(propertiesIntoMetastore.getProperty("db.password.file"));
 
     // load the saved properties and verify
@@ -312,7 +313,7 @@ public void testPasswordInMetastoreWithRecordEnabledAndNonSecureOption()
     // this is what is used to record password into the metastore
     Properties propertiesIntoMetastore = out.writeProperties();
 
-    assertNotNull(propertiesIntoMetastore.getProperty("db.password"));
+    assertNotNull(propertiesIntoMetastore.getProperty(DB_PASSWORD_KEY));
     assertNull(propertiesIntoMetastore.getProperty("db.password.file"));
 
     // load the saved properties and verify

src/test/org/apache/sqoop/metastore/PasswordRedactorTest.java

@@ -0,0 +1,58 @@
+/**
+ * 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.sqoop.metastore;
+
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.sqoop.SqoopOptions.DB_PASSWORD_KEY;
+import static org.apache.sqoop.metastore.PasswordRedactor.REDACTED_PASSWORD_STRING;
+import static org.junit.Assert.assertEquals;
+
+public class PasswordRedactorTest {
+
+    @Test
+    public void testRedactValueWithPasswordFieldReturnsRedactedValue() {
+        assertEquals(REDACTED_PASSWORD_STRING, PasswordRedactor.redactValue(DB_PASSWORD_KEY, "secret"));
+    }
+
+    @Test
+    public void testRedactValueWithNonPasswordFieldReturnsInputValue() {
+        String nonPasswordFieldKey = "non_password_field";
+        String inputValue = "not_a_secret";
+        assertEquals(inputValue, PasswordRedactor.redactValue(nonPasswordFieldKey, inputValue));
+    }
+
+    @Test
+    public void testRedactValuesRedactsPasswordFieldAndDoesNotChangeTheOthers() {
+        String nonPasswordFieldKey = "non_password_field";
+        String nonPasswordFieldValue = "non_password_value";
+        Map<String, String> input = new HashMap<>();
+        input.put(nonPasswordFieldKey, nonPasswordFieldValue);
+        input.put(DB_PASSWORD_KEY, "secret");
+
+        Map<String, String> expected = new HashMap<>();
+        expected.put(nonPasswordFieldKey, nonPasswordFieldValue);
+        expected.put(DB_PASSWORD_KEY, REDACTED_PASSWORD_STRING);
+
+        assertEquals(expected, PasswordRedactor.redactValues(input));
+    }
+
+}