Add SSL/TLS and basic auth support for external OpenSearch connections#1019
Add SSL/TLS and basic auth support for external OpenSearch connections#1019rexbut wants to merge 2 commits intokomoot:masterfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| return config.getTransportAddresses().stream() | ||
| .map(addr -> addr.split(":", 2)) | ||
| .map(parts -> new HttpHost(scheme, parts[0], | ||
| parts.length > 1 ? Integer.parseInt(parts[1]) : 9201)) |
There was a problem hiding this comment.
Default port 9201 wrong for external OpenSearch connections
Medium Severity
The buildHosts() method defaults to port 9201 when no port is specified in a transport address, but the standard OpenSearch HTTP port is 9200. This causes connection failures for users who specify an address without an explicit port (e.g., opensearch.example.com). The value 9201 was carried over from internal runner discovery config and is not correct for external connections. The test also codifies this incorrect default.
Additional Locations (1)
There was a problem hiding this comment.
Just moving existing code (refactoring) from Server.java without changes
There was a problem hiding this comment.
I'm leaning toward fixing this while reworking this code.
|
Thanks. I need to put this on hold for a bit. httpclient5 has a new version 5.6 out which causes failing tests in Photon. This needs resolved first before adding additional features. |
| @Parameter(names = "-opensearch-password", category = GROUP, password = true, description = """ | ||
| Password for basic authentication with an external OpenSearch cluster | ||
| """) | ||
| private String opensearchPassword; |
There was a problem hiding this comment.
JCommander password=true prevents command-line value acceptance
High Severity
The password = true attribute on the -opensearch-password, -opensearch-truststore-password, and -opensearch-keystore-password parameters sets their arity to 0 in JCommander, causing them to prompt for console input rather than accepting values from the command line. Without an explicit arity = 1, passing e.g. -opensearch-password secret won't consume secret as the value — it becomes a stray argument and likely triggers a parse error. The existing -password parameter in PostgresqlConfig notably does not use password = true, following the correct pattern.
Additional Locations (2)
There was a problem hiding this comment.
Can you comment on that? cursor seems to be right here.
|
Hi @lonvia, I've updated the dependencies to the latest versions:
I ran the tests locally with |
lonvia
left a comment
lonvia
left a comment
There was a problem hiding this comment.
The code looks okay but it is very verbose, even for Java standards. I'm also rather reluctant to add 7 more parameters, especially when 3 of them are passwords. I can already see the "security bug bounty hunters" having a field day with that.
It's really hard to find decent documentation on this whole SSL/TLS thing but I see references coming up of being able to configure key and trust stores via Java system variables like this:
-Djavax.net.ssl.keyStoreType=pkcs12
-Djavax.net.ssl.trustStoreType=jks
-Djavax.net.ssl.keyStore=keystore.p12
-Djavax.net.ssl.trustStore=truststore.jks
-Djavax.net.ssl.keyStorePassword=$PASS
-Djavax.net.ssl.trustStorePassword=$PASS
So there must be some 'default' way of setting up a SSL connection that is hopefully less verbose and just does the right thing. This would be my preferred way of configuring.
The -opensearch-ssl parameter might be obliterated by requiring a protocol (https or http) in front of the node URLs and assuming http when it is omitted for backwards compatibility.
That would leave the user and password for the OpenSearch database proper. I don't see a good way around that.
| final var builder = SSLContextBuilder.create(); | ||
|
|
||
| if (config.getOpensearchTruststore() != null) { | ||
| final var truststore = KeyStore.getInstance("PKCS12"); |
There was a problem hiding this comment.
KeyStore has a getInstance() that takes a file and password. Would that be usable here?
| return config.getTransportAddresses().stream() | ||
| .map(addr -> addr.split(":", 2)) | ||
| .map(parts -> new HttpHost(scheme, parts[0], | ||
| parts.length > 1 ? Integer.parseInt(parts[1]) : 9201)) |
There was a problem hiding this comment.
I'm leaning toward fixing this while reworking this code.
| @Parameter(names = "-opensearch-password", category = GROUP, password = true, description = """ | ||
| Password for basic authentication with an external OpenSearch cluster | ||
| """) | ||
| private String opensearchPassword; |
There was a problem hiding this comment.
Can you comment on that? cursor seems to be right here.
|
I don't see why we can't create Trust- and KeyStore in-memory from supplied PEM files? |
|
My knowledge on Trust/Keystores is rather limited, I'm afraid. Can you elaborate on what you mean and/or point to documentation? |
|
You can create an empty KeyStore on the fly and add certificates/keys: KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());
trustStore.load(null, null);
CertificateFactory factory = CertificateFactory.getInstance("X.509");
InputStream certContent = // load PEM content
X509Certificate cert = (X509Certificate) factory.generateCertificate(certContent);
trustStore.setCertificateEntry("ca-root", cert);IMHO handling PEM files is a lot easier from a sysop perspective than having to deal with Java truststore shenanigans. We can also remove any kind of password handling if we rely on the files having reasonable permissions. |
|
I see. But that would mean customizing the process even further. Anything where we can rely on standard Java processes is preferable, even if the standard Java process is a bit convoluted. The goal here is to minimize the code that needs to be maintained in Photon. |
3 participants
Summary
Add support for basic authentication, SSL/TLS and mutual TLS (mTLS) when connecting to an external OpenSearch cluster.
#942
Motivation
In production environments (e.g. Kubernetes), external OpenSearch clusters typically require authentication and encrypted connections. This was not possible until now.
Changes
-opensearch-user,-opensearch-password,-opensearch-ssl,-opensearch-truststore,-opensearch-truststore-password,-opensearch-keystore,-opensearch-keystore-password)OpenSearchTransportBuilderUsage examples
Basic authentication only:
With SSL and a custom CA:
With mutual TLS (mTLS):
Tests
OpenSearchTransportBuilderTest: host parsing/scheme selection, SSL context construction with truststore, full auth+SSL smoke testPhotonDBConfigTest: CLI parameter parsing round-trip and defaultsProof of actual usage
Tested against an external OpenSearch 3.x cluster with basic auth and SSL enabled:
AI disclosure
This PR was developed with AI assistance (Cursor / Claude). All code was reviewed, tested against a real OpenSearch cluster, and validated by the author.