sqoop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From b...@apache.org
Subject sqoop git commit: SQOOP-3257: Sqoop must not log database passwords
Date Wed, 29 Nov 2017 13:48:32 GMT
Repository: sqoop
Updated Branches:
  refs/heads/trunk d96f867bc -> a50869437


SQOOP-3257: Sqoop must not log database passwords

(Szabolcs Vasas via Boglarka Egyed)


Project: http://git-wip-us.apache.org/repos/asf/sqoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/sqoop/commit/a5086943
Tree: http://git-wip-us.apache.org/repos/asf/sqoop/tree/a5086943
Diff: http://git-wip-us.apache.org/repos/asf/sqoop/diff/a5086943

Branch: refs/heads/trunk
Commit: a50869437e29dad59c3e118501879d30ec6e9870
Parents: d96f867
Author: Boglarka Egyed <bogi@apache.org>
Authored: Wed Nov 29 14:44:56 2017 +0100
Committer: Boglarka Egyed <bogi@apache.org>
Committed: Wed Nov 29 14:44:56 2017 +0100

----------------------------------------------------------------------
 src/docs/user/saved-jobs.txt                    |  3 +-
 .../org/apache/sqoop/SqoopJobDataPublisher.java |  3 +-
 src/java/org/apache/sqoop/SqoopOptions.java     |  6 +-
 .../sqoop/metastore/GenericJobStorage.java      |  2 +-
 .../sqoop/metastore/PasswordRedactor.java       | 52 ++++++++++++++++++
 src/java/org/apache/sqoop/tool/JobTool.java     |  7 ++-
 .../credentials/TestPassingSecurePassword.java  |  7 ++-
 .../sqoop/metastore/PasswordRedactorTest.java   | 58 ++++++++++++++++++++
 8 files changed, 127 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/docs/user/saved-jobs.txt
----------------------------------------------------------------------
diff --git a/src/docs/user/saved-jobs.txt b/src/docs/user/saved-jobs.txt
index d21df02..6885079 100644
--- a/src/docs/user/saved-jobs.txt
+++ b/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 = ********
  ...
 ----
 

http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/java/org/apache/sqoop/SqoopJobDataPublisher.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/sqoop/SqoopJobDataPublisher.java b/src/java/org/apache/sqoop/SqoopJobDataPublisher.java
index 93be02f..cee0e7d 100644
--- a/src/java/org/apache/sqoop/SqoopJobDataPublisher.java
+++ b/src/java/org/apache/sqoop/SqoopJobDataPublisher.java
@@ -22,6 +22,7 @@ import org.apache.hadoop.security.UserGroupInformation;
 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 class SqoopJobDataPublisher {
             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);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/java/org/apache/sqoop/SqoopOptions.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/sqoop/SqoopOptions.java b/src/java/org/apache/sqoop/SqoopOptions.java
index 587d4e1..d5fdfba 100644
--- a/src/java/org/apache/sqoop/SqoopOptions.java
+++ b/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 @@ public class SqoopOptions implements Cloneable {
       // 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 @@ public class SqoopOptions implements Cloneable {
     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

http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/java/org/apache/sqoop/metastore/GenericJobStorage.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/sqoop/metastore/GenericJobStorage.java b/src/java/org/apache/sqoop/metastore/GenericJobStorage.java
index 9e1b18b..e4ffde2 100644
--- a/src/java/org/apache/sqoop/metastore/GenericJobStorage.java
+++ b/src/java/org/apache/sqoop/metastore/GenericJobStorage.java
@@ -723,7 +723,7 @@ public class GenericJobStorage extends JobStorage {
   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 {

http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/java/org/apache/sqoop/metastore/PasswordRedactor.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/sqoop/metastore/PasswordRedactor.java b/src/java/org/apache/sqoop/metastore/PasswordRedactor.java
new file mode 100644
index 0000000..9daecd5
--- /dev/null
+++ b/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;
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/java/org/apache/sqoop/tool/JobTool.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/sqoop/tool/JobTool.java b/src/java/org/apache/sqoop/tool/JobTool.java
index dbe8934..5b95c7d 100644
--- a/src/java/org/apache/sqoop/tool/JobTool.java
+++ b/src/java/org/apache/sqoop/tool/JobTool.java
@@ -52,6 +52,7 @@ import com.cloudera.sqoop.metastore.JobData;
 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 @@ public class JobTool extends com.cloudera.sqoop.tool.BaseSqoopTool {
 
     System.out.println("Options:");
     System.out.println("----------------------------");
-    Properties props = childOpts.writeProperties();
-    for (Map.Entry<Object, Object> entry : props.entrySet()) {
-      System.out.println(entry.getKey().toString() + " = " + entry.getValue());
+    Map<String, String> props = PasswordRedactor.redactValues(childOpts.writeProperties());
+    for (Map.Entry<String, String> entry : props.entrySet()) {
+      System.out.println(entry.getKey() + " = " + entry.getValue());
     }
 
     // TODO: This does not show entries in the Configuration

http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java
----------------------------------------------------------------------
diff --git a/src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java b/src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java
index bd911f2..244c744 100644
--- a/src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java
+++ b/src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java
@@ -51,6 +51,7 @@ import java.util.ArrayList;
 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 class TestPassingSecurePassword extends BaseSqoopTestCase {
     // 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 class TestPassingSecurePassword extends BaseSqoopTestCase {
     // 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 class TestPassingSecurePassword extends BaseSqoopTestCase {
     // 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

http://git-wip-us.apache.org/repos/asf/sqoop/blob/a5086943/src/test/org/apache/sqoop/metastore/PasswordRedactorTest.java
----------------------------------------------------------------------
diff --git a/src/test/org/apache/sqoop/metastore/PasswordRedactorTest.java b/src/test/org/apache/sqoop/metastore/PasswordRedactorTest.java
new file mode 100644
index 0000000..a2dbc71
--- /dev/null
+++ b/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));
+    }
+
+}
\ No newline at end of file


Mime
View raw message