Skip to content

Commit d9c6225

Browse files
jrhee17jrhee17
authored
Move allowUnsafeCiphers to TlsSpec (#6792)
## Motivation: This is a preparatory refactor for xDS server-side support. Currently, `allowUnsafeCiphers` is passed as a separate boolean through method chains (`SslContextFactory`, `SslContextUtil`, `VirtualHostBuilder.sslContext()`), which means a `ServerTlsSpec` alone doesn't fully describe a virtual host's TLS configuration. By folding `allowUnsafeCiphers` into the TLS spec objects, each virtual host's TLS configuration can be completely expressed as a single `ServerTlsSpec`. This makes it possible for static TLS configurations to be represented as a `TlsProvider`, which is needed for xDS server-side integration. ## Modifications: - Added `allowUnsafeCiphers` field, constructor parameter, and getter to `AbstractTlsSpec`. - Added `allowUnsafeCiphers` field, setter, and copy-constructor parameter to `AbstractTlsSpecBuilder`. - Removed the `allowUnsafeCiphers` parameter from `SslContextUtil.toSslContext()` — it now reads from the spec directly. - Removed the `allowUnsafeCiphers` constructor parameter and `getOrCreate()` parameter from `SslContextFactory`. - Updated `VirtualHostBuilder` to compute `allowUnsafeCiphers` in `buildServerTlsSpec()` and pass it to `VirtualHostTlsSetter.toServerTlsSpec()`, instead of passing it separately to `sslContext()`. - Updated `TlsProviderMapping` to set `allowUnsafeCiphers` on the spec builder instead of passing it to `SslContextFactory.getOrCreate()`. - Updated `HttpClientFactory` to set `allowUnsafeCiphers` on the `ClientTlsSpec` via `toBuilder()` instead of on the `SslContextFactory`. - Updated `HttpClientDelegate`, `ServerSslContextUtil`, and `MinifiedAuthZpeClient` to set `allowUnsafeCiphers` on the spec builder. - Deprecated `allowUnsafeCiphers` on both `AbstractTlsSpec` and `AbstractTlsSpecBuilder`. ## Result: - TLS spec objects are now self-contained — a `ServerTlsSpec` or `ClientTlsSpec` fully describes a TLS configuration without needing a separate `allowUnsafeCiphers` flag. - No behavioral changes for users.
1 parent e174785 commit d9c6225

16 files changed

Lines changed: 91 additions & 47 deletions

File tree

‎athenz/src/main/java/com/linecorp/armeria/server/athenz/MinifiedAuthZpeClient.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,14 @@ private static JwtsSigningKeyResolver newDefaultJwtsSigningKeyResolver(ZtsBaseCl
147147
}
148148
final ClientFactory clientFactory = ztsBaseClient.clientFactory();
149149
final TlsProvider tlsProvider = clientFactory.options().tlsProvider();
150-
final ClientTlsSpec clientTlsSpec = toTlsSpec(tlsProvider);
151150
final boolean allowUnsafeCiphers = clientFactory.options().tlsConfig().allowsUnsafeCiphers();
151+
final ClientTlsSpec clientTlsSpec = toTlsSpec(tlsProvider, allowUnsafeCiphers);
152152
final JdkSslContext sslContext =
153-
(JdkSslContext) SslContextUtil.toSslContext(clientTlsSpec, allowUnsafeCiphers);
153+
(JdkSslContext) SslContextUtil.toSslContext(clientTlsSpec);
154154
return new JwtsSigningKeyResolver(ztsUri + oauth2KeysPath, sslContext.context(), proxyUriStr);
155155
}
156156

157-
private static ClientTlsSpec toTlsSpec(TlsProvider tlsProvider) {
157+
private static ClientTlsSpec toTlsSpec(TlsProvider tlsProvider, boolean allowUnsafeCiphers) {
158158
final ClientTlsSpecBuilder builder = ClientTlsSpec.builder();
159159
final TlsKeyPair tlsKeyPair = tlsProvider.keyPair("*");
160160
if (tlsKeyPair != null) {
@@ -166,6 +166,7 @@ private static ClientTlsSpec toTlsSpec(TlsProvider tlsProvider) {
166166
}
167167
builder.engineType(TlsEngineType.JDK);
168168
builder.alpnProtocols(SslContextUtil.DEFAULT_ALPN_PROTOCOLS);
169+
builder.allowUnsafeCiphers(allowUnsafeCiphers);
169170
return builder.build();
170171
}
171172

‎core/src/main/java/com/linecorp/armeria/client/ClientTlsSpec.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,11 @@ public static ClientTlsSpecBuilder builder() {
6565
List<TlsPeerVerifierFactory> verifierFactories, TlsEngineType engineType,
6666
Consumer<? super SslContextBuilder> tlsCustomizer,
6767
@Nullable KeyManagerFactory keyManagerFactory,
68-
String endpointIdentificationAlgorithm) {
68+
String endpointIdentificationAlgorithm,
69+
boolean allowUnsafeCiphers) {
6970
super(tlsVersions, alpnProtocols, ciphers, tlsKeyPair,
70-
trustedCertificates, verifierFactories, engineType, tlsCustomizer, keyManagerFactory);
71+
trustedCertificates, verifierFactories, engineType, tlsCustomizer, keyManagerFactory,
72+
allowUnsafeCiphers);
7173
this.endpointIdentificationAlgorithm = endpointIdentificationAlgorithm;
7274
}
7375

@@ -112,6 +114,7 @@ public String toString() {
112114
.add("tlsCustomizer", tlsCustomizer())
113115
.add("keyManagerFactory", keyManagerFactory())
114116
.add("endpointIdentificationAlgorithm", endpointIdentificationAlgorithm())
117+
.add("allowUnsafeCiphers", allowUnsafeCiphers())
115118
.toString();
116119
}
117120

‎core/src/main/java/com/linecorp/armeria/client/ClientTlsSpecBuilder.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ public final class ClientTlsSpecBuilder extends AbstractTlsSpecBuilder<ClientTls
5151

5252
ClientTlsSpecBuilder(ClientTlsSpec clientTlsSpec) {
5353
super(clientTlsSpec.ciphers(), clientTlsSpec.tlsKeyPair(), clientTlsSpec.trustedCertificates(),
54-
clientTlsSpec.verifierFactories(), clientTlsSpec.engineType());
54+
clientTlsSpec.verifierFactories(), clientTlsSpec.engineType(),
55+
clientTlsSpec.allowUnsafeCiphers());
5556
alpnProtocols = clientTlsSpec.alpnProtocols();
5657
keyManagerFactory = clientTlsSpec.keyManagerFactory();
5758
tlsCustomizer = clientTlsSpec.tlsCustomizer();
@@ -108,6 +109,6 @@ public ClientTlsSpec build() {
108109
return new ClientTlsSpec(SslContextUtil.supportedTlsVersions(engineType().sslProvider()),
109110
alpnProtocols, ciphers(), tlsKeyPair(),
110111
trustedCertificates(), verifierFactories(), engineType(), tlsCustomizer,
111-
keyManagerFactory, endpointIdentificationAlgorithm);
112+
keyManagerFactory, endpointIdentificationAlgorithm, allowUnsafeCiphers());
112113
}
113114
}

‎core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ private ClientTlsSpec determineTlsSpec(Endpoint endpoint, SessionProtocol sessio
318318
ClientTlsSpec.builder()
319319
.tlsCustomizer(config.tlsCustomizer())
320320
.engineType(factory.options().tlsEngineType())
321+
.allowUnsafeCiphers(config.allowsUnsafeCiphers())
321322
.alpnProtocols(sessionProtocol);
322323
if (keyPair != null) {
323324
builder.tlsKeyPair(keyPair);

‎core/src/main/java/com/linecorp/armeria/client/HttpClientFactory.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,11 @@ final class HttpClientFactory implements ClientFactory {
170170
meterIdPrefix = options.tlsConfig().meterIdPrefix();
171171
allowUnsafeCiphers = options.tlsConfig().allowsUnsafeCiphers();
172172
}
173-
sslContextFactory = new SslContextFactory(meterIdPrefix, options.meterRegistry(),
174-
allowUnsafeCiphers);
175-
bootstrapSslContexts = new BootstrapSslContexts(baseClientTlsSpec, options, sslContextFactory);
173+
final ClientTlsSpec resolvedTlsSpec = baseClientTlsSpec.toBuilder()
174+
.allowUnsafeCiphers(allowUnsafeCiphers)
175+
.build();
176+
sslContextFactory = new SslContextFactory(meterIdPrefix, options.meterRegistry());
177+
bootstrapSslContexts = new BootstrapSslContexts(resolvedTlsSpec, options, sslContextFactory);
176178

177179
http2InitialConnectionWindowSize = options.http2InitialConnectionWindowSize();
178180
http2InitialStreamWindowSize = options.http2InitialStreamWindowSize();

‎core/src/main/java/com/linecorp/armeria/common/AbstractTlsSpec.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public abstract class AbstractTlsSpec {
5454
private final Consumer<? super SslContextBuilder> tlsCustomizer;
5555
@Nullable
5656
private final KeyManagerFactory keyManagerFactory;
57+
private final boolean allowUnsafeCiphers;
5758

5859
/**
5960
* Creates a new instance with the specified TLS configuration.
@@ -62,7 +63,8 @@ protected AbstractTlsSpec(Set<String> tlsVersions, Set<String> alpnProtocols, Se
6263
@Nullable TlsKeyPair tlsKeyPair, List<X509Certificate> trustedCertificates,
6364
List<TlsPeerVerifierFactory> verifierFactories, TlsEngineType engineType,
6465
Consumer<? super SslContextBuilder> tlsCustomizer,
65-
@Nullable KeyManagerFactory keyManagerFactory) {
66+
@Nullable KeyManagerFactory keyManagerFactory,
67+
boolean allowUnsafeCiphers) {
6668
checkArgument(tlsKeyPair == null || keyManagerFactory == null,
6769
"'tlsKeyPair' and 'keyManagerFactory' cannot both be set");
6870
SslContextUtil.checkVersionsSupported(tlsVersions, engineType.sslProvider());
@@ -75,6 +77,7 @@ protected AbstractTlsSpec(Set<String> tlsVersions, Set<String> alpnProtocols, Se
7577
this.engineType = engineType;
7678
this.tlsCustomizer = tlsCustomizer;
7779
this.keyManagerFactory = keyManagerFactory;
80+
this.allowUnsafeCiphers = allowUnsafeCiphers;
7881
}
7982

8083
/**
@@ -146,6 +149,15 @@ public final Consumer<? super SslContextBuilder> tlsCustomizer() {
146149
return keyManagerFactory;
147150
}
148151

152+
/**
153+
* Returns whether unsafe ciphers are allowed for this TLS configuration.
154+
* @deprecated will be removed
155+
*/
156+
@Deprecated
157+
public final boolean allowUnsafeCiphers() {
158+
return allowUnsafeCiphers;
159+
}
160+
149161
/**
150162
* Returns {@code true} if this is a server-side TLS specification, {@code false} otherwise.
151163
*/
@@ -168,13 +180,15 @@ public boolean equals(@Nullable Object o) {
168180
Objects.equal(verifierFactories, tlsSpec.verifierFactories()) &&
169181
engineType == tlsSpec.engineType() &&
170182
Objects.equal(tlsCustomizer, tlsSpec.tlsCustomizer()) &&
171-
Objects.equal(keyManagerFactory, tlsSpec.keyManagerFactory());
183+
Objects.equal(keyManagerFactory, tlsSpec.keyManagerFactory()) &&
184+
allowUnsafeCiphers == tlsSpec.allowUnsafeCiphers;
172185
}
173186

174187
@Override
175188
public int hashCode() {
176189
return Objects.hashCode(tlsVersions, alpnProtocols(), ciphers, tlsKeyPair, trustedCertificates,
177-
verifierFactories, engineType, tlsCustomizer, keyManagerFactory);
190+
verifierFactories, engineType, tlsCustomizer, keyManagerFactory,
191+
allowUnsafeCiphers);
178192
}
179193

180194
@Override
@@ -189,6 +203,7 @@ public String toString() {
189203
.add("engineType", engineType)
190204
.add("tlsCustomizer", tlsCustomizer)
191205
.add("keyManagerFactory", keyManagerFactory)
206+
.add("allowUnsafeCiphers", allowUnsafeCiphers)
192207
.toString();
193208
}
194209
}

‎core/src/main/java/com/linecorp/armeria/common/AbstractTlsSpecBuilder.java‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public abstract class AbstractTlsSpecBuilder<SELF extends AbstractTlsSpecBuilder
4343
private List<X509Certificate> trustedCertificates = ImmutableList.of();
4444
private List<TlsPeerVerifierFactory> verifierFactories = ImmutableList.of();
4545
private TlsEngineType engineType = Flags.tlsEngineType();
46+
private boolean allowUnsafeCiphers;
4647

4748
/**
4849
* Creates a new builder with default settings.
@@ -54,12 +55,15 @@ protected AbstractTlsSpecBuilder() {}
5455
*/
5556
protected AbstractTlsSpecBuilder(Set<String> ciphers, @Nullable TlsKeyPair tlsKeyPair,
5657
List<X509Certificate> trustedCertificates,
57-
List<TlsPeerVerifierFactory> verifierFactories, TlsEngineType engineType) {
58+
List<TlsPeerVerifierFactory> verifierFactories,
59+
TlsEngineType engineType,
60+
boolean allowUnsafeCiphers) {
5861
this.ciphers = ciphers;
5962
this.tlsKeyPair = tlsKeyPair;
6063
this.trustedCertificates = trustedCertificates;
6164
this.verifierFactories = verifierFactories;
6265
this.engineType = engineType;
66+
this.allowUnsafeCiphers = allowUnsafeCiphers;
6367
}
6468

6569
/**
@@ -172,6 +176,23 @@ protected final TlsEngineType engineType() {
172176
return engineType;
173177
}
174178

179+
/**
180+
* Sets whether to allow unsafe ciphers.
181+
* @deprecated will be removed
182+
*/
183+
@Deprecated
184+
public final SELF allowUnsafeCiphers(boolean allowUnsafeCiphers) {
185+
this.allowUnsafeCiphers = allowUnsafeCiphers;
186+
return self();
187+
}
188+
189+
/**
190+
* Returns whether unsafe ciphers are allowed.
191+
*/
192+
protected final boolean allowUnsafeCiphers() {
193+
return allowUnsafeCiphers;
194+
}
195+
175196
@SuppressWarnings("unchecked")
176197
final SELF self() {
177198
return (SELF) this;

‎core/src/main/java/com/linecorp/armeria/common/Flags.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ private static void detectTlsEngineAndDumpOpenSslInfo() {
658658
final ClientTlsSpec tlsSpec = ClientTlsSpec.builder()
659659
.alpnProtocols(SslContextUtil.DEFAULT_ALPN_PROTOCOLS)
660660
.build();
661-
final SSLEngine engine = SslContextUtil.toSslContext(tlsSpec, false)
661+
final SSLEngine engine = SslContextUtil.toSslContext(tlsSpec)
662662
.newEngine(ByteBufAllocator.DEFAULT);
663663
logger.info("All available SSL protocols: {}",
664664
ImmutableList.copyOf(engine.getSupportedProtocols()));

‎core/src/main/java/com/linecorp/armeria/internal/common/SslContextFactory.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ public final class SslContextFactory {
5050
private final Map<SslContext, AbstractTlsSpec> reverseCache = new HashMap<>();
5151

5252
private final MeterRegistry meterRegistry;
53-
private final boolean allowUnsafeCiphers;
5453
@Nullable
5554
private final MeterIdPrefix meterIdPrefix;
5655

@@ -61,23 +60,17 @@ public SslContextFactory(MeterRegistry meterRegistry) {
6160
}
6261

6362
public SslContextFactory(@Nullable MeterIdPrefix meterIdPrefix, MeterRegistry meterRegistry) {
64-
this(meterIdPrefix, meterRegistry, false);
65-
}
66-
67-
public SslContextFactory(@Nullable MeterIdPrefix meterIdPrefix, MeterRegistry meterRegistry,
68-
boolean allowUnsafeCiphers) {
6963
this.meterIdPrefix = meterIdPrefix;
7064
this.meterRegistry = meterRegistry;
71-
this.allowUnsafeCiphers = allowUnsafeCiphers;
7265
}
7366

74-
public SslContext getOrCreate(ServerTlsSpec serverTlsSpec, boolean allowsUnsafeCiphers) {
67+
public SslContext getOrCreate(ServerTlsSpec serverTlsSpec) {
7568
lock.lock();
7669
try {
7770
final SslContextHolder contextHolder =
7871
cache.computeIfAbsent(serverTlsSpec, unused -> {
7972
final SslContext sslContext =
80-
SslContextUtil.toSslContext(serverTlsSpec, allowsUnsafeCiphers);
73+
SslContextUtil.toSslContext(serverTlsSpec);
8174
return toContextHolder(serverTlsSpec, sslContext);
8275
});
8376
contextHolder.retain();
@@ -94,7 +87,7 @@ public SslContext getOrCreate(ClientTlsSpec clientTlsSpec) {
9487
final SslContextHolder contextHolder =
9588
cache.computeIfAbsent(clientTlsSpec, unused -> {
9689
final SslContext sslContext =
97-
SslContextUtil.toSslContext(clientTlsSpec, allowUnsafeCiphers);
90+
SslContextUtil.toSslContext(clientTlsSpec);
9891
return toContextHolder(clientTlsSpec, sslContext);
9992
});
10093
contextHolder.retain();

‎core/src/main/java/com/linecorp/armeria/internal/common/util/SslContextUtil.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,13 @@ public final class SslContextUtil {
9595
private static boolean warnedMissingEssentialCipherSuite;
9696
private static boolean warnedBadCipherSuite;
9797

98-
public static SslContext toSslContext(ClientTlsSpec clientTlsSpec, boolean allowUnsafeCiphers) {
98+
public static SslContext toSslContext(ClientTlsSpec clientTlsSpec) {
9999
checkArgument(!clientTlsSpec.alpnProtocols().isEmpty(), "Specify at least one ALPN protocol.");
100100
return MinifiedBouncyCastleProvider.call(() -> {
101101
SslContext sslContext = null;
102102
try {
103103
sslContext = toSslContext0(clientTlsSpec);
104-
validateSslContext(allowUnsafeCiphers, sslContext);
104+
validateSslContext(clientTlsSpec.allowUnsafeCiphers(), sslContext);
105105
} catch (Exception e) {
106106
ReferenceCountUtil.release(sslContext);
107107
return Exceptions.throwUnsafely(e);
@@ -110,13 +110,13 @@ public static SslContext toSslContext(ClientTlsSpec clientTlsSpec, boolean allow
110110
});
111111
}
112112

113-
public static SslContext toSslContext(ServerTlsSpec serverTlsSpec, boolean allowUnsafeCiphers) {
113+
public static SslContext toSslContext(ServerTlsSpec serverTlsSpec) {
114114
checkArgument(!serverTlsSpec.alpnProtocols().isEmpty(), "Specify at least one ALPN protocol.");
115115
return MinifiedBouncyCastleProvider.call(() -> {
116116
SslContext sslContext = null;
117117
try {
118118
sslContext = toSslContext0(serverTlsSpec);
119-
validateSslContext(allowUnsafeCiphers, sslContext);
119+
validateSslContext(serverTlsSpec.allowUnsafeCiphers(), sslContext);
120120
} catch (Exception e) {
121121
ReferenceCountUtil.release(sslContext);
122122
return Exceptions.throwUnsafely(e);

0 commit comments

Comments
 (0)