Repository: pgjdbc/pgjdbc (java)

Issue:
Version from 42.1.0 can't connect to 127.0.0.1 with System SOCKS proxy
Hi,
I found Postgresql JDBC Driver cannot connect to local postgresql server when I set a system SOCKS proxy.

org.postgresql.util.PSQLException: The connection attempt failed.
	at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:275)
	at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:49)
	at org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:194)
	at org.postgresql.Driver.makeConnection(Driver.java:450)
	at org.postgresql.Driver.connect(Driver.java:252)
	at java.sql.DriverManager.getConnection(DriverManager.java:664)
	at java.sql.DriverManager.getConnection(DriverManager.java:208)
	at org.apache.ibatis.datasource.unpooled.UnpooledDataSource.doGetConnection(UnpooledDataSource.java:201)
	at org.apache.ibatis.datasource.unpooled.UnpooledDataSource.doGetConnection(UnpooledDataSource.java:196)
	at org.apache.ibatis.datasource.unpooled.UnpooledDataSource.getConnection(UnpooledDataSource.java:93)
	at org.apache.ibatis.datasource.pooled.PooledDataSource.popConnection(PooledDataSource.java:404)
.......

Caused by: java.net.UnknownHostException: 127.0.0.1
	at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:184)
	at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
	at java.net.Socket.connect(Socket.java:589)
	at org.postgresql.core.PGStream.<init>(PGStream.java:68)
	at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:144)
	... 69 more

system:  macOS High Sierra
java: 1.8.0_144


Pull request description:
Added support for socksNonProxyHosts property
Fixes #975

When a `socksProxyHost` is configured, there needs to be an escape valve so that some hosts are resolved immediately. The Java Networking spec[1] does not specify such an option, but `socksNonProxyHosts` is used in the `sun.net.spi.DefaultProxySelector`. The behavior is mentioned in bug report 5001942[2]. 

This commit uses the `DefaultProxySelector.select` method to determine if the host should be resolved immediately, or deferred and resolved through the SOCKS proxy. This provides the best level of parity with the JVM internals.

[1] http://docs.oracle.com/javase/8/docs/technotes/guides/net/proxies.html
[2] http://bugs.java.com/bugdatabase/view_bug.do?bug_id=5001942

Patch:
diff --git a/pgjdbc/src/main/java/org/postgresql/core/PGStream.java b/pgjdbc/src/main/java/org/postgresql/core/PGStream.java
index f9a3e4ab..c65bb18d 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/PGStream.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/PGStream.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, PostgreSQL Global Development Group
+ * Copyright (c) 2017, PostgreSQL Global Development Group
  * See the LICENSE file in the project root for more information.
  */
 
@@ -21,7 +21,6 @@ import java.io.Writer;
 import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.sql.SQLException;
-
 import javax.net.SocketFactory;
 
 /**
@@ -63,7 +62,7 @@ public class PGStream implements Closeable {
       // When using a SOCKS proxy, the host might not be resolvable locally,
       // thus we defer resolution until the traffic reaches the proxy. If there
       // is no proxy, we must resolve the host to an IP to connect the socket.
-      InetSocketAddress address = System.getProperty("socksProxyHost") == null
+      InetSocketAddress address = hostSpec.shouldResolve()
           ? new InetSocketAddress(hostSpec.getHost(), hostSpec.getPort())
           : InetSocketAddress.createUnresolved(hostSpec.getHost(), hostSpec.getPort());
       socket.connect(address, timeout);
diff --git a/pgjdbc/src/main/java/org/postgresql/ssl/jdbc4/LibPQFactory.java b/pgjdbc/src/main/java/org/postgresql/ssl/jdbc4/LibPQFactory.java
index a1292012..32c40d4c 100644
--- a/pgjdbc/src/main/java/org/postgresql/ssl/jdbc4/LibPQFactory.java
+++ b/pgjdbc/src/main/java/org/postgresql/ssl/jdbc4/LibPQFactory.java
@@ -24,7 +24,10 @@ import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateFactory;
+import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
+import java.util.Collection;
+import java.util.List;
 import java.util.Properties;
 
 import javax.naming.InvalidNameException;
@@ -48,6 +51,8 @@ import javax.security.auth.x500.X500Principal;
  */
 public class LibPQFactory extends WrappedFactory implements HostnameVerifier {
 
+  private static final int ALT_DNS_NAME = 2;
+
   LazyKeyManager km = null;
   String sslmode;
 
@@ -229,6 +234,39 @@ public class LibPQFactory extends WrappedFactory implements HostnameVerifier {
     }
   }
 
+  public static boolean verifyHostName(String hostname, String pattern) {
+    if (hostname == null || pattern == null) {
+      return false;
+    }
+    if (!pattern.startsWith("*")) {
+      // No wildcard => just compare hostnames
+      return hostname.equalsIgnoreCase(pattern);
+    }
+    // pattern starts with *, so hostname should be at least (pattern.length-1) long
+    if (hostname.length() < pattern.length() - 1) {
+      return false;
+    }
+    // Compare ignore case
+    final boolean ignoreCase = true;
+    // Below code is "hostname.endsWithIgnoreCase(pattern.withoutFirstStar())"
+
+    // E.g. hostname==sub.host.com; pattern==*.host.com
+    // We need to start the offset of ".host.com" in hostname
+    // For this we take hostname.length() - pattern.length()
+    // and +1 is required since pattern is known to start with *
+    int toffset = hostname.length() - pattern.length() + 1;
+
+    // Wildcard covers just one domain level
+    // a.b.c.com should not be covered by *.c.com
+    if (hostname.lastIndexOf('.', toffset - 1) >= 0) {
+      // If there's a dot in between 0..toffset
+      return false;
+    }
+
+    return hostname.regionMatches(ignoreCase, toffset,
+        pattern, 1, pattern.length() - 1);
+  }
+
   /**
    * Verifies the server certificate according to the libpq rules. The cn attribute of the
    * certificate is matched against the hostname. If the cn attribute starts with an asterisk (*),
@@ -252,6 +290,26 @@ public class LibPQFactory extends WrappedFactory implements HostnameVerifier {
     }
     // Extract the common name
     X509Certificate serverCert = peerCerts[0];
+
+    try {
+      // Check for Subject Alternative Names (see RFC 6125)
+      Collection<List<?>> subjectAltNames = serverCert.getSubjectAlternativeNames();
+
+      if (subjectAltNames != null) {
+        for (List<?> sanit : subjectAltNames) {
+          Integer type = (Integer) sanit.get(0);
+          String san = (String) sanit.get(1);
+
+          // this mimics libpq check for ALT_DNS_NAME
+          if (type != null && type == ALT_DNS_NAME && verifyHostName(hostname, san)) {
+            return true;
+          }
+        }
+      }
+    } catch (CertificateParsingException e) {
+      return false;
+    }
+
     LdapName DN;
     try {
       DN = new LdapName(serverCert.getSubjectX500Principal().getName(X500Principal.RFC2253));
@@ -266,17 +324,6 @@ public class LibPQFactory extends WrappedFactory implements HostnameVerifier {
         break;
       }
     }
-    if (CN == null) {
-      return false;
-    } else if (CN.startsWith("*")) { // We have a wildcard
-      if (hostname.endsWith(CN.substring(1))) {
-        // Avoid IndexOutOfBoundsException because hostname already ends with CN
-        return !(hostname.substring(0, hostname.length() - CN.length() + 1).contains("."));
-      } else {
-        return false;
-      }
-    } else {
-      return CN.equals(hostname);
-    }
+    return verifyHostName(hostname, CN);
   }
 }
diff --git a/pgjdbc/src/main/java/org/postgresql/util/HostSpec.java b/pgjdbc/src/main/java/org/postgresql/util/HostSpec.java
index 064dd03c..6869b311 100644
--- a/pgjdbc/src/main/java/org/postgresql/util/HostSpec.java
+++ b/pgjdbc/src/main/java/org/postgresql/util/HostSpec.java
@@ -5,10 +5,17 @@
 
 package org.postgresql.util;
 
+import static java.util.regex.Pattern.compile;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * Simple container for host and port.
  */
 public class HostSpec {
+  public static String DEFAULT_NON_PROXY_HOSTS = "localhost|127.*|[::1]|0.0.0.0|[::0]";
+
   protected final String host;
   protected final int port;
 
@@ -39,4 +46,50 @@ public class HostSpec {
   public int hashCode() {
     return port ^ host.hashCode();
   }
+
+  public Boolean shouldResolve() {
+    if (System.getProperty("socksProxyHost") == null) {
+      return true;
+    }
+    return matchesNonProxyHosts();
+  }
+
+  private Boolean matchesNonProxyHosts() {
+    String nonProxyHosts = System.getProperty("socksNonProxyHosts", DEFAULT_NON_PROXY_HOSTS);
+    if (nonProxyHosts == null || this.host.isEmpty()) {
+      return false;
+    }
+
+    Pattern pattern = toPattern(nonProxyHosts);
+    Matcher matcher = pattern == null ? null : pattern.matcher(this.host);
+    return matcher != null && matcher.matches();
+  }
+
+  private Pattern toPattern(String mask) {
+    StringBuilder joiner = new StringBuilder();
+    String separator = "";
+    for (String disjunct : mask.split("\\|")) {
+      if (!disjunct.isEmpty()) {
+        String regex = disjunctToRegex(disjunct.toLowerCase());
+        joiner.append(separator).append(regex);
+        separator = "|";
+      }
+    }
+
+    return joiner.length() == 0 ? null : compile(joiner.toString());
+  }
+
+  private String disjunctToRegex(String disjunct) {
+    String regex;
+
+    if (disjunct.startsWith("*")) {
+      regex = ".*" + Pattern.quote(disjunct.substring(1));
+    } else if (disjunct.endsWith("*")) {
+      regex = Pattern.quote(disjunct.substring(0, disjunct.length() - 1)) + ".*";
+    } else {
+      regex = Pattern.quote(disjunct);
+    }
+
+    return regex;
+  }
 }


Test patch:
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc4/Jdbc4TestSuite.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc4/Jdbc4TestSuite.java
index 405d977f..46fcc4bb 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc4/Jdbc4TestSuite.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc4/Jdbc4TestSuite.java
@@ -24,6 +24,7 @@ import org.junit.runners.Suite;
         BinaryStreamTest.class,
         CharacterStreamTest.class,
         UUIDTest.class,
+        LibPQFactoryHostNameTest.class,
         XmlTest.class
 })
 public class Jdbc4TestSuite {
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc4/LibPQFactoryHostNameTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc4/LibPQFactoryHostNameTest.java
new file mode 100644
index 00000000..1ee140c4
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc4/LibPQFactoryHostNameTest.java
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2017, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.jdbc4;
+
+import org.postgresql.ssl.jdbc4.LibPQFactory;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Arrays;
+
+@RunWith(Parameterized.class)
+public class LibPQFactoryHostNameTest {
+
+  private final String hostname;
+  private final String pattern;
+  private final boolean expected;
+
+  public LibPQFactoryHostNameTest(String hostname, String pattern, boolean expected) {
+    this.hostname = hostname;
+    this.pattern = pattern;
+    this.expected = expected;
+  }
+
+  @Parameterized.Parameters(name = "host={0}, pattern={1}")
+  public static Iterable<Object[]> data() {
+    return Arrays.asList(new Object[][]{
+        {"host.com", "pattern.com", false},
+        {"host.com", ".pattern.com", false},
+        {"host.com", "*.pattern.com", false},
+        {"host.com", "*.host.com", false},
+        {"a.com", "*.host.com", false},
+        {".a.com", "*.host.com", false},
+        {"longhostname.com", "*.com", true},
+        {"longhostname.ru", "*.com", false},
+        {"host.com", "host.com", true},
+        {"sub.host.com", "host.com", false},
+        {"sub.host.com", "sub.host.com", true},
+        {"sub.host.com", "*.host.com", true},
+        {"Sub.host.com", "sub.host.com", true},
+        {"sub.host.com", "Sub.host.com", true},
+        {"sub.host.com", "*.hoSt.com", true},
+        {"*.host.com", "host.com", false},
+        {"sub.sub.host.com", "*.host.com", false}, // Wildcard should cover just one level
+    });
+  }
+
+  @Test
+  public void checkPattern() throws Exception {
+    Assert.assertEquals(expected, LibPQFactory.verifyHostName(hostname, pattern));
+  }
+}
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jre8/core/Jre8TestSuite.java b/pgjdbc/src/test/java/org/postgresql/test/jre8/core/Jre8TestSuite.java
new file mode 100644
index 00000000..96cee652
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/jre8/core/Jre8TestSuite.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) 2017, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.jre8.core;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+/**
+ * @author Joe Kutner on 10/24/17.
+ *         Twitter: @codefinger
+ */
+@RunWith(Suite.class)
+@Suite.SuiteClasses({SocksProxyTest.class})
+public class Jre8TestSuite {
+}
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jre8/core/SocksProxyTest.java b/pgjdbc/src/test/java/org/postgresql/test/jre8/core/SocksProxyTest.java
new file mode 100644
index 00000000..8d01f780
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/jre8/core/SocksProxyTest.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2017, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.jre8.core;
+
+import static org.junit.Assert.assertNotNull;
+
+import org.postgresql.test.TestUtil;
+
+import org.junit.After;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+
+/**
+ * @author Joe Kutner on 10/9/17.
+ *         Twitter: @codefinger
+ */
+public class SocksProxyTest {
+
+  @After
+  public void cleanup() {
+    System.clearProperty("socksProxyHost");
+    System.clearProperty("socksProxyPort");
+    System.clearProperty("socksNonProxyHosts");
+  }
+
+  /**
+   * Tests the connect method by connecting to the test database
+   */
+  @Test
+  public void testConnectWithSocksNonProxyHost() throws Exception {
+    System.setProperty("socksProxyHost", "fake-socks-proxy");
+    System.setProperty("socksProxyPort", "9999");
+    System.setProperty("socksNonProxyHosts", TestUtil.getServer());
+
+    TestUtil.initDriver(); // Set up log levels, etc.
+
+    Connection con =
+        DriverManager.getConnection(TestUtil.getURL(), TestUtil.getUser(), TestUtil.getPassword());
+
+    assertNotNull(con);
+    con.close();
+  }
+}
diff --git a/pgjdbc/src/test/java/org/postgresql/test/util/HostSpecTest.java b/pgjdbc/src/test/java/org/postgresql/test/util/HostSpecTest.java
new file mode 100644
index 00000000..32acc31f
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/util/HostSpecTest.java
@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2017, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.util;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.postgresql.util.HostSpec;
+
+import org.junit.After;
+import org.junit.Test;
+
+/**
+ * @author Joe Kutner on 10/19/17.
+ *         Twitter: @codefinger
+ */
+public class HostSpecTest {
+
+  @After
+  public void cleanup() {
+    System.clearProperty("socksProxyHost");
+    System.clearProperty("socksProxyPort");
+    System.clearProperty("socksNonProxyHosts");
+  }
+
+  @Test
+  public void testShouldResolve() throws Exception {
+    HostSpec hostSpec = new HostSpec("localhost", 5432);
+    assertTrue(hostSpec.shouldResolve());
+  }
+
+  @Test
+  public void testShouldResolveWithSocksProxyHost() throws Exception {
+    System.setProperty("socksProxyHost", "fake-socks-proxy");
+    HostSpec hostSpec = new HostSpec("example.com", 5432);
+    assertFalse(hostSpec.shouldResolve());
+  }
+
+  @Test
+  public void testShouldResolveWithSocksProxyHostWithLocalhost() throws Exception {
+    System.setProperty("socksProxyHost", "fake-socks-proxy");
+    HostSpec hostSpec = new HostSpec("localhost", 5432);
+    assertTrue(hostSpec.shouldResolve());
+  }
+
+  @Test
+  public void testShouldResolveWithSocksNonProxyHost() throws Exception {
+    System.setProperty("socksProxyHost", "fake-socks-proxy");
+    System.setProperty("socksNonProxyHosts", "example.com");
+    HostSpec hostSpec = new HostSpec("example.com", 5432);
+    assertTrue(hostSpec.shouldResolve());
+  }
+
+  @Test
+  public void testShouldResolveWithSocksNonProxyHosts() throws Exception {
+    System.setProperty("socksProxyHost", "fake-socks-proxy");
+    System.setProperty("socksNonProxyHosts", "example.com|localhost");
+    HostSpec hostSpec = new HostSpec("example.com", 5432);
+    assertTrue(hostSpec.shouldResolve());
+  }
+
+  @Test
+  public void testShouldResolveWithSocksNonProxyHostsNotMatching() throws Exception {
+    System.setProperty("socksProxyHost", "fake-socks-proxy");
+    System.setProperty("socksNonProxyHosts", "example.com|localhost");
+    HostSpec hostSpec = new HostSpec("example.org", 5432);
+    assertFalse(hostSpec.shouldResolve());
+  }
+}