Correct handling of SSL mode always for PostgreSQL

vertx-pg-client/src/main/java/io/vertx/pgclient/impl/PgConnectionFactory.java

@@ -70,17 +70,8 @@ protected Future<Connection> doConnectInternal(PgConnectOptions options, Context
     } catch (Exception e) {
       return context.failedFuture(e);
     }
-    String username = options.getUser();
-    String password = options.getPassword();
-    String database = options.getDatabase();
     SocketAddress server = options.getSocketAddress();
-    Map<String, String> properties = options.getProperties() != null ? Collections.unmodifiableMap(options.getProperties()) : null;
-    return doConnect(server, context, options).flatMap(conn -> {
-      PgSocketConnection socket = (PgSocketConnection) conn;
-      socket.init();
-      return Future.<Connection>future(p -> socket.sendStartupMessage(username, password, database, properties, p))
-        .map(conn);
-    });
+    return doConnect(server, context, options);
   }
 
   public void cancelRequest(PgConnectOptions options, int processId, int secretKey, Handler<AsyncResult<Void>> handler) {
@@ -121,6 +112,19 @@ private Future<Connection> doConnect(SocketAddress server, ContextInternal conte
   }
 
   private Future<Connection> doConnect(ConnectOptions connectOptions, ContextInternal context, boolean ssl, PgConnectOptions options) {
+    return doConnect_(connectOptions, context, ssl, options).flatMap(conn -> {
+      String username = options.getUser();
+      String password = options.getPassword();
+      String database = options.getDatabase();
+      Map<String, String> properties = options.getProperties() != null ? Collections.unmodifiableMap(options.getProperties()) : null;
+      PgSocketConnection socket = (PgSocketConnection) conn;
+      socket.init();
+      return Future.<Connection>future(p -> socket.sendStartupMessage(username, password, database, properties, p))
+        .map(conn);
+    });
+  }
+
+  private Future<Connection> doConnect_(ConnectOptions connectOptions, ContextInternal context, boolean ssl, PgConnectOptions options) {
     Future<NetSocket> soFut;
     try {
       soFut = client.connect(connectOptions);

vertx-pg-client/src/test/java/io/vertx/pgclient/TLSTest.java

@@ -35,7 +35,13 @@
 public class TLSTest {
 
   @ClassRule
-  public static ContainerPgRule rule = new ContainerPgRule().ssl(true);
+  public static ContainerPgRule ruleOptionalSll = new ContainerPgRule().ssl(true);
+
+  @ClassRule
+  public static ContainerPgRule ruleForceSsl = new ContainerPgRule().ssl(true).forceSsl(true);
+
+  @ClassRule
+  public static ContainerPgRule ruleSllOff = new ContainerPgRule().ssl(false);
 
   private Vertx vertx;
 
@@ -53,7 +59,7 @@ public void teardown(TestContext ctx) {
   public void testTLS(TestContext ctx) {
     Async async = ctx.async();
 
-    PgConnectOptions options = new PgConnectOptions(rule.options())
+    PgConnectOptions options = new PgConnectOptions(ruleOptionalSll.options())
       .setSslMode(SslMode.REQUIRE)
       .setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/server.crt")));
     PgConnection.connect(vertx, options.setSslMode(SslMode.REQUIRE)).onComplete(ctx.asyncAssertSuccess(conn -> {
@@ -74,7 +80,7 @@ public void testTLS(TestContext ctx) {
   @Test
   public void testTLSTrustAll(TestContext ctx) {
     Async async = ctx.async();
-    PgConnection.connect(vertx, rule.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustAll(true))).onComplete(ctx.asyncAssertSuccess(conn -> {
+    PgConnection.connect(vertx, ruleOptionalSll.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustAll(true))).onComplete(ctx.asyncAssertSuccess(conn -> {
       ctx.assertTrue(conn.isSSL());
       async.complete();
     }));
@@ -83,7 +89,7 @@ public void testTLSTrustAll(TestContext ctx) {
   @Test
   public void testTLSInvalidCertificate(TestContext ctx) {
     Async async = ctx.async();
-    PgConnection.connect(vertx, rule.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/another.crt")))).onComplete(ctx.asyncAssertFailure(err -> {
+    PgConnection.connect(vertx, ruleOptionalSll.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/another.crt")))).onComplete(ctx.asyncAssertFailure(err -> {
 //      ctx.assertEquals(err.getClass(), VertxException.class);
       ctx.assertEquals(err.getMessage(), "SSL handshake failed");
       async.complete();
@@ -93,7 +99,7 @@ public void testTLSInvalidCertificate(TestContext ctx) {
   @Test
   public void testSslModeDisable(TestContext ctx) {
     Async async = ctx.async();
-    PgConnectOptions options = rule.options()
+    PgConnectOptions options = ruleOptionalSll.options()
       .setSslMode(SslMode.DISABLE);
     PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
       ctx.assertFalse(conn.isSSL());
@@ -104,18 +110,30 @@ public void testSslModeDisable(TestContext ctx) {
   @Test
   public void testSslModeAllow(TestContext ctx) {
     Async async = ctx.async();
-    PgConnectOptions options = rule.options()
+    PgConnectOptions options = ruleOptionalSll.options()
       .setSslMode(SslMode.ALLOW);
     PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
       ctx.assertFalse(conn.isSSL());
       async.complete();
     }));
   }
 
+  @Test
+  public void testSslModeAllowFallback(TestContext ctx) {
+    Async async = ctx.async();
+    PgConnectOptions options = ruleForceSsl.options()
+      .setSslMode(SslMode.ALLOW)
+      .setSslOptions(new ClientSSLOptions().setTrustAll(true));
+    PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
+      ctx.assertFalse(conn.isSSL());
+      async.complete();
+    }));
+  }
+
   @Test
   public void testSslModePrefer(TestContext ctx) {
     Async async = ctx.async();
-    PgConnectOptions options = rule.options()
+    PgConnectOptions options = ruleOptionalSll.options()
       .setSslMode(SslMode.PREFER)
       .setSslOptions(new ClientSSLOptions().setTrustAll(true));
     PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
@@ -124,9 +142,21 @@ public void testSslModePrefer(TestContext ctx) {
     }));
   }
 
+  @Test
+  public void testSslModePreferFallback(TestContext ctx) {
+    Async async = ctx.async();
+    PgConnectOptions options = ruleSllOff.options()
+      .setSslMode(SslMode.PREFER)
+      .setSslOptions(new ClientSSLOptions().setTrustAll(true));
+    PgConnection.connect(vertx, options).onComplete(ctx.asyncAssertSuccess(conn -> {
+      ctx.assertTrue(conn.isSSL());
+      async.complete();
+    }));
+  }
+
   @Test
   public void testSslModeVerifyCaConf(TestContext ctx) {
-    PgConnectOptions options = rule.options()
+    PgConnectOptions options = ruleOptionalSll.options()
       .setSslMode(SslMode.VERIFY_CA)
       .setSslOptions(new ClientSSLOptions().setTrustAll(true));
     PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertFailure(error -> {
@@ -136,7 +166,7 @@ public void testSslModeVerifyCaConf(TestContext ctx) {
 
   @Test
   public void testSslModeVerifyFullConf(TestContext ctx) {
-    PgConnectOptions options = rule.options()
+    PgConnectOptions options = ruleOptionalSll.options()
       .setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/another.crt")))
       .setSslMode(SslMode.VERIFY_FULL);
     PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertFailure(error -> {

vertx-pg-client/src/test/java/io/vertx/pgclient/junit/ContainerPgRule.java

@@ -36,18 +36,25 @@ public class ContainerPgRule extends ExternalResource {
 
   private static final String connectionUri = System.getProperty("connection.uri");
   private static final String tlsConnectionUri = System.getProperty("tls.connection.uri");
+  private static final String tlsForceConnectionUri = System.getProperty("tls.force.connection.uri");
 
   private ServerContainer<?> server;
   private PgConnectOptions options;
   private String databaseVersion;
   private boolean ssl;
+  private boolean forceSsl;
   private String user = "postgres";
 
   public ContainerPgRule ssl(boolean ssl) {
     this.ssl = ssl;
     return this;
   }
 
+  public ContainerPgRule forceSsl(boolean forceSsl) {
+    this.forceSsl = forceSsl;
+    return this;
+  }
+
   public PgConnectOptions options() {
     return new PgConnectOptions(options);
   }
@@ -75,6 +82,11 @@ private void initServer(String version) throws Exception {
         .withClasspathResourceMapping("tls/server.crt", "/server.crt", BindMode.READ_ONLY)
         .withClasspathResourceMapping("tls/server.key", "/server.key", BindMode.READ_ONLY)
         .withClasspathResourceMapping("tls/ssl.sh", "/docker-entrypoint-initdb.d/ssl.sh", BindMode.READ_ONLY);
+        if (forceSsl) {
+          server
+            .withClasspathResourceMapping("tls/pg_hba.conf", "/tmp/pg_hba.conf", BindMode.READ_ONLY)
+            .withClasspathResourceMapping("tls/force_ssl.sh", "/docker-entrypoint-initdb.d/force_ssl.sh", BindMode.READ_ONLY);
+        }
     }
     if (System.getProperties().containsKey("containerFixedPort")) {
       server.withFixedExposedPort(POSTGRESQL_PORT, POSTGRESQL_PORT);
@@ -84,7 +96,7 @@ private void initServer(String version) throws Exception {
   }
 
   public static boolean isTestingWithExternalDatabase() {
-    return isSystemPropertyValid(connectionUri) || isSystemPropertyValid(tlsConnectionUri);
+    return isSystemPropertyValid(connectionUri) || isSystemPropertyValid(tlsConnectionUri) || isSystemPropertyValid(tlsForceConnectionUri);
   }
 
   private static boolean isSystemPropertyValid(String systemProperty) {
@@ -133,7 +145,11 @@ protected void before() throws Throwable {
     if (isTestingWithExternalDatabase()) {
 
       if (ssl) {
-        options =  PgConnectOptions.fromUri(tlsConnectionUri);
+        if (forceSsl) {
+          options = PgConnectOptions.fromUri(tlsForceConnectionUri);
+        } else {
+          options = PgConnectOptions.fromUri(tlsConnectionUri);
+        }
       }
       else {
         options = PgConnectOptions.fromUri(connectionUri);

vertx-pg-client/src/test/resources/tls/force_ssl.sh

@@ -0,0 +1,4 @@
+#!/bin/bash
+
+# force ssl
+cp /tmp/pg_hba.conf /var/lib/postgresql/data/pg_hba.conf

vertx-pg-client/src/test/resources/tls/pg_hba.conf

@@ -0,0 +1,128 @@
+# PostgreSQL Client Authentication Configuration File
+# ===================================================
+#
+# Refer to the "Client Authentication" section in the PostgreSQL
+# documentation for a complete description of this file.  A short
+# synopsis follows.
+#
+# ----------------------
+# Authentication Records
+# ----------------------
+#
+# This file controls: which hosts are allowed to connect, how clients
+# are authenticated, which PostgreSQL user names they can use, which
+# databases they can access.  Records take one of these forms:
+#
+# local         DATABASE  USER  METHOD  [OPTIONS]
+# host          DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
+# hostssl       DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
+# hostnossl     DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
+# hostgssenc    DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
+# hostnogssenc  DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
+#
+# (The uppercase items must be replaced by actual values.)
+#
+# The first field is the connection type:
+# - "local" is a Unix-domain socket
+# - "host" is a TCP/IP socket (encrypted or not)
+# - "hostssl" is a TCP/IP socket that is SSL-encrypted
+# - "hostnossl" is a TCP/IP socket that is not SSL-encrypted
+# - "hostgssenc" is a TCP/IP socket that is GSSAPI-encrypted
+# - "hostnogssenc" is a TCP/IP socket that is not GSSAPI-encrypted
+#
+# DATABASE can be "all", "sameuser", "samerole", "replication", a
+# database name, a regular expression (if it starts with a slash (/))
+# or a comma-separated list thereof.  The "all" keyword does not match
+# "replication".  Access to replication must be enabled in a separate
+# record (see example below).
+#
+# USER can be "all", a user name, a group name prefixed with "+", a
+# regular expression (if it starts with a slash (/)) or a comma-separated
+# list thereof.  In both the DATABASE and USER fields you can also write
+# a file name prefixed with "@" to include names from a separate file.
+#
+# ADDRESS specifies the set of hosts the record matches.  It can be a
+# host name, or it is made up of an IP address and a CIDR mask that is
+# an integer (between 0 and 32 (IPv4) or 128 (IPv6) inclusive) that
+# specifies the number of significant bits in the mask.  A host name
+# that starts with a dot (.) matches a suffix of the actual host name.
+# Alternatively, you can write an IP address and netmask in separate
+# columns to specify the set of hosts.  Instead of a CIDR-address, you
+# can write "samehost" to match any of the server's own IP addresses,
+# or "samenet" to match any address in any subnet that the server is
+# directly connected to.
+#
+# METHOD can be "trust", "reject", "md5", "password", "scram-sha-256",
+# "gss", "sspi", "ident", "peer", "pam", "ldap", "radius" or "cert".
+# Note that "password" sends passwords in clear text; "md5" or
+# "scram-sha-256" are preferred since they send encrypted passwords.
+#
+# OPTIONS are a set of options for the authentication in the format
+# NAME=VALUE.  The available options depend on the different
+# authentication methods -- refer to the "Client Authentication"
+# section in the documentation for a list of which options are
+# available for which authentication methods.
+#
+# Database and user names containing spaces, commas, quotes and other
+# special characters must be quoted.  Quoting one of the keywords
+# "all", "sameuser", "samerole" or "replication" makes the name lose
+# its special character, and just match a database or username with
+# that name.
+#
+# ---------------
+# Include Records
+# ---------------
+#
+# This file allows the inclusion of external files or directories holding
+# more records, using the following keywords:
+#
+# include           FILE
+# include_if_exists FILE
+# include_dir       DIRECTORY
+#
+# FILE is the file name to include, and DIR is the directory name containing
+# the file(s) to include.  Any file in a directory will be loaded if suffixed
+# with ".conf".  The files of a directory are ordered by name.
+# include_if_exists ignores missing files.  FILE and DIRECTORY can be
+# specified as a relative or an absolute path, and can be double-quoted if
+# they contain spaces.
+#
+# -------------
+# Miscellaneous
+# -------------
+#
+# This file is read on server startup and when the server receives a
+# SIGHUP signal.  If you edit the file on a running system, you have to
+# SIGHUP the server for the changes to take effect, run "pg_ctl reload",
+# or execute "SELECT pg_reload_conf()".
+#
+# ----------------------------------
+# Put your actual configuration here
+# ----------------------------------
+#
+# If you want to allow non-local connections, you need to add more
+# "host" records.  In that case you will also need to make PostgreSQL
+# listen on a non-local interface via the listen_addresses
+# configuration parameter, or via the -i or -h command line switches.
+
+# CAUTION: Configuring the system for local "trust" authentication
+# allows any local user to connect as any PostgreSQL user, including
+# the database superuser.  If you do not trust all your local users,
+# use another authentication method.
+
+
+# TYPE  DATABASE        USER            ADDRESS                 METHOD
+
+# "local" is for Unix domain socket connections only
+local   all             all                                     trust
+# IPv4 local connections:
+hostssl    all             all             127.0.0.1/32            trust
+# IPv6 local connections:
+hostssl    all             all             ::1/128                 trust
+# Allow replication connections from localhost, by a user with the
+# replication privilege.
+local   replication     all                                     trust
+host    replication     all             127.0.0.1/32            trust
+host    replication     all             ::1/128                 trust
+
+hostssl all all all md5