Skip to content

Commit 3f03af7

Browse files
finnegancarrollfinnegancarroll
committed
CertType to store user cert id.
CertType needs to discretely store the unique identifier for a set of certs which the user will use in CertificatesInfo API. Previously inferred from the setting prefix, but transport client and transport server are problematic for this model and cannot be changed for bwc. Signed-off-by: Finn Carroll <carrofin@amazon.com>
1 parent 613f684 commit 3f03af7

File tree

9 files changed

+114
-106
lines changed

9 files changed

+114
-106
lines changed

‎src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import java.util.List;
1717
import java.util.Map;
1818
import java.util.StringJoiner;
19-
import java.util.function.Consumer;
2019
import java.util.stream.Collectors;
2120

2221
import com.carrotsearch.randomizedtesting.RandomizedContext;
@@ -151,7 +150,7 @@ private void assertSSLCertsInfo(
151150
Expect each node transport configured with root and issued cert.
152151
*/
153152
for (CertType expectCert : expectedCertTypes) {
154-
final JsonNode transportCertificates = certificates.get(expectCert.certID());
153+
final JsonNode transportCertificates = certificates.get(expectCert.id());
155154
assertThat(prettyStringBody, transportCertificates.isArray());
156155
assertThat(prettyStringBody, transportCertificates.size(), is(2));
157156
verifyCertsJson(n.nodeNumber(), transportCertificates.get(0));

‎src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesInfo.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public void writeTo(StreamOutput out) throws IOException {
4343
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
4444
builder.startObject("certificates");
4545
for (Map.Entry<CertType, List<CertificateInfo>> entry : certificates.entrySet()) {
46-
builder.field(entry.getKey().certID(), certificates.get(entry.getKey()));
46+
builder.field(entry.getKey().id(), certificates.get(entry.getKey()));
4747
}
4848
return builder.endObject();
4949
}

‎src/main/java/org/opensearch/security/dlic/rest/api/ssl/TransportCertificatesInfoNodesAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ protected CertificatesNodesResponse.CertificatesNodeResponse nodeOperation(final
100100
protected CertificatesInfo loadCertificates(final Optional<String> loadCertType) {
101101
Map<CertType, List<CertificateInfo>> certInfos = new HashMap<>();
102102
for (CertType certType : CertType.CERT_TYPE_REGISTRY) {
103-
if (loadCertType.isEmpty() || loadCertType.get().equalsIgnoreCase(certType.certID())) {
103+
if (loadCertType.isEmpty() || loadCertType.get().equalsIgnoreCase(certType.id())) {
104104
List<CertificateInfo> certs = sslSettingsManager.sslContextHandler(certType)
105105
.map(SslContextHandler::certificates)
106106
.map(this::certificatesDetails)

‎src/main/java/org/opensearch/security/ssl/SslSettingsManager.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ private Map<CertType, SslContextHandler> buildSslContexts(final Environment envi
119119
Optional.ofNullable(configurations.get(cert))
120120
.ifPresentOrElse(
121121
sslConfiguration -> contexts.put(cert, new SslContextHandler(sslConfiguration)),
122-
() -> LOGGER.warn("SSL Configuration for {} Layer hasn't been set", cert.certID())
122+
() -> LOGGER.warn("SSL Configuration for {} Layer hasn't been set", cert.id())
123123
);
124124
});
125125
return contexts.build();
@@ -129,12 +129,12 @@ public synchronized void reloadSslContext(final CertType certType) {
129129
sslContextHandler(certType).ifPresentOrElse(sscContextHandler -> {
130130
try {
131131
if (sscContextHandler.reloadSslContext()) {
132-
LOGGER.info("{} SSL context reloaded", certType.certID());
132+
LOGGER.info("{} SSL context reloaded", certType.id());
133133
}
134134
} catch (CertificateException e) {
135135
throw new OpenSearchException(e);
136136
}
137-
}, () -> LOGGER.error("Missing SSL Context for {}", certType.certID()));
137+
}, () -> LOGGER.error("Missing SSL Context for {}", certType.id()));
138138
}
139139

140140
private Map<CertType, SslConfiguration> loadConfigurations(final Environment environment) {
@@ -164,8 +164,8 @@ private Map<CertType, SslConfiguration> loadConfigurations(final Environment env
164164
auxCert,
165165
new SslConfiguration(auxSslParameters, auxTrustAndKeyStore.v1(), auxTrustAndKeyStore.v2())
166166
);
167-
LOGGER.info("TLS {} Provider : {}", auxCert.certID(), auxSslParameters.provider());
168-
LOGGER.info("Enabled TLS protocols for {} layer : {}", auxCert.certID(), auxSslParameters.allowedProtocols());
167+
LOGGER.info("TLS {} Provider : {}", auxCert.id(), auxSslParameters.provider());
168+
LOGGER.info("Enabled TLS protocols for {} layer : {}", auxCert.id(), auxSslParameters.allowedProtocols());
169169
}
170170
}
171171

@@ -291,10 +291,10 @@ private void validateSettings(final CertType certType, final Settings settings,
291291
} else {
292292
throw new OpenSearchException(
293293
"Wrong "
294-
+ certType.certID()
294+
+ certType.id()
295295
+ " SSL configuration. One of Keystore and Truststore files or X.509 PEM certificates and "
296296
+ "PKCS#8 keys groups should be set to configure "
297-
+ certType.certID()
297+
+ certType.id()
298298
+ " layer"
299299
);
300300
}
@@ -316,7 +316,7 @@ private void validatePemStoreSettings(CertType transportType, final Settings set
316316
if (!transportSettings.hasValue(PEM_CERT_FILEPATH) || !transportSettings.hasValue(PEM_KEY_FILEPATH)) {
317317
throw new OpenSearchException(
318318
"Wrong "
319-
+ transportType.certID().toLowerCase(Locale.ROOT)
319+
+ transportType.id().toLowerCase(Locale.ROOT)
320320
+ " SSL configuration. "
321321
+ String.join(", ", transportSettings.get(PEM_CERT_FILEPATH), transportSettings.get(PEM_KEY_FILEPATH))
322322
+ " must be set"
@@ -325,7 +325,7 @@ private void validatePemStoreSettings(CertType transportType, final Settings set
325325
if (clientAuth == ClientAuth.REQUIRE && !transportSettings.hasValue(PEM_TRUSTED_CAS_FILEPATH)) {
326326
throw new OpenSearchException(
327327
"Wrong "
328-
+ transportType.certID().toLowerCase(Locale.ROOT)
328+
+ transportType.id().toLowerCase(Locale.ROOT)
329329
+ " SSL configuration. "
330330
+ PEM_TRUSTED_CAS_FILEPATH
331331
+ " must be set if client auth is required"
@@ -349,7 +349,7 @@ private void validateKeyStoreSettings(CertType transportType, final Settings set
349349
if (!transportSettings.hasValue(KEYSTORE_FILEPATH)) {
350350
throw new OpenSearchException(
351351
"Wrong "
352-
+ transportType.certID().toLowerCase(Locale.ROOT)
352+
+ transportType.id().toLowerCase(Locale.ROOT)
353353
+ " SSL configuration. "
354354
+ transportSettings.get(KEYSTORE_FILEPATH)
355355
+ " must be set"
@@ -358,7 +358,7 @@ private void validateKeyStoreSettings(CertType transportType, final Settings set
358358
if (clientAuth == ClientAuth.REQUIRE && !transportSettings.hasValue(TRUSTSTORE_FILEPATH)) {
359359
throw new OpenSearchException(
360360
"Wrong "
361-
+ transportType.certID().toLowerCase(Locale.ROOT)
361+
+ transportType.id().toLowerCase(Locale.ROOT)
362362
+ " SSL configuration. "
363363
+ TRUSTSTORE_FILEPATH
364364
+ " must be set if client auth is required"

‎src/main/java/org/opensearch/security/ssl/config/CertType.java‎

Lines changed: 76 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,76 @@
3030
import static org.opensearch.security.ssl.util.SSLConfigConstants.SSL_TRANSPORT_SERVER_PREFIX;
3131

3232
/**
33-
* CertTypes have a 1-to-1 relationship with ssl contexts and identify
34-
* the setting prefix under which configuration settings are located.
35-
* CertTypes are uniquely identified by a `certID` which is used as the key for registering CertTypes on a node
36-
* and fetching certificate info through a CertificatesInfoNodesRequest.
33+
* CertTypes identify the setting prefix under which configuration settings for a set of certificates
34+
* are located as well as the id which uniquely identifies a certificate type to the end user.
35+
* CertTypes have a 1-to-1 relationship with ssl contexts and are registered in the global
36+
* CERT_TYPE_REGISTRY but default for mandatory transports, or dynamically for pluggable auxiliary transports.
3737
*/
3838
public class CertType implements Writeable {
39-
private final String sslConfigSettingPrefix;
39+
private final String certSettingPrefix;
40+
private final String certID;
41+
42+
/**
43+
* In most cases the certID is the last element of the setting prefix.
44+
* We expect this to be the case for all auxiliary transports.
45+
* Exceptions where this pattern does not hold include:
46+
* "plugins.security.ssl.transport.server."
47+
* "plugins.security.ssl.transport.client."
48+
* Where users identify these certificates respectively as:
49+
* "transport_server" & "transport_client"
50+
*/
51+
public CertType(String certSettingPrefix) {
52+
this.certSettingPrefix = certSettingPrefix;
53+
String[] parts = certSettingPrefix.split("\\.");
54+
this.certID = parts[parts.length - 1].toLowerCase(Locale.ROOT);
55+
}
4056

41-
public static CertType HTTP = new CertType(SSL_HTTP_PREFIX);
42-
public static CertType TRANSPORT = new CertType(SSL_TRANSPORT_PREFIX);
43-
public static CertType TRANSPORT_CLIENT = new CertType(SSL_TRANSPORT_CLIENT_PREFIX) {
44-
@Override
45-
public String certID() {
46-
return "transport_client";
47-
}
48-
};
49-
public static CertType TRANSPORT_SERVER = new CertType(SSL_TRANSPORT_SERVER_PREFIX) {
50-
@Override
51-
public String certID() {
52-
return "transport_server";
53-
}
54-
};
57+
public CertType(String certSettingPrefix, String certID) {
58+
this.certSettingPrefix = certSettingPrefix;
59+
this.certID = certID;
60+
}
61+
62+
public CertType(final StreamInput in) throws IOException {
63+
this.certSettingPrefix = in.readString();
64+
this.certID = in.readString();
65+
}
66+
67+
@Override
68+
public void writeTo(StreamOutput out) throws IOException {
69+
out.writeString(this.certSettingPrefix);
70+
out.writeString(this.certID);
71+
}
72+
73+
public String sslSettingPrefix() {
74+
return certSettingPrefix;
75+
}
76+
77+
public String id() {
78+
return this.certID;
79+
}
80+
81+
@Override
82+
public String toString() {
83+
return this.id();
84+
}
85+
86+
@Override
87+
public boolean equals(Object o) {
88+
if (this == o) return true;
89+
if (o == null || getClass() != o.getClass()) return false;
90+
CertType certType = (CertType) o;
91+
return this.id().equals(certType.id());
92+
}
93+
94+
@Override
95+
public int hashCode() {
96+
return Objects.hash(this.id());
97+
}
5598

99+
/*
100+
Write only set for tracking certificate types discovered and registered on a node.
101+
Not all ssl context configurations are known at compile time, so we track newly discovered CertTypes here.
102+
*/
56103
public static class NodeCertTypeRegistry implements Iterable<CertType> {
57104
private final Set<CertType> RegisteredCertType = new HashSet<>();
58105

@@ -75,7 +122,7 @@ public boolean contains(CertType certType) {
75122

76123
public boolean contains(String certID) {
77124
for (CertType certType : RegisteredCertType) {
78-
if (Objects.equals(certType.certID(), certID)) {
125+
if (Objects.equals(certType.id(), certID)) {
79126
return true;
80127
}
81128
}
@@ -90,54 +137,16 @@ public Iterator<CertType> iterator() {
90137
}
91138

92139
/*
93-
Write only map for tracking certificates type discovered and registered on a node.
94-
Not all ssl context configurations are known at compile time, so we track newly discovered CertTypes here.
140+
Mandatory transports.
95141
*/
142+
public static CertType HTTP = new CertType(SSL_HTTP_PREFIX);
143+
public static CertType TRANSPORT = new CertType(SSL_TRANSPORT_PREFIX);
144+
public static CertType TRANSPORT_CLIENT = new CertType(SSL_TRANSPORT_CLIENT_PREFIX, "transport_client");
145+
public static CertType TRANSPORT_SERVER = new CertType(SSL_TRANSPORT_SERVER_PREFIX, "transport_server");
96146
public static final NodeCertTypeRegistry CERT_TYPE_REGISTRY = new NodeCertTypeRegistry(
97-
HTTP,
98-
TRANSPORT,
99-
TRANSPORT_CLIENT,
100-
TRANSPORT_SERVER
147+
HTTP,
148+
TRANSPORT,
149+
TRANSPORT_CLIENT,
150+
TRANSPORT_SERVER
101151
);
102-
103-
public CertType(String sslConfigSettingPrefix) {
104-
this.sslConfigSettingPrefix = sslConfigSettingPrefix;
105-
}
106-
107-
public CertType(final StreamInput in) throws IOException {
108-
this.sslConfigSettingPrefix = in.readString();
109-
}
110-
111-
@Override
112-
public void writeTo(StreamOutput out) throws IOException {
113-
out.writeString(sslConfigSettingPrefix);
114-
}
115-
116-
public String sslSettingPrefix() {
117-
return sslConfigSettingPrefix;
118-
}
119-
120-
public String certID() {
121-
String[] parts = sslConfigSettingPrefix.split("\\.");
122-
String id = parts[parts.length - 1];
123-
return id.toLowerCase(Locale.ROOT);
124-
}
125-
126-
@Override
127-
public String toString() {
128-
return this.certID();
129-
}
130-
131-
@Override
132-
public boolean equals(Object o) {
133-
if (this == o) return true;
134-
if (o == null || getClass() != o.getClass()) return false;
135-
CertType certType = (CertType) o;
136-
return this.certID().equals(certType.certID());
137-
}
138-
139-
@Override
140-
public int hashCode() {
141-
return Objects.hash(this.certID());
142-
}
143152
}

‎src/main/java/org/opensearch/security/ssl/config/SslParameters.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,10 @@ public SslParameters load() {
166166
validateCertDNsOnReload(sslConfigSettings)
167167
);
168168
if (sslParameters.allowedProtocols().isEmpty()) {
169-
throw new OpenSearchSecurityException("No ssl protocols for " + certType.certID() + " layer");
169+
throw new OpenSearchSecurityException("No ssl protocols for " + certType.id() + " layer");
170170
}
171171
if (sslParameters.allowedCiphers().isEmpty()) {
172-
throw new OpenSearchSecurityException("No valid cipher suites for " + certType.certID() + " layer");
172+
throw new OpenSearchSecurityException("No valid cipher suites for " + certType.id() + " layer");
173173
}
174174
return sslParameters;
175175
}

‎src/main/java/org/opensearch/security/ssl/util/SSLConfigConstants.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,11 @@ public final class SSLConfigConstants {
162162

163163
// helper to resolve setting key for CertType namespace
164164
public static String getStringAffixKeyForCertType(Setting.AffixSetting<String> affix, CertType certType) {
165-
return affix.getConcreteSettingForNamespace(certType.certID()).getKey();
165+
return affix.getConcreteSettingForNamespace(certType.id()).getKey();
166166
}
167167

168168
public static String getBoolAffixKeyForCertType(Setting.AffixSetting<Boolean> affix, CertType certType) {
169-
return affix.getConcreteSettingForNamespace(certType.certID()).getKey();
169+
return affix.getConcreteSettingForNamespace(certType.id()).getKey();
170170
}
171171

172172
/**

‎src/test/java/org/opensearch/security/ssl/SslSettingsManagerReloadListenerTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public void testReloadsSslContextOnPemStoreFilesChangedForHttp() throws Exceptio
107107
public void testReloadsSslContextOnPemStoreFilesChangedForAux() throws Exception {
108108
Settings.Builder settings = defaultSettingsBuilder().putList(
109109
AUX_TRANSPORT_TYPES_SETTING.getKey(),
110-
List.of(MOCK_AUX_CERT_TYPE_FOO.certID())
110+
List.of(MOCK_AUX_CERT_TYPE_FOO.id())
111111
).put(MOCK_AUX_CERT_TYPE_FOO.sslSettingPrefix() + ENABLED, true);
112112
reloadSslContextOnPemFilesChangedForTransportType(MOCK_AUX_CERT_TYPE_FOO, settings);
113113
}
@@ -126,7 +126,7 @@ public void testReloadsSslContextOnJdkStoreFilesChangedForHttp() throws Exceptio
126126
public void testReloadsSslContextOnJdkStoreFilesChangedForAux() throws Exception {
127127
Settings.Builder settings = defaultSettingsBuilder().putList(
128128
AUX_TRANSPORT_TYPES_SETTING.getKey(),
129-
List.of(MOCK_AUX_CERT_TYPE_FOO.certID())
129+
List.of(MOCK_AUX_CERT_TYPE_FOO.id())
130130
).put(MOCK_AUX_CERT_TYPE_FOO.sslSettingPrefix() + ENABLED, true);
131131
reloadSslContextOnJdkStoreFilesChangedForTransportType(MOCK_AUX_CERT_TYPE_FOO, settings);
132132
}
@@ -143,7 +143,7 @@ private void reloadSslContextOnJdkStoreFilesChangedForTransportType(CertType cer
143143
final String trustStoreTypeSetting = settingPrefix + TRUSTSTORE_TYPE;
144144
final String keyStorePathSetting = settingPrefix + KEYSTORE_FILEPATH;
145145
final String keyStoreTypeSetting = settingPrefix + KEYSTORE_TYPE;
146-
final String certTypeFilePrefix = certType.certID().toLowerCase(Locale.ROOT);
146+
final String certTypeFilePrefix = certType.id().toLowerCase(Locale.ROOT);
147147
final var keyStorePassword = randomAsciiAlphanumOfLength(10);
148148
final var secureSettings = new MockSecureSettings();
149149
secureSettings.setString(settingPrefix + "truststore_password_secure", keyStorePassword);
@@ -186,7 +186,7 @@ private void reloadSslContextOnPemFilesChangedForTransportType(CertType certType
186186
final String pemTrustCasPathSetting = settingPrefix + PEM_TRUSTED_CAS_FILEPATH;
187187
final String pemCertPathSetting = settingPrefix + PEM_CERT_FILEPATH;
188188
final String pemKeyPathSetting = settingPrefix + PEM_KEY_FILEPATH;
189-
final String certTypeFilePrefix = certType.certID().toLowerCase(Locale.ROOT);
189+
final String certTypeFilePrefix = certType.id().toLowerCase(Locale.ROOT);
190190
MockSecureSettings secureSettings = new MockSecureSettings();
191191
secureSettings.setString(settingPrefix + "pemkey_password_secure", certificatesRule.privateKeyPassword());
192192
reloadSslContextOnFilesChanged(
@@ -214,7 +214,7 @@ private void reloadSslContextOnPemFilesChangedForTransportType(CertType certType
214214

215215
private void reloadSslContextOnFilesChanged(CertType certType, final Settings settings, final CertificatesWriter certificatesWriter)
216216
throws Exception {
217-
final String certNamePrefix = certType.certID().toLowerCase(Locale.ROOT);
217+
final String certNamePrefix = certType.id().toLowerCase(Locale.ROOT);
218218
final var defaultCertificates = generateCertificates();
219219
var defaultKeyPair = defaultCertificates.v1();
220220
var caCertificate = defaultCertificates.v2().v1();

0 commit comments

Comments
 (0)