Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls_available support #1127

Merged
merged 2 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/main/java/io/nats/client/Options.java
Original file line number Diff line number Diff line change
Expand Up @@ -1710,6 +1710,10 @@ else if (useDefaultTls) {
}
}

if (tlsFirst && sslContext == null) {
throw new IllegalStateException("SSL context required for tls handshake first");
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not used to check this before, but if there is no SSL Context, we can't do any tls anyway, so it's a fast fail essentially.

if (credentialPath != null) {
File file = new File(credentialPath).getAbsoluteFile();
authHandler = Nats.credentials(file.toString());
Expand Down Expand Up @@ -2101,10 +2105,10 @@ public int getMaxControlLine() {

/**
*
* @return true if there is an sslContext for this Options, otherwise false, see {@link Builder#secure() secure()} in the builder doc
* @return true if there is an sslContext for these Options, otherwise false, see {@link Builder#secure() secure()} in the builder doc
*/
public boolean isTLSRequired() {
return tlsFirst || this.sslContext != null;
return sslContext != null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reflects checking tls first and ssl context in the builder

}

/**
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/io/nats/client/api/ServerInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class ServerInfo {
private final boolean headersSupported;
private final boolean authRequired;
private final boolean tlsRequired;
private final boolean tlsAvailable;
private final long maxPayload;
private final List<String> connectURLs;
private final int protocolVersion;
Expand Down Expand Up @@ -67,7 +68,8 @@ public ServerInfo(String json) {
headersSupported = readBoolean(jv, HEADERS);
authRequired = readBoolean(jv, AUTH_REQUIRED);
nonce = readBytes(jv, NONCE);
tlsRequired = readBoolean(jv, TLS);
tlsRequired = readBoolean(jv, TLS_REQUIRED);
tlsAvailable = readBoolean(jv, TLS_AVAILABLE);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

71: Better variable name
72: read the value from the json

lameDuckMode = readBoolean(jv, LAME_DUCK_MODE);
jetStream = readBoolean(jv, JETSTREAM);
port = readInteger(jv, PORT, 0);
Expand Down Expand Up @@ -121,6 +123,10 @@ public boolean isTLSRequired() {
return this.tlsRequired;
}

public boolean isTLSAvailable() {
return tlsAvailable;
}

public long getMaxPayload() {
return this.maxPayload;
}
Expand Down Expand Up @@ -181,6 +187,7 @@ public String toString() {
", headersSupported=" + headersSupported +
", authRequired=" + authRequired +
", tlsRequired=" + tlsRequired +
", tlsAvailable=" + tlsAvailable +
", maxPayload=" + maxPayload +
", connectURLs=" + connectURLs +
", protocolVersion=" + protocolVersion +
Expand Down
41 changes: 15 additions & 26 deletions src/main/java/io/nats/client/impl/NatsConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -584,36 +584,25 @@ void checkVersionRequirements() throws IOException {
}

void upgradeToSecureIfNeeded(NatsUri nuri) throws IOException {
Options clientOptions = getOptions();
if (clientOptions.isTlsFirst()) {
this.dataPort.upgradeToSecure();
}
else {
ServerInfo serverInfo = getInfo();
boolean before2_9_19 = serverInfo.isOlderThanVersion("2.9.19");

boolean isTLSRequired = clientOptions.isTLSRequired();
boolean upgradeRequired = isTLSRequired;
if (isTLSRequired && nuri.isWebsocket()) {
// We are already communicating over "https" websocket, so
// do NOT try to upgrade to secure.
if (before2_9_19) {
isTLSRequired = false;
}
upgradeRequired = false;
}
if (isTLSRequired && !serverInfo.isTLSRequired()) {
throw new IOException("SSL connection wanted by client.");
}
else if (!isTLSRequired && serverInfo.isTLSRequired()) {
throw new IOException("SSL required by server.");
// When already communicating over "https" websocket, do NOT try to upgrade to secure.
if (!nuri.isWebsocket()) {
if (options.isTlsFirst()) {
dataPort.upgradeToSecure();
}
if (upgradeRequired) {
this.dataPort.upgradeToSecure();
else {
ServerInfo serverInfo = getInfo();
if (options.isTLSRequired()) {
if (!serverInfo.isTLSRequired() && !serverInfo.isTLSAvailable()) {
throw new IOException("SSL connection wanted by client.");
}
dataPort.upgradeToSecure();
}
else if (serverInfo.isTLSRequired()) {
throw new IOException("SSL required by server.");
}
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole block is simplified.

  1. getOptions returns the options variable, just use the variable directly. There is no lazy loading or anything.
  2. No need to check 2.9.19 because websocket never does upgrade, it was just extraneous

// Called from reader/writer thread
void handleCommunicationIssue(Exception io) {
// If we are connecting or disconnecting, note exception and leave
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/io/nats/client/support/ApiConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ public interface ApiConstants {
String TIME = "time";
String TIMESTAMP = "ts";
String TLS = "tls_required";
String TLS_REQUIRED = TLS;
String TLS_AVAILABLE = "tls_available";
String TOTAL = "total";
String TYPE = "type";
String VERSION = "version";
Expand Down
1 change: 1 addition & 0 deletions src/test/java/io/nats/client/api/ServerInfoTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public void testValidInfoString() {
assertEquals(7777, info.getPort());
assertTrue(info.isAuthRequired());
assertTrue(info.isTLSRequired());
assertTrue(info.isTLSAvailable());
assertTrue(info.isHeadersSupported());
assertEquals(100_000_000_000L, info.getMaxPayload());
assertEquals(1, info.getProtocolVersion());
Expand Down
93 changes: 92 additions & 1 deletion src/test/java/io/nats/client/impl/TLSConnectTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -397,4 +397,95 @@ public void testSSLContextFactoryPropertiesPassOnCorrectly() throws NoSuchAlgori
assertEquals("tlsAlgorithm", factory.properties.tlsAlgorithm);
assertEquals("tlsAlgorithm", factory.properties.getTlsAlgorithm());
}
}

private static final int SERVER_INSECURE = 1;
private static final int SERVER_TLS_AVAILABLE = 2;
private static final int SERVER_TLS_REQUIRED = 3;
static class ProxyConnection extends NatsConnection {
int serverType;

public ProxyConnection(String servers, boolean tlsFirst, ErrorListener listener, int serverType) throws Exception {
super(makeMiddleman(servers, tlsFirst, listener));
this.serverType = serverType;
}

private static Options makeMiddleman(String servers, boolean tlsFirst, ErrorListener listener) throws Exception {
Options.Builder builder = new Options.Builder()
.server(servers)
.maxReconnects(0)
.sslContext(SslTestingHelper.createTestSSLContext())
.errorListener(listener);

if (tlsFirst) {
builder.tlsFirst();
}

return builder.build();
}

@Override
void handleInfo(String infoJson) {
switch (serverType) {
case SERVER_INSECURE:
super.handleInfo(infoJson.replace(",\"tls_required\":true", "")); break;
case SERVER_TLS_AVAILABLE:
super.handleInfo(infoJson.replace("\"tls_required\":true", "\"tls_available\":true")); break;
default:
super.handleInfo(infoJson);
}
}
}

/*
1. client tls first | secure proxy | server insecure -> connects
2. client tls first | secure proxy | server tls required -> connects
3. client tls first | secure proxy | server tls available -> connects
4. client regular secure | secure proxy | server insecure -> mismatch exception
5. client regular secure | secure proxy | server tls required -> connects
6. client regular secure | secure proxy | server tls available -> connects
*/

@Test
public void testProxyTlsFirst() throws Exception {
if (TestBase.atLeast2_10_3(ensureRunServerInfo())) {
try (NatsTestServer ts = new NatsTestServer("src/test/resources/tls_first.conf", false)) {
// 1. client tls first | secure proxy | server insecure -> connects
ProxyConnection connTI = new ProxyConnection(ts.getURI(), true, null, SERVER_INSECURE);
connTI.connect(false);
closeConnection(standardConnectionWait(connTI), 1000);

// 2. client tls first | secure proxy | server tls required -> connects
ProxyConnection connTR = new ProxyConnection(ts.getURI(), true, null, SERVER_TLS_REQUIRED);
connTR.connect(false);
closeConnection(standardConnectionWait(connTR), 1000);

// 3. client tls first | secure proxy | server tls available -> connects
ProxyConnection connTA = new ProxyConnection(ts.getURI(), true, null, SERVER_TLS_AVAILABLE);
connTA.connect(false);
closeConnection(standardConnectionWait(connTA), 1000);
}
}
}

@Test
public void testProxyNotTlsFirst() throws Exception {
try (NatsTestServer ts = new NatsTestServer("src/test/resources/tls.conf", false)) {
// 4. client regular secure | secure proxy | server insecure -> mismatch exception
ListenerForTesting listener = new ListenerForTesting();
ProxyConnection connRI = new ProxyConnection(ts.getURI(), false, listener, SERVER_INSECURE);
assertThrows(Exception.class, () -> connRI.connect(false));
assertEquals(1, listener.getExceptions().size());
assertTrue(listener.getExceptions().get(0).getMessage().contains("SSL connection wanted by client"));

// 5. client regular secure | secure proxy | server tls required -> connects
ProxyConnection connRR = new ProxyConnection(ts.getURI(), false, null, SERVER_TLS_REQUIRED);
connRR.connect(false);
closeConnection(standardConnectionWait(connRR), 1000);

// 6. client regular secure | secure proxy | server tls available -> connects
ProxyConnection connRA = new ProxyConnection(ts.getURI(), false, null, SERVER_TLS_AVAILABLE);
connRA.connect(false);
closeConnection(standardConnectionWait(connRA), 1000);
}
}
}
16 changes: 16 additions & 0 deletions src/test/java/io/nats/client/impl/WebsocketConnectTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,22 @@ public void testURISchemeWSSConnection() throws Exception {
}
}

@Test
public void testURISchemeWSSConnectionEnsureTlsFirstHasNoEffect() throws Exception {
SSLContext originalDefault = SSLContext.getDefault();
try (NatsTestServer ts = new NatsTestServer("src/test/resources/wss.conf", false)) {
SSLContext.setDefault(SslTestingHelper.createTestSSLContext());
Options options = Options.builder()
.server(getLocalhostUri("wss", ts.getPort("wss")))
.maxReconnects(0)
.tlsFirst()
.build();
assertCanConnect(options);
} finally {
SSLContext.setDefault(originalDefault);
}
}

@Test
public void testTLSMessageFlow() throws Exception {
try (NatsTestServer ts = new NatsTestServer("src/test/resources/wssverify.conf", false)) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/resources/data/ServerInfoJson.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
INFO {"server_id": "serverId","server_name": "serverName","version": "1.2.3","go": "go0.0.0","host": "host","port": 7777,"headersSupported": true,"auth_required": true,"tls_required": true,"max_payload": 100000000000,"proto": 1,"ldm": true,"jetstream": true,"client_id": 42,"client_ip": "127.0.0.1","cluster": "cluster","connect_urls":["url0", "url1"],"nonce":"<encoded>","headers": true,}
INFO {"server_id": "serverId","server_name": "serverName","version": "1.2.3","go": "go0.0.0","host": "host","port": 7777,"headersSupported": true,"auth_required": true,"tls_required": true,"tls_available": true,"max_payload": 100000000000,"proto": 1,"ldm": true,"jetstream": true,"client_id": 42,"client_ip": "127.0.0.1","cluster": "cluster","connect_urls":["url0", "url1"],"nonce":"<encoded>","headers": true,}
Loading