Skip to content

Commit 79820ef

Browse files
MatKuhrMatKuhr
authored
feat: [Connectivity] Improve Certificate Rotation with ZTIS for OAuth2TokenService (#1142)
1 parent d96432d commit 79820ef

File tree

12 files changed

+139
-61
lines changed

12 files changed

+139
-61
lines changed

‎cloudplatform/connectivity-oauth/pom.xml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,6 @@
135135
<groupId>org.apache.httpcomponents.core5</groupId>
136136
<artifactId>httpcore5</artifactId>
137137
</dependency>
138-
<dependency>
139-
<groupId>org.apache.commons</groupId>
140-
<artifactId>commons-lang3</artifactId>
141-
</dependency>
142138
<dependency>
143139
<groupId>com.google.guava</groupId>
144140
<artifactId>guava</artifactId>

‎cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/BtpServicePropertySuppliers.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ private void attachClientKeyStore( @Nonnull final OAuth2Options.Builder optionsB
267267
{
268268
final KeyStore maybeClientStore = getClientKeyStore();
269269
if( maybeClientStore != null ) {
270+
// 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
271+
// Token retrieval supports certificate rotation in place, but mTLS to the target system requires re-loading the destination instead.
270272
optionsBuilder.withClientKeyStore(maybeClientStore);
271273
}
272274
}
@@ -275,8 +277,8 @@ private void attachClientKeyStore( @Nonnull final OAuth2Options.Builder optionsB
275277
private KeyStore getClientKeyStore()
276278
{
277279
final ClientIdentity clientIdentity = getClientIdentity();
278-
if( clientIdentity instanceof ZtisClientIdentity ) {
279-
return ((ZtisClientIdentity) clientIdentity).getKeyStore();
280+
if( clientIdentity instanceof ZtisClientIdentity ztisClientIdentity ) {
281+
return ztisClientIdentity.getKeyStore();
280282
}
281283
if( !(clientIdentity instanceof ClientCertificate) ) {
282284
return null;

‎cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultOAuth2PropertySupplier.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import java.net.URI;
44
import java.net.URISyntaxException;
5-
import java.security.KeyStore;
65
import java.util.ArrayList;
76
import java.util.Arrays;
87
import java.util.Collections;
@@ -173,14 +172,7 @@ private ZtisClientIdentity getZtisIdentity( @Nonnull final String clientid )
173172
}
174173
final ZeroTrustIdentityService ztis = ZeroTrustIdentityService.getInstance();
175174

176-
final KeyStore keyStore;
177-
try {
178-
keyStore = ztis.getOrCreateKeyStore();
179-
}
180-
catch( final Exception e ) {
181-
throw new CloudPlatformException("Failed to load X509 certificate for credential type X509_ATTESTED.", e);
182-
}
183-
return new ZtisClientIdentity(clientid, keyStore);
175+
return new ZtisClientIdentity(clientid, ztis::getOrCreateKeyStore);
184176
}
185177

186178
@Nonnull

‎cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ class OAuth2Service
9797
OAuth2TokenService getTokenService( @Nullable final String tenantId )
9898
{
9999
final CacheKey key = CacheKey.fromIds(tenantId, null).append(identity);
100+
// ZTIS certificates rotate at runtime, thus we explicitly add the current KeyStore as cache key
101+
// once the certificate rotates, this produces a cache miss, ensuring we construct a new HTTP client with a new certificate
102+
if( identity instanceof final ZtisClientIdentity ztisIdentity ) {
103+
key.append(ztisIdentity.getKeyStore());
104+
}
100105
return tokenServiceCache.get(key, this::createTokenService);
101106
}
102107

‎cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/SecurityLibWorkarounds.java‎

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,16 @@
11
package com.sap.cloud.sdk.cloudplatform.connectivity;
22

3-
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationKeyStoreComparator.resolveCertificatesOnly;
4-
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationKeyStoreComparator.resolveKeyStoreHashCode;
5-
63
import java.security.KeyStore;
4+
import java.util.function.Supplier;
75

86
import javax.annotation.Nonnull;
97

10-
import org.apache.commons.lang3.builder.EqualsBuilder;
11-
import org.apache.commons.lang3.builder.HashCodeBuilder;
12-
8+
import com.sap.cloud.sdk.cloudplatform.exception.CloudPlatformException;
139
import com.sap.cloud.security.config.ClientIdentity;
1410

15-
import lombok.AllArgsConstructor;
16-
import lombok.Getter;
11+
import lombok.EqualsAndHashCode;
12+
import lombok.ToString;
13+
import lombok.Value;
1714

1815
final class SecurityLibWorkarounds
1916
{
@@ -22,44 +19,36 @@ private SecurityLibWorkarounds()
2219
throw new IllegalStateException("This utility class should never be instantiated.");
2320
}
2421

25-
@Getter
26-
@AllArgsConstructor
22+
@Value
2723
static class ZtisClientIdentity implements ClientIdentity
2824
{
2925
@Nonnull
30-
private final String id;
26+
String id;
27+
28+
// Exclude certificates from equals & hash code since they rotate dynamically at runtime
29+
// Instead, the OAuth2Service cache explicitly checks for outdated KeyStores
3130
@Nonnull
32-
private final KeyStore keyStore;
31+
@EqualsAndHashCode.Exclude
32+
@ToString.Exclude
33+
Supplier<KeyStore> keyStoreSource;
3334

3435
@Override
3536
public boolean isCertificateBased()
3637
{
3738
return true;
3839
}
3940

40-
// The identity will be used as cache key, so it's important we correctly implement equals/hashCode
41-
@Override
42-
public boolean equals( final Object obj )
41+
@Nonnull
42+
KeyStore getKeyStore()
4343
{
44-
if( this == obj ) {
45-
return true;
44+
try {
45+
return keyStoreSource.get();
4646
}
47-
48-
if( obj == null || getClass() != obj.getClass() ) {
49-
return false;
47+
catch( final Exception e ) {
48+
throw new CloudPlatformException(
49+
"Failed to load X509 certificate for credential type X509_ATTESTED.",
50+
e);
5051
}
51-
52-
final ZtisClientIdentity that = (ZtisClientIdentity) obj;
53-
return new EqualsBuilder()
54-
.append(id, that.id)
55-
.append(resolveCertificatesOnly(keyStore), resolveCertificatesOnly(that.keyStore))
56-
.isEquals();
57-
}
58-
59-
@Override
60-
public int hashCode()
61-
{
62-
return new HashCodeBuilder(41, 71).append(id).append(resolveKeyStoreHashCode(keyStore)).build();
6352
}
6453
}
6554
}

‎cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/BtpServicePropertySuppliersTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,8 +757,12 @@ void testMutualTlsWithZeroTrustIdentityService()
757757

758758
final OAuth2PropertySupplier sut = IDENTITY_AUTHENTICATION.resolve(options);
759759
assertThat(sut).isNotNull();
760+
assertThat(sut.getClientIdentity()).isInstanceOf(SecurityLibWorkarounds.ZtisClientIdentity.class);
760761

761-
assertThatThrownBy(sut::getClientIdentity)
762+
final var identity = (SecurityLibWorkarounds.ZtisClientIdentity) sut.getClientIdentity();
763+
assertThat(identity.getId()).isEqualTo("ias-client-id");
764+
765+
assertThatThrownBy(identity::getKeyStore)
762766
.isInstanceOf(CloudPlatformException.class)
763767
.describedAs("We are not mocking the ZTIS service here so this should fail")
764768
.hasRootCauseInstanceOf(ServiceBindingAccessException.class);

‎cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultOAuth2PropertySupplierTest.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,15 @@ void testCredentialTypeX509_ATTESTED()
206206
sut = new DefaultOAuth2PropertySupplier(options);
207207

208208
assertThat(sut.getCredentialType()).isEqualTo(CredentialType.X509_ATTESTED);
209-
assertThatThrownBy(sut::getClientIdentity)
209+
assertThat(sut).isNotNull();
210+
assertThat(sut.getClientIdentity()).isInstanceOf(SecurityLibWorkarounds.ZtisClientIdentity.class);
211+
212+
final var identity = (SecurityLibWorkarounds.ZtisClientIdentity) sut.getClientIdentity();
213+
assertThat(identity.getId()).isEqualTo("id");
214+
215+
assertThatThrownBy(identity::getKeyStore)
210216
.isInstanceOf(CloudPlatformException.class)
211-
.describedAs("We are not mocking the Zero Trust Identity Service here, so this should be a failure")
217+
.describedAs("We are not mocking the ZTIS service here so this should fail")
212218
.hasRootCauseInstanceOf(ServiceBindingAccessException.class);
213219
}
214220

‎cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ void testCreateHttpClientWithCertificateBasedClientIdentity()
159159
void testCreateHttpClientWithZtisClientIdentity()
160160
{
161161
final KeyStore keyStore = createEmptyKeyStore();
162-
final ClientIdentity ztisIdentity = new ZtisClientIdentity("ztis-client-id", keyStore);
162+
final ClientIdentity ztisIdentity = new ZtisClientIdentity("ztis-client-id", () -> keyStore);
163163

164164
// ZtisClientIdentity is certificate-based but doesn't implement certificate methods properly
165165
// This should fail with certificate validation error
@@ -175,7 +175,7 @@ void testCreateHttpClientWithZtisClientIdentityAndExplicitKeyStore()
175175
{
176176
final KeyStore embeddedKeyStore = createEmptyKeyStore();
177177
final KeyStore explicitKeyStore = createEmptyKeyStore();
178-
final ClientIdentity ztisIdentity = new ZtisClientIdentity("ztis-client-id", embeddedKeyStore);
178+
final ClientIdentity ztisIdentity = new ZtisClientIdentity("ztis-client-id", () -> embeddedKeyStore);
179179

180180
final CloseableHttpClient result =
181181
HttpClient5OAuth2TokenService.createHttpClient(ztisIdentity, explicitKeyStore);

‎cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2IntegrationTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ void testIasFlowWithZeroTrustAndSubscriberTenant()
216216
{
217217
final KeyStore ks = KeyStore.getInstance("JKS");
218218
ks.load(null, null);
219-
final ClientIdentity identity = new SecurityLibWorkarounds.ZtisClientIdentity("myClientId", ks);
219+
final ClientIdentity identity = new SecurityLibWorkarounds.ZtisClientIdentity("myClientId", () -> ks);
220220

221221
stubFor(
222222
post("/oauth2/token")

‎cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceTest.java‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static org.mockito.Mockito.doReturn;
1919
import static org.mockito.Mockito.mock;
2020
import static org.mockito.Mockito.spy;
21+
import static org.mockito.Mockito.when;
2122

2223
import java.io.IOException;
2324
import java.net.URI;
@@ -28,6 +29,7 @@
2829
import java.time.Duration;
2930
import java.util.List;
3031
import java.util.Map;
32+
import java.util.function.Supplier;
3133

3234
import org.junit.jupiter.api.BeforeEach;
3335
import org.junit.jupiter.api.Test;
@@ -401,15 +403,47 @@ void testZeroTrustClientIdentity()
401403
{
402404
final KeyStore ks = KeyStore.getInstance("JKS");
403405
ks.load(null, null);
404-
ClientIdentity identity = new ZtisClientIdentity("id", ks);
406+
ClientIdentity identity = new ZtisClientIdentity("id", () -> ks);
405407
OAuth2Service service = OAuth2Service.builder().withTokenUri(SERVER_1.baseUrl()).withIdentity(identity).build();
406408

407409
final OAuth2TokenService result = service.getTokenService(null);
408410
assertThat(result).isSameAs(service.getTokenService(null));
409411

410-
identity = new ZtisClientIdentity("other-id", ks);
412+
identity = new ZtisClientIdentity("other-id", () -> ks);
411413
service = OAuth2Service.builder().withTokenUri(SERVER_1.baseUrl()).withIdentity(identity).build();
412414

413415
assertThat(result).isNotSameAs(service.getTokenService(null));
414416
}
417+
418+
@Test
419+
@SneakyThrows
420+
void testZeroTrustCertificateRotationCausesCacheMiss()
421+
{
422+
// we need to use actual KeyStores here because the code will build an HTTP Client and mocks don't suffice
423+
final KeyStore ks1 = KeyStore.getInstance("JKS");
424+
final KeyStore ks2 = KeyStore.getInstance("JKS");
425+
ks1.load(null, null);
426+
ks2.load(null, null);
427+
428+
assertThat(ks1).describedAs("Sanity check: objects should be different").isNotSameAs(ks2).isNotEqualTo(ks2);
429+
430+
@SuppressWarnings( "unchecked" )
431+
final Supplier<KeyStore> mockZtis = mock(Supplier.class);
432+
when(mockZtis.get()).thenReturn(ks1);
433+
434+
final ZtisClientIdentity identity = new ZtisClientIdentity("id", mockZtis);
435+
436+
final OAuth2Service service =
437+
OAuth2Service.builder().withTokenUri(SERVER_1.baseUrl()).withIdentity(identity).build();
438+
439+
// Before rotation: same KeyStore → cache hit
440+
final OAuth2TokenService tokenService1 = service.getTokenService(null);
441+
assertThat(tokenService1).isSameAs(service.getTokenService(null));
442+
443+
when(mockZtis.get()).thenReturn(ks2);
444+
445+
// After rotation: different KeyStore → cache miss → new token service with new certificate
446+
final OAuth2TokenService tokenService2 = service.getTokenService(null);
447+
assertThat(tokenService2).isNotSameAs(tokenService1);
448+
}
415449
}

0 commit comments

Comments
 (0)