Skip to content

Commit 5ab12ad

Browse files
vietjvietj
authored
Merge pull request #1413 from eclipse-vertx/pendula95-fix-recover-for-ssl-mode-always
Correct handling of SSL mode always for PostgreSQL
2 parents 6218246 + 88e8209 commit 5ab12ad

File tree

8 files changed

+226
-45
lines changed

8 files changed

+226
-45
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: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -70,56 +70,61 @@ protected Future<Connection> doConnectInternal(PgConnectOptions options, Context
7070
} catch (Exception e) {
7171
return context.failedFuture(e);
7272
}
73-
String username = options.getUser();
74-
String password = options.getPassword();
75-
String database = options.getDatabase();
7673
SocketAddress server = options.getSocketAddress();
77-
Map<String, String> properties = options.getProperties() != null ? Collections.unmodifiableMap(options.getProperties()) : null;
78-
return doConnect(server, context, options).flatMap(conn -> {
79-
PgSocketConnection socket = (PgSocketConnection) conn;
80-
socket.init();
81-
return Future.<Connection>future(p -> socket.sendStartupMessage(username, password, database, properties, p))
82-
.map(conn);
83-
});
74+
return connect(server, context, true, options);
8475
}
8576

86-
public void cancelRequest(PgConnectOptions options, int processId, int secretKey, Handler<AsyncResult<Void>> handler) {
87-
doConnect(options.getSocketAddress(), vertx.createEventLoopContext(), options).onComplete(ar -> {
88-
if (ar.succeeded()) {
89-
PgSocketConnection conn = (PgSocketConnection) ar.result();
90-
conn.sendCancelRequestMessage(processId, secretKey, handler);
91-
} else {
92-
handler.handle(Future.failedFuture(ar.cause()));
93-
}
94-
});
77+
public Future<Void> cancelRequest(PgConnectOptions options, int processId, int secretKey) {
78+
return connect(options.getSocketAddress(), vertx.createEventLoopContext(), false, options)
79+
.compose(conn -> {
80+
PgSocketConnection socket = (PgSocketConnection) conn;
81+
return socket.sendCancelRequestMessage(processId, secretKey);
82+
});
9583
}
9684

97-
private Future<Connection> doConnect(SocketAddress server, ContextInternal context, PgConnectOptions options) {
85+
private Future<Connection> connect(SocketAddress server, ContextInternal context, boolean sendStartupMessage, PgConnectOptions options) {
9886
SslMode sslMode = options.isUsingDomainSocket() ? SslMode.DISABLE : options.getSslMode();
9987
ConnectOptions connectOptions = new ConnectOptions()
10088
.setRemoteAddress(server);
10189
Future<Connection> connFuture;
10290
switch (sslMode) {
10391
case DISABLE:
104-
connFuture = doConnect(connectOptions, context, false, options);
92+
connFuture = connect(connectOptions, context, false, sendStartupMessage, options);
10593
break;
10694
case ALLOW:
107-
connFuture = doConnect(connectOptions, context, false, options).recover(err -> doConnect(connectOptions, context, true, options));
95+
connFuture = connect(connectOptions, context, false, sendStartupMessage, options).recover(err -> connect(connectOptions, context, true, sendStartupMessage, options));
10896
break;
10997
case PREFER:
110-
connFuture = doConnect(connectOptions, context, true, options).recover(err -> doConnect(connectOptions, context, false, options));
98+
connFuture = connect(connectOptions, context, true, sendStartupMessage, options).recover(err -> connect(connectOptions, context, false, sendStartupMessage, options));
11199
break;
112100
case REQUIRE:
113101
case VERIFY_CA:
114102
case VERIFY_FULL:
115-
connFuture = doConnect(connectOptions, context, true, options);
103+
connFuture = connect(connectOptions, context, true, sendStartupMessage, options);
116104
break;
117105
default:
118106
return context.failedFuture(new IllegalArgumentException("Unsupported SSL mode"));
119107
}
120108
return connFuture;
121109
}
122110

111+
private Future<Connection> connect(ConnectOptions connectOptions, ContextInternal context, boolean ssl, boolean sendStartupMessage, PgConnectOptions options) {
112+
Future<Connection> res = doConnect(connectOptions, context, ssl, options);
113+
if (sendStartupMessage) {
114+
return res.flatMap(conn -> {
115+
PgSocketConnection socket = (PgSocketConnection) conn;
116+
socket.init();
117+
String username = options.getUser();
118+
String password = options.getPassword();
119+
String database = options.getDatabase();
120+
Map<String, String> properties = options.getProperties() != null ? Collections.unmodifiableMap(options.getProperties()) : null;
121+
return socket.sendStartupMessage(username, password, database, properties);
122+
});
123+
} else {
124+
return res;
125+
}
126+
}
127+
123128
private Future<Connection> doConnect(ConnectOptions connectOptions, ContextInternal context, boolean ssl, PgConnectOptions options) {
124129
Future<NetSocket> soFut;
125130
try {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public Future<Void> cancelRequest() {
123123
Promise<Void> promise = context.owner().getOrCreateContext().promise();
124124
context.emit(promise, p -> {
125125
PgSocketConnection unwrap = (PgSocketConnection) conn.unwrap();
126-
((PgConnectionFactory) factory).cancelRequest(unwrap.connectOptions(), this.processId(), this.secretKey(), p);
126+
((PgConnectionFactory) factory).cancelRequest(unwrap.connectOptions(), this.processId(), this.secretKey()).onComplete(p);
127127
});
128128
return promise.future();
129129
}

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,29 +83,26 @@ public void init() {
8383
}
8484

8585
// TODO RETURN FUTURE ???
86-
void sendStartupMessage(String username, String password, String database, Map<String, String> properties, Promise<Connection> completionHandler) {
86+
Future<Connection> sendStartupMessage(String username, String password, String database, Map<String, String> properties) {
8787
InitCommand cmd = new InitCommand(this, username, password, database, properties);
88-
schedule(context, cmd).onComplete(completionHandler);
88+
return schedule(context, cmd);
8989
}
9090

91-
void sendCancelRequestMessage(int processId, int secretKey, Handler<AsyncResult<Void>> handler) {
91+
Future<Void> sendCancelRequestMessage(int processId, int secretKey) {
9292
Buffer buffer = Buffer.buffer(16);
9393
buffer.appendInt(16);
9494
// cancel request code
9595
buffer.appendInt(80877102);
9696
buffer.appendInt(processId);
9797
buffer.appendInt(secretKey);
9898

99-
socket.write(buffer).onComplete(ar -> {
99+
return socket.write(buffer).andThen(ar -> {
100100
if (ar.succeeded()) {
101101
// directly close this connection
102102
if (status == Status.CONNECTED) {
103103
status = Status.CLOSING;
104104
socket.close();
105105
}
106-
handler.handle(Future.succeededFuture());
107-
} else {
108-
handler.handle(Future.failedFuture(ar.cause()));
109106
}
110107
});
111108
}

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

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,13 @@
3535
public class TLSTest {
3636

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

4046
private Vertx vertx;
4147

@@ -53,7 +59,7 @@ public void teardown(TestContext ctx) {
5359
public void testTLS(TestContext ctx) {
5460
Async async = ctx.async();
5561

56-
PgConnectOptions options = new PgConnectOptions(rule.options())
62+
PgConnectOptions options = new PgConnectOptions(ruleOptionalSll.options())
5763
.setSslMode(SslMode.REQUIRE)
5864
.setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/server.crt")));
5965
PgConnection.connect(vertx, options.setSslMode(SslMode.REQUIRE)).onComplete(ctx.asyncAssertSuccess(conn -> {
@@ -74,7 +80,7 @@ public void testTLS(TestContext ctx) {
7480
@Test
7581
public void testTLSTrustAll(TestContext ctx) {
7682
Async async = ctx.async();
77-
PgConnection.connect(vertx, rule.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustAll(true))).onComplete(ctx.asyncAssertSuccess(conn -> {
83+
PgConnection.connect(vertx, ruleOptionalSll.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustAll(true))).onComplete(ctx.asyncAssertSuccess(conn -> {
7884
ctx.assertTrue(conn.isSSL());
7985
async.complete();
8086
}));
@@ -83,7 +89,7 @@ public void testTLSTrustAll(TestContext ctx) {
8389
@Test
8490
public void testTLSInvalidCertificate(TestContext ctx) {
8591
Async async = ctx.async();
86-
PgConnection.connect(vertx, rule.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/another.crt")))).onComplete(ctx.asyncAssertFailure(err -> {
92+
PgConnection.connect(vertx, ruleOptionalSll.options().setSslMode(SslMode.REQUIRE).setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/another.crt")))).onComplete(ctx.asyncAssertFailure(err -> {
8793
// ctx.assertEquals(err.getClass(), VertxException.class);
8894
ctx.assertEquals(err.getMessage(), "SSL handshake failed");
8995
async.complete();
@@ -93,7 +99,7 @@ public void testTLSInvalidCertificate(TestContext ctx) {
9399
@Test
94100
public void testSslModeDisable(TestContext ctx) {
95101
Async async = ctx.async();
96-
PgConnectOptions options = rule.options()
102+
PgConnectOptions options = ruleOptionalSll.options()
97103
.setSslMode(SslMode.DISABLE);
98104
PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
99105
ctx.assertFalse(conn.isSSL());
@@ -104,18 +110,30 @@ public void testSslModeDisable(TestContext ctx) {
104110
@Test
105111
public void testSslModeAllow(TestContext ctx) {
106112
Async async = ctx.async();
107-
PgConnectOptions options = rule.options()
113+
PgConnectOptions options = ruleOptionalSll.options()
108114
.setSslMode(SslMode.ALLOW);
109115
PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
110116
ctx.assertFalse(conn.isSSL());
111117
async.complete();
112118
}));
113119
}
114120

121+
@Test
122+
public void testSslModeAllowFallback(TestContext ctx) {
123+
Async async = ctx.async();
124+
PgConnectOptions options = ruleForceSsl.options()
125+
.setSslMode(SslMode.ALLOW)
126+
.setSslOptions(new ClientSSLOptions().setTrustAll(true));
127+
PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
128+
ctx.assertTrue(conn.isSSL());
129+
async.complete();
130+
}));
131+
}
132+
115133
@Test
116134
public void testSslModePrefer(TestContext ctx) {
117135
Async async = ctx.async();
118-
PgConnectOptions options = rule.options()
136+
PgConnectOptions options = ruleOptionalSll.options()
119137
.setSslMode(SslMode.PREFER)
120138
.setSslOptions(new ClientSSLOptions().setTrustAll(true));
121139
PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertSuccess(conn -> {
@@ -124,9 +142,21 @@ public void testSslModePrefer(TestContext ctx) {
124142
}));
125143
}
126144

145+
@Test
146+
public void testSslModePreferFallback(TestContext ctx) {
147+
Async async = ctx.async();
148+
PgConnectOptions options = ruleSllOff.options()
149+
.setSslMode(SslMode.PREFER)
150+
.setSslOptions(new ClientSSLOptions().setTrustAll(true));
151+
PgConnection.connect(vertx, options).onComplete(ctx.asyncAssertSuccess(conn -> {
152+
ctx.assertFalse(conn.isSSL());
153+
async.complete();
154+
}));
155+
}
156+
127157
@Test
128158
public void testSslModeVerifyCaConf(TestContext ctx) {
129-
PgConnectOptions options = rule.options()
159+
PgConnectOptions options = ruleOptionalSll.options()
130160
.setSslMode(SslMode.VERIFY_CA)
131161
.setSslOptions(new ClientSSLOptions().setTrustAll(true));
132162
PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertFailure(error -> {
@@ -136,7 +166,7 @@ public void testSslModeVerifyCaConf(TestContext ctx) {
136166

137167
@Test
138168
public void testSslModeVerifyFullConf(TestContext ctx) {
139-
PgConnectOptions options = rule.options()
169+
PgConnectOptions options = ruleOptionalSll.options()
140170
.setSslOptions(new ClientSSLOptions().setTrustOptions(new PemTrustOptions().addCertPath("tls/another.crt")))
141171
.setSslMode(SslMode.VERIFY_FULL);
142172
PgConnection.connect(vertx, new PgConnectOptions(options)).onComplete(ctx.asyncAssertFailure(error -> {

‎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)