Skip to content

Commit 4cdf60f

Browse files
vietjvietj
authored
Merge pull request #1423 from eclipse-vertx/pendula95-fix-recover-for-ssl-mode-always-4.x
Correct handling of SSL mode always for PostgreSQL
2 parents 3e8e754 + e13e5e8 commit 4cdf60f

File tree

6 files changed

+221
-33
lines changed

6 files changed

+221
-33
lines changed

‎vertx-pg-client/README.adoc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ mvn test -DcontainerFixedPort
307307
You can run tests with an external database:
308308

309309
- the script `src/test/resources/create-postgres.sql` creates the test data
310-
- the `TLSTest` expects the database to be configured with SSL with `src/test/resources/tls/server.key` / `src/test/resources/tls/server.cert``
310+
- the `TLSTest` expects the database to be configured with SSL with `src/test/resources/tls/server.key` / `src/test/resources/tls/server.cert` `src/test/resources/tls/pg_hba.conf` as an example how to force SSL
311311

312312
You need to add some properties for testing:
313313

@@ -317,6 +317,7 @@ You need to add some properties for testing:
317317

318318
- connection.uri(mandatory): configure the client to connect the specified database
319319
- tls.connection.uri(mandatory): configure the client to run `TLSTest` with the specified Postgres with SSL enabled
320+
- tls.force.connection.uri(mandatory): configure the client to run `TLSTest` with the specified Postgres with SSL forced (only option)
320321
- unix.socket.directory(optional): the single unix socket directory(multiple socket directories are not supported) to test Unix domain socket with a specified database, domain socket tests will be skipped if this property is not specified
321322
(Note: Make sure you can access the unix domain socket with this directory under your host machine)
322323
- unix.socket.port(optional): unix socket file is named `.s.PGSQL.nnnn` and `nnnn` is the server's port number,
@@ -335,5 +336,5 @@ Run the Postgres containers with `docker-compose`:
335336
Run tests:
336337

337338
```
338-
> mvn test -Dconnection.uri=postgres://$username:$password@$host:$port/$database -Dtls.connection.uri=postgres://$username:$password@$host:$port/$database -Dunix.socket.directory=$path -Dunix.socket.port=$port
339+
> mvn test -Dconnection.uri=postgres://$username:$password@$host:$port/$database -Dtls.connection.uri=postgres://$username:$password@$host:$port/$database -Dtls.force.connection.uri=postgres://$username:$password@$host:$port/$database -Dunix.socket.directory=$path -Dunix.socket.port=$port
339340
```

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

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -75,21 +75,12 @@ protected Future<Connection> doConnectInternal(SqlConnectOptions options, Contex
7575
} catch (Exception e) {
7676
return context.failedFuture(e);
7777
}
78-
String username = options.getUser();
79-
String password = options.getPassword();
80-
String database = options.getDatabase();
8178
SocketAddress server = options.getSocketAddress();
82-
Map<String, String> properties = options.getProperties() != null ? Collections.unmodifiableMap(options.getProperties()) : null;
83-
return doConnect(server, context, pgOptions).flatMap(conn -> {
84-
PgSocketConnection socket = (PgSocketConnection) conn;
85-
socket.init();
86-
return Future.<Connection>future(p -> socket.sendStartupMessage(username, password, database, properties, p))
87-
.map(conn);
88-
});
79+
return connect(server, context, true, pgOptions);
8980
}
9081

9182
public void cancelRequest(PgConnectOptions options, int processId, int secretKey, Handler<AsyncResult<Void>> handler) {
92-
doConnect(options.getSocketAddress(), vertx.createEventLoopContext(), options).onComplete(ar -> {
83+
connect(options.getSocketAddress(), vertx.createEventLoopContext(), false, options).onComplete(ar -> {
9384
if (ar.succeeded()) {
9485
PgSocketConnection conn = (PgSocketConnection) ar.result();
9586
conn.sendCancelRequestMessage(processId, secretKey, handler);
@@ -99,39 +90,56 @@ public void cancelRequest(PgConnectOptions options, int processId, int secretKey
9990
});
10091
}
10192

102-
private Future<Connection> doConnect(SocketAddress server, ContextInternal context, PgConnectOptions options) {
93+
private Future<Connection> connect(SocketAddress server, ContextInternal context, boolean sendStartupMessage, PgConnectOptions options) {
10394
SslMode sslMode = options.isUsingDomainSocket() ? SslMode.DISABLE : options.getSslMode();
10495
Future<Connection> connFuture;
10596
switch (sslMode) {
10697
case DISABLE:
107-
connFuture = doConnect(server, context, false, options);
98+
connFuture = connect(server, options, context, false, sendStartupMessage);
10899
break;
109100
case ALLOW:
110-
connFuture = doConnect(server, context,false, options).recover(err -> doConnect(server, context,true, options));
101+
connFuture = connect(server, options, context,false, sendStartupMessage).recover(err -> connect(server, options, context, true, sendStartupMessage));
111102
break;
112103
case PREFER:
113-
connFuture = doConnect(server, context,true, options).recover(err -> doConnect(server, context,false, options));
104+
connFuture = connect(server, options, context,true, sendStartupMessage).recover(err -> connect(server, options, context, false, sendStartupMessage));
114105
break;
115106
case REQUIRE:
116107
case VERIFY_CA:
117108
case VERIFY_FULL:
118-
connFuture = doConnect(server, context, true, options);
109+
connFuture = connect(server, options, context, true, sendStartupMessage);
119110
break;
120111
default:
121112
return context.failedFuture(new IllegalArgumentException("Unsupported SSL mode"));
122113
}
123114
return connFuture;
124115
}
125116

126-
private Future<Connection> doConnect(SocketAddress server, ContextInternal context, boolean ssl, PgConnectOptions options) {
117+
private Future<Connection> connect(SocketAddress server, PgConnectOptions connectOptions, ContextInternal context, boolean ssl, boolean sendStartupMessage) {
118+
Future<Connection> res = doConnect(server, connectOptions, context, ssl);
119+
if (sendStartupMessage) {
120+
return res.flatMap(conn -> {
121+
PgSocketConnection socket = (PgSocketConnection) conn;
122+
socket.init();
123+
String username = connectOptions.getUser();
124+
String password = connectOptions.getPassword();
125+
String database = connectOptions.getDatabase();
126+
Map<String, String> properties = connectOptions.getProperties() != null ? Collections.unmodifiableMap(connectOptions.getProperties()) : null;
127+
return Future.future(p -> socket.sendStartupMessage(username, password, database, properties, p));
128+
});
129+
} else {
130+
return res;
131+
}
132+
}
133+
134+
private Future<Connection> doConnect(SocketAddress server, PgConnectOptions connectOptions, ContextInternal context, boolean ssl) {
127135
Future<NetSocket> soFut;
128136
try {
129-
soFut = netClient(options).connect(server, (String) null);
137+
soFut = netClient(connectOptions).connect(server, (String) null);
130138
} catch (Exception e) {
131139
// Client is closed
132140
return context.failedFuture(e);
133141
}
134-
Future<Connection> connFut = soFut.map(so -> newSocketConnection(context, (NetSocketInternal) so, options));
142+
Future<Connection> connFut = soFut.map(so -> newSocketConnection(context, (NetSocketInternal) so, connectOptions));
135143
if (ssl && !server.isDomainSocket()) {
136144
// upgrade connection to SSL if needed
137145
connFut = connFut.flatMap(conn -> Future.future(p -> {

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

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,13 @@
3434
public class TLSTest {
3535

3636
@ClassRule
37-
public static ContainerPgRule rule = new ContainerPgRule().ssl(true);
37+
public static ContainerPgRule ruleOptionalSll = new ContainerPgRule().ssl(true);
38+
39+
@ClassRule
40+
public static ContainerPgRule ruleForceSsl = new ContainerPgRule().ssl(true).forceSsl(true);
41+
42+
@ClassRule
43+
public static ContainerPgRule ruleSllOff = new ContainerPgRule().ssl(false);
3844

3945
private Vertx vertx;
4046

@@ -52,7 +58,7 @@ public void teardown(TestContext ctx) {
5258
public void testTLS(TestContext ctx) {
5359
Async async = ctx.async();
5460

55-
PgConnectOptions options = new PgConnectOptions(rule.options())
61+
PgConnectOptions options = new PgConnectOptions(ruleOptionalSll.options())
5662
.setSslMode(SslMode.REQUIRE)
5763
.setPemTrustOptions(new PemTrustOptions().addCertPath("tls/server.crt"));
5864
PgConnection.connect(vertx, options.setSslMode(SslMode.REQUIRE), ctx.asyncAssertSuccess(conn -> {
@@ -72,7 +78,7 @@ public void testTLS(TestContext ctx) {
7278
@Test
7379
public void testTLSTrustAll(TestContext ctx) {
7480
Async async = ctx.async();
75-
PgConnection.connect(vertx, rule.options().setSslMode(SslMode.REQUIRE).setTrustAll(true), ctx.asyncAssertSuccess(conn -> {
81+
PgConnection.connect(vertx, ruleOptionalSll.options().setSslMode(SslMode.REQUIRE).setTrustAll(true), ctx.asyncAssertSuccess(conn -> {
7682
ctx.assertTrue(conn.isSSL());
7783
async.complete();
7884
}));
@@ -81,7 +87,7 @@ public void testTLSTrustAll(TestContext ctx) {
8187
@Test
8288
public void testTLSInvalidCertificate(TestContext ctx) {
8389
Async async = ctx.async();
84-
PgConnection.connect(vertx, rule.options().setSslMode(SslMode.REQUIRE), ctx.asyncAssertFailure(err -> {
90+
PgConnection.connect(vertx, ruleOptionalSll.options().setSslMode(SslMode.REQUIRE), ctx.asyncAssertFailure(err -> {
8591
// ctx.assertEquals(err.getClass(), VertxException.class);
8692
ctx.assertEquals(err.getMessage(), "SSL handshake failed");
8793
async.complete();
@@ -91,7 +97,7 @@ public void testTLSInvalidCertificate(TestContext ctx) {
9197
@Test
9298
public void testSslModeDisable(TestContext ctx) {
9399
Async async = ctx.async();
94-
PgConnectOptions options = rule.options()
100+
PgConnectOptions options = ruleOptionalSll.options()
95101
.setSslMode(SslMode.DISABLE);
96102
PgConnection.connect(vertx, new PgConnectOptions(options), ctx.asyncAssertSuccess(conn -> {
97103
ctx.assertFalse(conn.isSSL());
@@ -102,18 +108,30 @@ public void testSslModeDisable(TestContext ctx) {
102108
@Test
103109
public void testSslModeAllow(TestContext ctx) {
104110
Async async = ctx.async();
105-
PgConnectOptions options = rule.options()
111+
PgConnectOptions options = ruleOptionalSll.options()
106112
.setSslMode(SslMode.ALLOW);
107113
PgConnection.connect(vertx, new PgConnectOptions(options), ctx.asyncAssertSuccess(conn -> {
108114
ctx.assertFalse(conn.isSSL());
109115
async.complete();
110116
}));
111117
}
112118

119+
@Test
120+
public void testSslModeAllowFallback(TestContext ctx) {
121+
Async async = ctx.async();
122+
PgConnectOptions options = ruleForceSsl.options()
123+
.setSslMode(SslMode.ALLOW)
124+
.setTrustAll(true);
125+
PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
126+
ctx.assertTrue(conn.isSSL());
127+
async.complete();
128+
}));
129+
}
130+
113131
@Test
114132
public void testSslModePrefer(TestContext ctx) {
115133
Async async = ctx.async();
116-
PgConnectOptions options = rule.options()
134+
PgConnectOptions options = ruleOptionalSll.options()
117135
.setSslMode(SslMode.PREFER)
118136
.setTrustAll(true);
119137
PgConnection.connect(vertx, new PgConnectOptions(options), ctx.asyncAssertSuccess(conn -> {
@@ -122,9 +140,21 @@ public void testSslModePrefer(TestContext ctx) {
122140
}));
123141
}
124142

143+
@Test
144+
public void testSslModePreferFallback(TestContext ctx) {
145+
Async async = ctx.async();
146+
PgConnectOptions options = ruleSllOff.options()
147+
.setSslMode(SslMode.PREFER)
148+
.setTrustAll(true);
149+
PgConnection.connect(vertx, options).onComplete(ctx.asyncAssertSuccess(conn -> {
150+
ctx.assertFalse(conn.isSSL());
151+
async.complete();
152+
}));
153+
}
154+
125155
@Test
126156
public void testSslModeVerifyCaConf(TestContext ctx) {
127-
PgConnectOptions options = rule.options()
157+
PgConnectOptions options = ruleOptionalSll.options()
128158
.setSslMode(SslMode.VERIFY_CA)
129159
.setTrustAll(true);
130160
PgConnection.connect(vertx, new PgConnectOptions(options), ctx.asyncAssertFailure(error -> {
@@ -134,8 +164,9 @@ public void testSslModeVerifyCaConf(TestContext ctx) {
134164

135165
@Test
136166
public void testSslModeVerifyFullConf(TestContext ctx) {
137-
PgConnectOptions options = rule.options()
138-
.setSslMode(SslMode.VERIFY_FULL);
167+
PgConnectOptions options = ruleOptionalSll.options()
168+
.setSslMode(SslMode.VERIFY_FULL)
169+
.setTrustOptions(new PemTrustOptions().addCertPath("tls/another.crt"));
139170
PgConnection.connect(vertx, new PgConnectOptions(options), ctx.asyncAssertFailure(error -> {
140171
ctx.assertEquals("Host verification algorithm must be specified under verify-full sslmode", error.getMessage());
141172
}));

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,25 @@ public class ContainerPgRule extends ExternalResource {
3636

3737
private static final String connectionUri = System.getProperty("connection.uri");
3838
private static final String tlsConnectionUri = System.getProperty("tls.connection.uri");
39+
private static final String tlsForceConnectionUri = System.getProperty("tls.force.connection.uri");
3940

4041
private ServerContainer<?> server;
4142
private PgConnectOptions options;
4243
private String databaseVersion;
4344
private boolean ssl;
45+
private boolean forceSsl;
4446
private String user = "postgres";
4547

4648
public ContainerPgRule ssl(boolean ssl) {
4749
this.ssl = ssl;
4850
return this;
4951
}
5052

53+
public ContainerPgRule forceSsl(boolean forceSsl) {
54+
this.forceSsl = forceSsl;
55+
return this;
56+
}
57+
5158
public PgConnectOptions options() {
5259
return new PgConnectOptions(options);
5360
}
@@ -75,6 +82,11 @@ private void initServer(String version) throws Exception {
7582
.withClasspathResourceMapping("tls/server.crt", "/server.crt", BindMode.READ_ONLY)
7683
.withClasspathResourceMapping("tls/server.key", "/server.key", BindMode.READ_ONLY)
7784
.withClasspathResourceMapping("tls/ssl.sh", "/docker-entrypoint-initdb.d/ssl.sh", BindMode.READ_ONLY);
85+
if (forceSsl) {
86+
server
87+
.withClasspathResourceMapping("tls/pg_hba.conf", "/tmp/pg_hba.conf", BindMode.READ_ONLY)
88+
.withClasspathResourceMapping("tls/force_ssl.sh", "/docker-entrypoint-initdb.d/force_ssl.sh", BindMode.READ_ONLY);
89+
}
7890
}
7991
if (System.getProperties().containsKey("containerFixedPort")) {
8092
server.withFixedExposedPort(POSTGRESQL_PORT, POSTGRESQL_PORT);
@@ -84,7 +96,7 @@ private void initServer(String version) throws Exception {
8496
}
8597

8698
public static boolean isTestingWithExternalDatabase() {
87-
return isSystemPropertyValid(connectionUri) || isSystemPropertyValid(tlsConnectionUri);
99+
return isSystemPropertyValid(connectionUri) || isSystemPropertyValid(tlsConnectionUri) || isSystemPropertyValid(tlsForceConnectionUri);
88100
}
89101

90102
private static boolean isSystemPropertyValid(String systemProperty) {
@@ -133,7 +145,11 @@ protected void before() throws Throwable {
133145
if (isTestingWithExternalDatabase()) {
134146

135147
if (ssl) {
136-
options = PgConnectOptions.fromUri(tlsConnectionUri);
148+
if (forceSsl) {
149+
options = PgConnectOptions.fromUri(tlsForceConnectionUri);
150+
} else {
151+
options = PgConnectOptions.fromUri(tlsConnectionUri);
152+
}
137153
}
138154
else {
139155
options = PgConnectOptions.fromUri(connectionUri);

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#!/bin/bash
2+
3+
# force ssl
4+
cp /tmp/pg_hba.conf /var/lib/postgresql/data/pg_hba.conf

0 commit comments

Comments
 (0)