Skip to content
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import java.util.function.Supplier;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -52,8 +53,8 @@ public final class DefaultHttpDestination implements HttpDestination
@Delegate
private final DestinationProperties baseProperties;

@Nullable
private final KeyStore keyStore;
@Nonnull
private final Supplier<Option<KeyStore>> keyStore;

@Nullable
private final KeyStore trustStore;
Expand Down Expand Up @@ -98,7 +99,7 @@ private DefaultHttpDestination(
@Nonnull final DestinationProperties baseProperties,
@Nonnull final ComplexDestinationPropertyFactory destinationPropertyFactory,
@Nullable final List<Header> customHeaders,
@Nullable final KeyStore keyStore,
@Nonnull final Supplier<Option<KeyStore>> keyStore,
@Nullable final KeyStore trustStore,
@Nullable final List<DestinationHeaderProvider> customHeaderProviders )
{
Expand Down Expand Up @@ -296,7 +297,7 @@ public Option<ProxyConfiguration> getProxyConfiguration()
@Override
public Option<KeyStore> getKeyStore()
{
return Option.of(keyStore);
return keyStore.get();
}

@Nonnull
Expand Down Expand Up @@ -516,7 +517,7 @@ public static Builder fromDestination( @Nonnull final Destination destination )
builder
.headerProviders(httpDestination.getCustomHeaderProviders().toArray(new DestinationHeaderProvider[0]));

httpDestination.getKeyStore().map(builder::keyStore);
builder.keyStoreSupplier(httpDestination.keyStore);
httpDestination.getTrustStore().map(builder::trustStore);
}

Expand All @@ -538,7 +539,9 @@ public boolean equals( @Nullable final Object o )
return new EqualsBuilder()
.append(baseProperties, that.baseProperties)
.append(customHeaders, that.customHeaders)
.append(resolveCertificatesOnly(keyStore), resolveCertificatesOnly(that.keyStore))
.append(
resolveCertificatesOnly(keyStore.get().getOrNull()),
resolveCertificatesOnly(that.keyStore.get().getOrNull()))
.append(resolveCertificatesOnly(trustStore), resolveCertificatesOnly(that.trustStore))
.isEquals();
}
Expand All @@ -549,7 +552,7 @@ public int hashCode()
return new HashCodeBuilder(17, 37)
.append(baseProperties)
.append(customHeaders)
.append(resolveKeyStoreHashCode(keyStore))
.append(resolveKeyStoreHashCode(keyStore.get().getOrNull()))
.append(resolveKeyStoreHashCode(trustStore))
.toHashCode();
}
Expand All @@ -568,11 +571,28 @@ public static class Builder
private DefaultHttpDestinationBuilderProxyHandler proxyHandler =
new DefaultHttpDestinationBuilderProxyHandler();

@Nonnull
Supplier<Option<KeyStore>> keystoreSupplier = Option::none;

/**
* The {@link KeyStore} to be used when communicating over HTTP.
*/
@Setter( onParam_ = @Nullable )
KeyStore keyStore = null;
@Nonnull
public Builder keyStore( @Nullable final KeyStore keyStore )
{
this.keystoreSupplier = () -> Option.of(keyStore);
return this;
}

/**
* A {@link Supplier<KeyStore>} to allow for dynamically resolve certificates at runtime for HTTP communication.
*/
@Nonnull
Builder keyStoreSupplier( @Nonnull final Supplier<Option<KeyStore>> supplier )
{
this.keystoreSupplier = supplier;
return this;
}

/**
* The trust store to be used when communicating over HTTP.
Expand Down Expand Up @@ -1022,7 +1042,7 @@ DefaultHttpDestination buildInternal()
builder.build(),
new ComplexDestinationPropertyFactory(),
headers,
keyStore,
keystoreSupplier,
trustStore,
customHeaderProviders);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ void testFromDestination()
assertThat(sut.get("bar", v -> (int) v)).containsExactly(42);
assertThat(sut.headers).containsExactly(header);
assertThat(sut.customHeaderProviders).containsExactly(headerProvider);
assertThat(sut.keyStore).isSameAs(keyStore);
assertThat(sut.keystoreSupplier.get().getOrNull()).isSameAs(keyStore);
assertThat(sut.trustStore).isSameAs(trustStore);
assertThat(sut.get(DestinationProperty.TRUST_ALL)).containsExactly(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.sap.cloud.sdk.cloudplatform.security.ClientCredentials;
import com.sap.cloud.sdk.cloudplatform.security.Credentials;

import io.vavr.control.Option;
import lombok.SneakyThrows;

class DefaultHttpDestinationTest
Expand Down Expand Up @@ -206,6 +207,22 @@ void testKeyStoreAndKeyStorePasswordCanBeSetAndRead()
assertThat(defaultHttpDestination.getKeyStorePassword().get()).isEqualTo(keyStorePassword);
}

@Test
void testDynamicKeyStoreSupplierIsInvokedOnEachAccess()
{
final KeyStore keyStore1 = mock(KeyStore.class);
final KeyStore keyStore2 = mock(KeyStore.class);
final KeyStore[] current = { keyStore1 };

final DefaultHttpDestination destination =
DefaultHttpDestination.builder("some-uri").keyStoreSupplier(() -> Option.of(current[0])).build();

assertThat(destination.getKeyStore().get()).isSameAs(keyStore1);

current[0] = keyStore2;
assertThat(destination.getKeyStore().get()).isSameAs(keyStore2);
}

@Test
void testTrustStoreCanBeSetAndRead()
{
Expand Down Expand Up @@ -706,7 +723,7 @@ void testToBuilderContainsAllProperties()
assertThat(sut.get("bar", v -> (int) v)).containsExactly(42);
assertThat(sut.headers).containsExactly(header);
assertThat(sut.customHeaderProviders).containsExactly(headerProvider);
assertThat(sut.keyStore).isSameAs(keyStore);
assertThat(sut.keystoreSupplier.get().getOrNull()).isSameAs(keyStore);
assertThat(sut.trustStore).isSameAs(trustStore);
assertThat(sut.get(DestinationProperty.TRUST_ALL)).containsExactly(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@

import java.net.URI;
import java.net.URISyntaxException;
import java.security.KeyStore;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import com.sap.cloud.environment.servicebinding.api.ServiceIdentifier;
import com.sap.cloud.sdk.cloudplatform.connectivity.BtpServiceOptions.BusinessLoggingOptions;
Expand Down Expand Up @@ -265,28 +263,18 @@ private boolean currentTenantIsProvider()
}

private void attachClientKeyStore( @Nonnull final OAuth2Options.Builder optionsBuilder )
{
final KeyStore maybeClientStore = getClientKeyStore();
if( maybeClientStore != null ) {
// note: in case the KS is loaded from ZTIS, the KS used for token retrieval and the KS registered here for mTLS to the target system may diverge
// Token retrieval supports certificate rotation in place, but mTLS to the target system requires re-loading the destination instead.
optionsBuilder.withClientKeyStore(maybeClientStore);
}
}

@Nullable
private KeyStore getClientKeyStore()
{
final ClientIdentity clientIdentity = getClientIdentity();

if( clientIdentity instanceof ZtisClientIdentity ztisClientIdentity ) {
return ztisClientIdentity.getKeyStore();
optionsBuilder.withClientKeyStoreSupplier(ztisClientIdentity::getKeyStore);
return;
}
if( !(clientIdentity instanceof ClientCertificate) ) {
return null;
return;
}

try {
return SSLContextFactory.getInstance().createKeyStore(clientIdentity);
optionsBuilder.withClientKeyStore(SSLContextFactory.getInstance().createKeyStore(clientIdentity));
}
catch( final Exception e ) {
throw new DestinationAccessException("Unable to extract client key store from IAS service binding.", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Supplier;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -65,12 +66,11 @@ public final class OAuth2Options
@Getter
private final TimeLimiterConfiguration timeLimiter;
/**
* The {@link KeyStore} to use for building an mTLS connection towards the <b>target system</b>. This
* A supplier for the {@link KeyStore} to use for building an mTLS connection towards the <b>target system</b>. This
* {@link KeyStore} <b>is not used</b> to build an mTLS connection towards the OAuth2 token service.
*/
@Nullable
@Getter
private final KeyStore clientKeyStore;
private final Supplier<KeyStore> clientKeyStoreSupplier;

/**
* Configuration for caching OAuth2 tokens.
Expand Down Expand Up @@ -112,6 +112,26 @@ public Map<String, String> getAdditionalTokenRetrievalParameters()
return new HashMap<>(additionalTokenRetrievalParameters);
}

/**
* Returns the {@link KeyStore} to use for building an mTLS connection towards the <b>target system</b>, or
* {@code null} if no key store is configured.
*/
@Nullable
public KeyStore getClientKeyStore()
{
return clientKeyStoreSupplier != null ? clientKeyStoreSupplier.get() : null;
}

/**
* Returns the supplier for the {@link KeyStore} to use for building an mTLS connection towards the <b>target
* system</b>, or {@code null} if no key store is configured.
*/
@Nullable
Supplier<KeyStore> getClientKeyStoreSupplier()
{
return clientKeyStoreSupplier;
}

/**
* Returns a new {@link Builder} instance that can be used to create a customized {@link OAuth2Options} instance.
*
Expand All @@ -131,7 +151,8 @@ public static class Builder
{
private boolean skipTokenRetrieval = false;
private final Map<String, String> additionalTokenRetrievalParameters = new HashMap<>();
private KeyStore clientKeyStore;
@Nullable
private Supplier<KeyStore> clientKeyStoreSupplier;
private TimeLimiterConfiguration timeLimiter = DEFAULT_TIMEOUT;
private TokenCacheParameters tokenCacheParameters = DEFAULT_TOKEN_CACHE_PARAMETERS;
@Nullable
Expand Down Expand Up @@ -194,7 +215,23 @@ public Builder withTokenRetrievalParameters( @Nonnull final Map<String, String>
@Nonnull
public Builder withClientKeyStore( @Nonnull final KeyStore clientKeyStore )
{
this.clientKeyStore = clientKeyStore;
this.clientKeyStoreSupplier = () -> clientKeyStore;
return this;
}

/**
* Sets a supplier for the {@link KeyStore} to use for building an mTLS connection towards the <b>target
* system</b>. The supplier is invoked on every request, allowing the key store to be rotated at runtime. This
* {@link KeyStore} <b>is not used</b> to build an mTLS connection towards the OAuth2 token service.
*
* @param supplier
* A supplier providing the {@link KeyStore} for mTLS towards the <b>target system</b>.
* @return This {@link Builder}.
*/
@Nonnull
Builder withClientKeyStoreSupplier( @Nonnull final Supplier<KeyStore> supplier )
{
this.clientKeyStoreSupplier = supplier;
return this;
}

Expand Down Expand Up @@ -255,7 +292,7 @@ public OAuth2Options build()
skipTokenRetrieval,
new HashMap<>(additionalTokenRetrievalParameters),
timeLimiter,
clientKeyStore,
clientKeyStoreSupplier,
tokenCacheParameters,
btpTenantApiBaseUri);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,10 @@ HttpDestination toDestination(
destinationBuilder.headerProviders(headerProvider);
}

if( oAuth2Options.getClientKeyStore() != null ) {
if( oAuth2Options.getClientKeyStoreSupplier() != null ) {
log.debug("Securing communication to OAuth2 destination '{}' using mTLS.", idString);
destinationBuilder.keyStore(oAuth2Options.getClientKeyStore());
final var supplier = oAuth2Options.getClientKeyStoreSupplier();
destinationBuilder.keyStoreSupplier(() -> Option.of(supplier.get()));
}

return destinationBuilder.build();
Expand Down Expand Up @@ -292,12 +293,13 @@ HttpDestination toProxiedDestination(
destinationBuilder.headerProviders(headerProvider);
}

if( oAuth2Options.getClientKeyStore() != null ) {
if( oAuth2Options.getClientKeyStoreSupplier() != null ) {
log
.debug(
"Securing communication to OAuth2 proxy server for proxied destination '{}' using mTLS.",
destinationName);
destinationBuilder.keyStore(oAuth2Options.getClientKeyStore());
final var supplier = oAuth2Options.getClientKeyStoreSupplier();
destinationBuilder.keyStoreSupplier(() -> Option.of(supplier.get()));
}

// don't override the proxy URL if it has been set explicitly/manually already
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,16 @@ void testMutualTlsWithZeroTrustIdentityService()
.isInstanceOf(CloudPlatformException.class)
.describedAs("We are not mocking the ZTIS service here so this should fail")
.hasRootCauseInstanceOf(ServiceBindingAccessException.class);

// verify that the OAuth2Options holds a dynamic supplier (not a static KeyStore copy)
final OAuth2Options oAuth2Options = sut.getOAuth2Options();
assertThat(oAuth2Options.getClientKeyStoreSupplier())
.describedAs("ZTIS binding must use a dynamic KeyStore supplier")
.isNotNull();
assertThatThrownBy(oAuth2Options::getClientKeyStore)
.isInstanceOf(CloudPlatformException.class)
.describedAs("Supplier invocation must propagate ZTIS failure")
.hasRootCauseInstanceOf(ServiceBindingAccessException.class);
}

@Test
Expand Down
Loading