From b23fc8ff1cd1274996372c36255d2ff50ae10a29 Mon Sep 17 00:00:00 2001 From: Randall Hauch Date: Mon, 8 Jun 2020 10:45:03 -0500 Subject: [PATCH 1/2] =?UTF-8?q?CC-8750:=20Changed=20the=20dialects=20to=20?= =?UTF-8?q?sanitize=20all=20URL=20properties=20that=20case-insensitively?= =?UTF-8?q?=20contain=20=E2=80=9Cpassword=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now sanitizing any URL property that case-insensitively contains (and not just equal to) `password`. Also added a few dialect-specific tests to ensure that different dialects are sanitizing DBMS-specific properties. --- .../jdbc/dialect/GenericDatabaseDialect.java | 2 +- .../dialect/GenericDatabaseDialectTest.java | 30 +++++++++++++++ .../dialect/OracleDatabaseDialectTest.java | 38 +++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java b/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java index ed17d510d..dc9ba0887 100644 --- a/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java +++ b/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java @@ -1747,7 +1747,7 @@ protected String getSqlType(SinkRecordField f) { */ protected String sanitizedUrl(String url) { // Only replace standard URL-type properties ... - return url.replaceAll("(?i)([?&]password=)[^&]*", "$1****"); + return url.replaceAll("(?i)([?&]([^=&]*)password([^=]*)=)[^&]*", "$1****"); } @Override diff --git a/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java b/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java index c7c6dc0cd..a9517b144 100644 --- a/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java +++ b/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java @@ -404,4 +404,34 @@ public void shouldSanitizeUrlWithCredentialsInUrlProperties() { + "user=smith&password=****&other=value" ); } + + @Test + public void shouldSanitizeUrlWithManyPasswordVariationsInUrlProperties() { + assertSanitizedUrl( + "jdbc:acme:db/foo:100?" + + "javax.net.ssl.keyStorePassword=secret2&" + + "password=secret&" + + "key1=value1&" + + "key2=value2&" + + "key3=value3&" + + "passworNotSanitized=not-secret&" + + "passwordShouldBeSanitized=value3&" + + "javax.net.ssl.trustStorePassword=superSecret&" + + "user=smith&" + + "Password=secret&" + + "other=value", + "jdbc:acme:db/foo:100?" + + "javax.net.ssl.keyStorePassword=****&" + + "password=****&" + + "key1=value1&" + + "key2=value2&" + + "key3=value3&" + + "passworNotSanitized=not-secret&" + + "passwordShouldBeSanitized=****&" + + "javax.net.ssl.trustStorePassword=****&" + + "user=smith&" + + "Password=****&" + + "other=value" + ); + } } \ No newline at end of file diff --git a/src/test/java/io/confluent/connect/jdbc/dialect/OracleDatabaseDialectTest.java b/src/test/java/io/confluent/connect/jdbc/dialect/OracleDatabaseDialectTest.java index 647f1a857..0367b2673 100644 --- a/src/test/java/io/confluent/connect/jdbc/dialect/OracleDatabaseDialectTest.java +++ b/src/test/java/io/confluent/connect/jdbc/dialect/OracleDatabaseDialectTest.java @@ -239,4 +239,42 @@ public void shouldSanitizeUrlWithCredentialsInUrlProperties() { + "key2=value2&key3=value3&user=smith&password=****&other=value" ); } + + @Test + public void shouldSanitizeUrlWithKerberosCredentialsInUrlProperties() { + assertSanitizedUrl( + "jdbc:oracle:thin:@myhost:1111/db?" + + "password=secret&" + + "javax.net.ssl.keyStorePassword=secret2&" + + "key1=value1&" + + "key2=value2&" + + "key3=value3&" + + "user=smith&" + + "password=secret&" + + "passworNotSanitized=not-secret&" + + "passwordShouldBeSanitized=value3&" + + "javax.net.ssl.trustStorePassword=superSecret&" + + "OCINewPassword=secret2&" + + "oracle.net.wallet_password=secret3&" + + "proxy_password=secret4&" + + "PROXY_USER_PASSWORD=secret5&" + + "other=value", + "jdbc:oracle:thin:@myhost:1111/db?" + + "password=****&" + + "javax.net.ssl.keyStorePassword=****&" + + "key1=value1&" + + "key2=value2&" + + "key3=value3&" + + "user=smith&" + + "password=****&" + + "passworNotSanitized=not-secret&" + + "passwordShouldBeSanitized=****&" + + "javax.net.ssl.trustStorePassword=****&" + + "OCINewPassword=****&" + + "oracle.net.wallet_password=****&" + + "proxy_password=****&" + + "PROXY_USER_PASSWORD=****&" + + "other=value" + ); + } } \ No newline at end of file From 1715a6efcd2060c7e18072b97247ef1779b5e485 Mon Sep 17 00:00:00 2001 From: Randall Hauch Date: Mon, 8 Jun 2020 18:43:55 -0500 Subject: [PATCH 2/2] CC-8750: Incorporate review feedback --- .../connect/jdbc/dialect/GenericDatabaseDialect.java | 4 +++- .../connect/jdbc/dialect/GenericDatabaseDialectTest.java | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java b/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java index dc9ba0887..ef24f0d57 100644 --- a/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java +++ b/src/main/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialect.java @@ -1742,12 +1742,14 @@ protected String getSqlType(SinkRecordField f) { /** * Return the sanitized form of the supplied JDBC URL, which masks any secrets or credentials. * + *

This implementation replaces the value of all properties that contain {@code password}. + * * @param url the JDBC URL; may not be null * @return the sanitized URL; never null */ protected String sanitizedUrl(String url) { // Only replace standard URL-type properties ... - return url.replaceAll("(?i)([?&]([^=&]*)password([^=]*)=)[^&]*", "$1****"); + return url.replaceAll("(?i)([?&]([^=&]*)password([^=&]*)=)[^&]*", "$1****"); } @Override diff --git a/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java b/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java index a9517b144..603b6f1ee 100644 --- a/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java +++ b/src/test/java/io/confluent/connect/jdbc/dialect/GenericDatabaseDialectTest.java @@ -411,6 +411,7 @@ public void shouldSanitizeUrlWithManyPasswordVariationsInUrlProperties() { "jdbc:acme:db/foo:100?" + "javax.net.ssl.keyStorePassword=secret2&" + "password=secret&" + + "password&" // incorrect parameter before a non-secret + "key1=value1&" + "key2=value2&" + "key3=value3&" @@ -423,6 +424,7 @@ public void shouldSanitizeUrlWithManyPasswordVariationsInUrlProperties() { "jdbc:acme:db/foo:100?" + "javax.net.ssl.keyStorePassword=****&" + "password=****&" + + "password&" + "key1=value1&" + "key2=value2&" + "key3=value3&"