Skip to content

Commit

Permalink
Polishing.
Browse files Browse the repository at this point in the history 
Make SslConfiguration.enabledProtocols and enabledCipherSuites non-null. Update since tags. Add author tags. Update documentation.

See gh-635
Original pull request: gh-640.
  • Loading branch information
@mp911de
mp911de committed Mar 16, 2021
1 parent a85fa86 commit 5640cc0
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 81 deletions.
13 changes: 6 additions & 7 deletions ...vault-core/src/main/java/org/springframework/vault/client/ClientHttpConnectorFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@
import org.springframework.vault.support.ClientOptions;
import org.springframework.vault.support.SslConfiguration;

import static org.springframework.vault.client.ClientHttpRequestFactoryFactory.createKeyManagerFactory;
import static org.springframework.vault.client.ClientHttpRequestFactoryFactory.createTrustManagerFactory;
import static org.springframework.vault.client.ClientHttpRequestFactoryFactory.hasSslConfiguration;
import static org.springframework.vault.client.ClientHttpRequestFactoryFactory.*;

/**
* Factory for {@link ClientHttpConnector} that supports
Expand All @@ -44,6 +42,7 @@
* dependencies.
*
* @author Mark Paluch
* @author Ryan Gow
* @since 2.2
*/
public class ClientHttpConnectorFactory {
Expand Down Expand Up @@ -106,11 +105,11 @@ private static void configureSsl(SslConfiguration sslConfiguration, SslContextBu
sslConfiguration.getKeyConfiguration()));
}

if (sslConfiguration.getEnabledProtocols() != null) {
if (!sslConfiguration.getEnabledProtocols().isEmpty()) {
sslContextBuilder.protocols(sslConfiguration.getEnabledProtocols());
}

if (sslConfiguration.getEnabledCipherSuites() != null) {
if (!sslConfiguration.getEnabledCipherSuites().isEmpty()) {
sslContextBuilder.ciphers(sslConfiguration.getEnabledCipherSuites());
}
}
Expand Down Expand Up @@ -197,12 +196,12 @@ private static org.eclipse.jetty.client.HttpClient getHttpClient(SslConfiguratio
sslContextFactory.setKeyManagerPassword(new String(keyConfiguration.getKeyPassword()));
}

if (sslConfiguration.getEnabledProtocols() != null) {
if (!sslConfiguration.getEnabledProtocols().isEmpty()) {
sslContextFactory
.setIncludeProtocols(sslConfiguration.getEnabledProtocols().toArray(new String[0]));
}

if (sslConfiguration.getEnabledCipherSuites() != null) {
if (!sslConfiguration.getEnabledCipherSuites().isEmpty()) {
sslContextFactory
.setIncludeCipherSuites(sslConfiguration.getEnabledCipherSuites().toArray(new String[0]));
}
Expand Down
23 changes: 12 additions & 11 deletions ...-core/src/main/java/org/springframework/vault/client/ClientHttpRequestFactoryFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
import javax.net.ssl.X509ExtendedKeyManager;
import javax.net.ssl.X509TrustManager;

import io.netty.handler.ssl.SslContextBuilder;
import io.netty.handler.ssl.SslProvider;
import okhttp3.ConnectionSpec;
import okhttp3.OkHttpClient.Builder;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.client.config.RequestConfig;
Expand All @@ -52,6 +56,7 @@
import org.apache.http.impl.client.LaxRedirectStrategy;
import org.apache.http.impl.conn.DefaultSchemePortResolver;
import org.apache.http.impl.conn.SystemDefaultRoutePlanner;

import org.springframework.http.client.ClientHttpRequestFactory;
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
import org.springframework.http.client.Netty4ClientHttpRequestFactory;
Expand All @@ -68,17 +73,13 @@
import org.springframework.vault.support.SslConfiguration.KeyConfiguration;
import org.springframework.vault.support.SslConfiguration.KeyStoreConfiguration;

import io.netty.handler.ssl.SslContextBuilder;
import io.netty.handler.ssl.SslProvider;
import okhttp3.ConnectionSpec;
import okhttp3.OkHttpClient.Builder;

/**
* Factory for {@link ClientHttpRequestFactory} that supports Apache HTTP Components,
* OkHttp, Netty and the JDK HTTP client (in that order). This factory configures a
* {@link ClientHttpRequestFactory} depending on the available dependencies.
*
* @author Mark Paluch
* @author Ryan Gow
* @since 2.2
*/
public class ClientHttpRequestFactoryFactory {
Expand Down Expand Up @@ -301,13 +302,13 @@ static ClientHttpRequestFactory usingHttpComponents(ClientOptions options, SslCo

String[] enabledProtocols = null;

if (sslConfiguration.getEnabledProtocols() != null) {
if (!sslConfiguration.getEnabledProtocols().isEmpty()) {
enabledProtocols = sslConfiguration.getEnabledProtocols().toArray(new String[0]);
}

String[] enabledCipherSuites = null;

if (sslConfiguration.getEnabledCipherSuites() != null) {
if (!sslConfiguration.getEnabledCipherSuites().isEmpty()) {
enabledCipherSuites = sslConfiguration.getEnabledCipherSuites().toArray(new String[0]);
}

Expand Down Expand Up @@ -362,11 +363,11 @@ static ClientHttpRequestFactory usingOkHttp3(ClientOptions options, SslConfigura

ConnectionSpec.Builder sslConnectionSpecBuilder = new ConnectionSpec.Builder(sslConnectionSpec);

if (sslConfiguration.getEnabledProtocols() != null) {
if (!sslConfiguration.getEnabledProtocols().isEmpty()) {
sslConnectionSpecBuilder.tlsVersions(sslConfiguration.getEnabledProtocols().toArray(new String[0]));
}

if (sslConfiguration.getEnabledCipherSuites() != null) {
if (!sslConfiguration.getEnabledCipherSuites().isEmpty()) {
sslConnectionSpecBuilder
.cipherSuites(sslConfiguration.getEnabledCipherSuites().toArray(new String[0]));
}
Expand Down Expand Up @@ -413,11 +414,11 @@ static ClientHttpRequestFactory usingNetty(ClientOptions options, SslConfigurati
sslConfiguration.getKeyConfiguration()));
}

if (sslConfiguration.getEnabledProtocols() != null) {
if (!sslConfiguration.getEnabledProtocols().isEmpty()) {
sslContextBuilder.protocols(sslConfiguration.getEnabledProtocols());
}

if (sslConfiguration.getEnabledCipherSuites() != null) {
if (!sslConfiguration.getEnabledCipherSuites().isEmpty()) {
sslContextBuilder.ciphers(sslConfiguration.getEnabledCipherSuites());
}

Expand Down
41 changes: 15 additions & 26 deletions ...lt-core/src/main/java/org/springframework/vault/config/EnvironmentVaultConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@

import java.net.URI;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
Expand All @@ -29,35 +32,15 @@
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.vault.authentication.AppIdAuthentication;
import org.springframework.vault.authentication.AppIdAuthenticationOptions;
import org.springframework.vault.authentication.*;
import org.springframework.vault.authentication.AppIdAuthenticationOptions.AppIdAuthenticationOptionsBuilder;
import org.springframework.vault.authentication.AppIdUserIdMechanism;
import org.springframework.vault.authentication.AppRoleAuthentication;
import org.springframework.vault.authentication.AppRoleAuthenticationOptions;
import org.springframework.vault.authentication.AppRoleAuthenticationOptions.AppRoleAuthenticationOptionsBuilder;
import org.springframework.vault.authentication.AppRoleAuthenticationOptions.RoleId;
import org.springframework.vault.authentication.AppRoleAuthenticationOptions.SecretId;
import org.springframework.vault.authentication.AwsEc2Authentication;
import org.springframework.vault.authentication.AwsEc2AuthenticationOptions;
import org.springframework.vault.authentication.AwsEc2AuthenticationOptions.AwsEc2AuthenticationOptionsBuilder;
import org.springframework.vault.authentication.AzureMsiAuthentication;
import org.springframework.vault.authentication.AzureMsiAuthenticationOptions;
import org.springframework.vault.authentication.AzureMsiAuthenticationOptions.AzureMsiAuthenticationOptionsBuilder;
import org.springframework.vault.authentication.ClientAuthentication;
import org.springframework.vault.authentication.ClientCertificateAuthentication;
import org.springframework.vault.authentication.CubbyholeAuthentication;
import org.springframework.vault.authentication.CubbyholeAuthenticationOptions;
import org.springframework.vault.authentication.CubbyholeAuthenticationOptions.CubbyholeAuthenticationOptionsBuilder;
import org.springframework.vault.authentication.IpAddressUserId;
import org.springframework.vault.authentication.KubernetesAuthentication;
import org.springframework.vault.authentication.KubernetesAuthenticationOptions;
import org.springframework.vault.authentication.KubernetesAuthenticationOptions.KubernetesAuthenticationOptionsBuilder;
import org.springframework.vault.authentication.KubernetesJwtSupplier;
import org.springframework.vault.authentication.KubernetesServiceAccountTokenFile;
import org.springframework.vault.authentication.MacAddressUserId;
import org.springframework.vault.authentication.StaticUserId;
import org.springframework.vault.authentication.TokenAuthentication;
import org.springframework.vault.client.VaultEndpoint;
import org.springframework.vault.support.SslConfiguration;
import org.springframework.vault.support.SslConfiguration.KeyStoreConfiguration;
Expand Down Expand Up @@ -105,6 +88,10 @@
* <li>Truststore resource: {@code vault.ssl.trust-store} (optional)</li>
* <li>Truststore password: {@code vault.ssl.trust-store-password} (optional)</li>
* <li>Truststore type: {@code vault.ssl.trust-store-password} (since 2.3, optional)</li>
* <li>Enabled SSL/TLS protocols: {@code vault.ssl.enabled-protocols} (since 2.3.2,
* optional, protocols separated with comma)</li>
* <li>Enabled SSL/TLS cipher suites: {@code vault.ssl.enabled-cipher-suites} (since
* 2.3.2, optional, cipher suites separated with comma)</li>
* </ul>
* </li>
* <li>Authentication method: {@code vault.authentication} (defaults to {@literal TOKEN},
Expand Down Expand Up @@ -174,6 +161,7 @@
* @author Michal Budzyn
* @author Raoof Mohammed
* @author Justin Bertrand
* @author Ryan Gow
* @see org.springframework.core.env.Environment
* @see org.springframework.core.env.PropertySource
* @see VaultEndpoint
Expand Down Expand Up @@ -232,9 +220,9 @@ public SslConfiguration sslConfiguration() {
KeyStoreConfiguration trustStoreConfiguration = getKeyStoreConfiguration("vault.ssl.trust-store",
"vault.ssl.trust-store-password", "vault.ssl.trust-store-type");

List<String> enabledProtocols = getList("vault.ssl.enabled-protocols");
List<String> enabledProtocols = getPropertyAsList("vault.ssl.enabled-protocols");

List<String> enabledCipherSuites = getList("vault.ssl.enabled-cipher-suites");
List<String> enabledCipherSuites = getPropertyAsList("vault.ssl.enabled-cipher-suites");

return new SslConfiguration(keyStoreConfiguration, trustStoreConfiguration, enabledProtocols,
enabledCipherSuites);
Expand Down Expand Up @@ -427,14 +415,15 @@ protected ClientAuthentication kubeAuthentication() {
return new KubernetesAuthentication(builder.build(), restOperations());
}

private List<String> getList(String key) {
private List<String> getPropertyAsList(String key) {

String val = getEnvironment().getProperty(key);

if (val == null) {
return null;
return Collections.emptyList();
}

return Arrays.asList(val.split(","));
return Arrays.stream(val.split(",")).map(String::trim).collect(Collectors.toList());
}

@Nullable
Expand Down
61 changes: 48 additions & 13 deletions spring-vault-core/src/main/java/org/springframework/vault/support/SslConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
* for Client Certificate authentication.
*
* @author Mark Paluch
* @author Ryan Gow
* @see Resource
* @see java.security.KeyStore
* @see org.springframework.vault.authentication.ClientCertificateAuthentication
Expand Down Expand Up @@ -113,7 +114,7 @@ public SslConfiguration(KeyStoreConfiguration keyStoreConfiguration,
* cipher suite strings used by the enabled Java SSL provider. May be {@literal null}
* to indicate the SSL socket factory should use a default list of enabled cipher
* suites.
* @since 2.4
* @since 2.3.2
* @see sun.security.ssl.ProtocolVersion
* @see sun.security.ssl.CipherSuite
*/
Expand All @@ -128,10 +129,8 @@ public SslConfiguration(KeyStoreConfiguration keyStoreConfiguration, KeyConfigur
this.keyStoreConfiguration = keyStoreConfiguration;
this.keyConfiguration = keyConfiguration;
this.trustStoreConfiguration = trustStoreConfiguration;
this.enabledProtocols = enabledProtocols != null
? Collections.unmodifiableList(new ArrayList<>(enabledProtocols)) : null;
this.enabledCipherSuites = enabledCipherSuites != null
? Collections.unmodifiableList(new ArrayList<>(enabledCipherSuites)) : null;
this.enabledProtocols = Collections.unmodifiableList(new ArrayList<>(enabledProtocols));
this.enabledCipherSuites = Collections.unmodifiableList(new ArrayList<>(enabledCipherSuites));
}

/**
Expand All @@ -146,7 +145,8 @@ public SslConfiguration(KeyStoreConfiguration keyStoreConfiguration, KeyConfigur
*/
public SslConfiguration(KeyStoreConfiguration keyStoreConfiguration, KeyConfiguration keyConfiguration,
KeyStoreConfiguration trustStoreConfiguration) {
this(keyStoreConfiguration, keyConfiguration, trustStoreConfiguration, null, null);
this(keyStoreConfiguration, keyConfiguration, trustStoreConfiguration, Collections.emptyList(),
Collections.emptyList());
}

/**
Expand All @@ -163,7 +163,7 @@ public SslConfiguration(KeyStoreConfiguration keyStoreConfiguration, KeyConfigur
* cipher suite strings used by the enabled Java SSL provider. May be {@literal null}
* to indicate the SSL socket factory should use a default list of enabled cipher
* suites.
* @since 2.4
* @since 2.3.2
* @see sun.security.ssl.ProtocolVersion
* @see sun.security.ssl.CipherSuite
*/
Expand Down Expand Up @@ -364,7 +364,7 @@ public static SslConfiguration unconfigured() {
* indicates that the SSL socket factory should use a default list of enabled protocol
* versions.
* @return the list of enabled SSL protocol versions.
* @since 2.4
* @since 2.3.2
*/
public List<String> getEnabledProtocols() {
return this.enabledProtocols;
Expand All @@ -373,12 +373,29 @@ public List<String> getEnabledProtocols() {
/**
* Create a new {@link SslConfiguration} with the enabled protocol versions applied
* retaining the other configuration from this instance.
* @param enabledProtocols may be {@literal null}.
* @param enabledProtocols must not be {@literal null}.
* @return a new {@link SslConfiguration} with the enabled protocol versions applied.
* @since 2.4
* @since 2.3.2
* @see sun.security.ssl.ProtocolVersion
*/
public SslConfiguration withEnabledProtocols(String... enabledProtocols) {

Assert.notNull(enabledProtocols, "Enabled protocols must not be null");

return withEnabledProtocols(Arrays.asList(enabledProtocols));
}

/**
* Create a new {@link SslConfiguration} with the enabled protocol versions applied
* retaining the other configuration from this instance.
* @param enabledProtocols must not be {@literal null}.
* @return a new {@link SslConfiguration} with the enabled protocol versions applied.
* @since 2.3.2
* @see sun.security.ssl.ProtocolVersion
*/
public SslConfiguration withEnabledProtocols(List<String> enabledProtocols) {

Assert.notNull(enabledProtocols, "Enabled protocols must not be null");
return new SslConfiguration(this.keyStoreConfiguration, this.keyConfiguration, this.trustStoreConfiguration,
enabledProtocols, this.enabledCipherSuites);
}
Expand All @@ -388,7 +405,7 @@ public SslConfiguration withEnabledProtocols(List<String> enabledProtocols) {
* indicates that the SSL socket factory should use a default list of enabled cipher
* suites.
* @return the list of enabled SSL cipher suites.
* @since 2.4
* @since 2.3.2
*/
public List<String> getEnabledCipherSuites() {
return this.enabledCipherSuites;
Expand All @@ -397,12 +414,30 @@ public List<String> getEnabledCipherSuites() {
/**
* Create a new {@link SslConfiguration} with the enabled cipher suites applied
* retaining the other configuration from this instance.
* @param enabledCipherSuites may be {@literal null}.
* @param enabledCipherSuites must not be {@literal null}.
* @return a new {@link SslConfiguration} with the enabled cipher suites applied.
* @since 2.4
* @since 2.3.2
* @see sun.security.ssl.CipherSuite
*/
public SslConfiguration withEnabledCipherSuites(String... enabledCipherSuites) {

Assert.notNull(enabledProtocols, "Enabled cipher suites must not be null");

return withEnabledCipherSuites(Arrays.asList(enabledCipherSuites));
}

/**
* Create a new {@link SslConfiguration} with the enabled cipher suites applied
* retaining the other configuration from this instance.
* @param enabledCipherSuites must not be {@literal null}.
* @return a new {@link SslConfiguration} with the enabled cipher suites applied.
* @since 2.3.2
* @see sun.security.ssl.CipherSuite
*/
public SslConfiguration withEnabledCipherSuites(List<String> enabledCipherSuites) {

Assert.notNull(enabledProtocols, "Enabled cipher suites must not be null");

return new SslConfiguration(this.keyStoreConfiguration, this.keyConfiguration, this.trustStoreConfiguration,
this.enabledProtocols, enabledCipherSuites);
}
Expand Down
12 changes: 6 additions & 6 deletions ...est/java/org/springframework/vault/client/ClientHttpConnectorFactoryIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package org.springframework.vault.client;

import java.util.ArrayList;
import java.util.List;

import org.junit.jupiter.api.Test;

import org.springframework.http.client.reactive.ClientHttpConnector;
Expand All @@ -23,17 +26,14 @@
import org.springframework.web.reactive.function.client.WebClient;
import org.springframework.web.reactive.function.client.WebClientResponseException;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.springframework.vault.client.ClientHttpConnectorFactory.JettyClient;
import static org.springframework.vault.client.ClientHttpConnectorFactory.ReactorNetty;

import java.util.ArrayList;
import java.util.List;
import static org.assertj.core.api.AssertionsForClassTypes.*;
import static org.springframework.vault.client.ClientHttpConnectorFactory.*;

/**
* Integration tests for {@link ClientHttpConnectorFactory}.
*
* @author Mark Paluch
* @author Ryan Gow
*/
class ClientHttpConnectorFactoryIntegrationTests {

Expand Down
Loading

0 comments on commit 5640cc0

Please sign in to comment.