Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix java security test compatibility #1379

Merged
merged 12 commits into from
Dec 6, 2023
Merged
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
3 changes: 3 additions & 0 deletions .github/workflows/maven-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
---
name: Maven Build main

env:
NVD_API_KEY: ${{ secrets.NVD_API_KEY }}

on:
push:
branches: [ main ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.sap.cloud.security.token.validation;

/**
* This interface is for INTERNAL usage only to add backward-compatibility for test credentials with trusted domain 'localhost' to the issuer validation.
*/
public interface TestIssuerValidator {
boolean isValidIssuer(String issuer);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.sap.cloud.security.token.validation;

/**
* This interface is for INTERNAL usage only to add backward-compatibility for test credentials with uaadomain 'localhost' during JKU construction.
*/
public interface XsuaaJkuFactory {
String create(String token);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@
import com.sap.cloud.security.config.Environments;
import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
import com.sap.cloud.security.config.Service;
import com.sap.cloud.security.config.ServiceConstants;
import com.sap.cloud.security.test.SecurityTestRule;
import com.sap.cloud.security.token.Token;
import com.sap.cloud.security.token.validation.CombiningValidator;
import com.sap.cloud.security.token.validation.ValidationResult;
import com.sap.cloud.security.token.validation.validators.JwtValidatorBuilder;
import org.junit.ClassRule;
import org.junit.Test;
import org.mockito.Mockito;

import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -33,16 +31,8 @@ public class XsuaaMultipleBindingsIntegrationTest {
public void createToken_integrationTest_tokenValidation() {
Token token = rule.getPreconfiguredJwtGenerator().createToken();
OAuth2ServiceConfiguration configuration = Environments.readFromInput(XsuaaMultipleBindingsIntegrationTest.class.getResourceAsStream("/vcap_services-multiple.json")).getXsuaaConfiguration();
OAuth2ServiceConfiguration mockConfig = Mockito.mock(OAuth2ServiceConfiguration.class);
Mockito.when(mockConfig.getClientId()).thenReturn(configuration.getClientId());
Mockito.when(mockConfig.getDomains()).thenReturn(configuration.getDomains());
Mockito.when(mockConfig.getUrl()).thenReturn(configuration.getUrl());
Mockito.when(mockConfig.hasProperty(ServiceConstants.XSUAA.APP_ID)).thenReturn(configuration.hasProperty(ServiceConstants.XSUAA.APP_ID));
Mockito.when(mockConfig.getProperty(ServiceConstants.XSUAA.APP_ID)).thenReturn(configuration.getProperty(ServiceConstants.XSUAA.APP_ID));
Mockito.when(mockConfig.getProperty(ServiceConstants.XSUAA.UAA_DOMAIN)).thenReturn(rule.getWireMockServer().baseUrl());
Mockito.when(mockConfig.getService()).thenReturn(configuration.getService());

CombiningValidator<Token> tokenValidator = JwtValidatorBuilder.getInstance(mockConfig).build();

CombiningValidator<Token> tokenValidator = JwtValidatorBuilder.getInstance(configuration).build();

ValidationResult result = tokenValidator.validate(token);
assertThat(result.isValid()).isTrue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ class JavaSSRFAttackTest {
@ParameterizedTest
@CsvSource({
"http://localhost:4242/token_keys, true",
"http://localhost:4242/[email protected]/token_keys, false",
"http://localhost:4242/token_keys///malicious.ondemand.com/token_keys, false",
"http://malicious.ondemand.com@localhost:4242/token_keys, true"
})
void maliciousPartOfJwksIsNotUsedToObtainToken(String jwksUrl, boolean isValid)
throws IOException, NoSuchAlgorithmException, InvalidKeySpecException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ class SpringSSRFAttackTest {
@ParameterizedTest
@CsvSource({
"http://localhost:4242/[email protected]/token_keys, false",
"http://malicious.ondemand.com@localhost:4242/token_keys, true",
"http://localhost:4242/token_keys///malicious.ondemand.com/token_keys, false",
})
void maliciousPartOfJwksIsNotUsedToObtainToken(String jwksUrl, boolean isValid)
Expand Down Expand Up @@ -93,7 +92,7 @@ void maliciousPartOfJwksIsNotUsedToObtainToken(String jwksUrl, boolean isValid)

private XsuaaCredentials createXsuaaCredentials() {
XsuaaCredentials xsuaaCredentials = new XsuaaCredentials();
xsuaaCredentials.setUaaDomain(extension.getContext().getWireMockServer().baseUrl());
xsuaaCredentials.setUaaDomain(SecurityTest.DEFAULT_DOMAIN);
xsuaaCredentials.setClientId(SecurityTest.DEFAULT_CLIENT_ID);
xsuaaCredentials.setXsAppName(SecurityTest.DEFAULT_APP_ID);
return xsuaaCredentials;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private CombiningValidator<Token> createOnlineTokenValidator() {

private OAuth2ServiceConfigurationBuilder createConfigurationBuilder() {
return OAuth2ServiceConfigurationBuilder.forService(XSUAA)
.withProperty(ServiceConstants.XSUAA.UAA_DOMAIN, securityTest.getWireMockServer().baseUrl())
.withProperty(ServiceConstants.XSUAA.UAA_DOMAIN, SecurityTest.DEFAULT_DOMAIN)
.withProperty(ServiceConstants.XSUAA.APP_ID, SecurityTest.DEFAULT_APP_ID)
.withClientId(SecurityTest.DEFAULT_CLIENT_ID);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ private JwtDecoder createOnlineJwtDecoder() {

private OAuth2ServiceConfigurationBuilder createXsuaaConfigurationBuilder() {
return OAuth2ServiceConfigurationBuilder.forService(XSUAA)
.withProperty(ServiceConstants.XSUAA.UAA_DOMAIN, securityTest.getWireMockServer().baseUrl())
.withProperty(ServiceConstants.XSUAA.UAA_DOMAIN, SecurityTest.DEFAULT_DOMAIN)
.withProperty(ServiceConstants.XSUAA.APP_ID, SecurityTest.DEFAULT_APP_ID)
.withClientId(SecurityTest.DEFAULT_CLIENT_ID);
}

private OAuth2ServiceConfigurationBuilder createIasConfigurationBuilder() {
return OAuth2ServiceConfigurationBuilder.forService(IAS)
.withDomains(securityIasTest.getWireMockServer().baseUrl())
.withDomains(SecurityTest.DEFAULT_DOMAIN)
.withClientId(SecurityTest.DEFAULT_CLIENT_ID);
}
}
Expand Down
4 changes: 2 additions & 2 deletions java-security-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@
<artifactId>jetty-util</artifactId>
</dependency>
<dependency>
<groupId>com.github.tomakehurst</groupId>
<artifactId>wiremock</artifactId>
<groupId>org.wiremock</groupId>
<artifactId>wiremock-standalone</artifactId>
</dependency>
<dependency>
<groupId>com.github.spotbugs</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

import javax.annotation.Nullable;
import java.io.IOException;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.security.NoSuchAlgorithmException;
import java.security.interfaces.RSAPublicKey;
Expand Down Expand Up @@ -190,7 +191,7 @@ public OAuth2ServiceConfigurationBuilder getOAuth2ServiceConfigurationBuilderFro
OAuth2ServiceConfigurationBuilder builder = ServiceBindingMapper.mapToOAuth2ServiceConfigurationBuilder(binding);
if (builder != null) {
// adjust domain and URL of the config to fit the mocked service instance
builder = builder.withDomains(issuerUrl).withUrl(issuerUrl);
builder = builder.withDomains(URI.create(issuerUrl).getHost()).withUrl(issuerUrl);

if(Objects.equals(Service.from(binding.getServiceName().get()), XSUAA)) {
builder.withProperty(ServiceConstants.XSUAA.UAA_DOMAIN, wireMockServer.baseUrl());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.sap.cloud.security.token.validation;

/**
* LocalhostIssuerValidator brings backward-compatibility for test credentials in consumer applications written before 2.17.0 that are used to validate java-security-test tokens.
* This is necessary for successful validation of localhost issuers that include a port when 'localhost' is defined as trusted domain without port in the service credentials.
* This class MUST NOT be loaded outside test scope and MUST be the ONLY implementation of {@link TestIssuerValidator}.
*/
public class LocalhostIssuerValidator implements TestIssuerValidator {

@Override
public boolean isValidIssuer(String issuer) {
return issuer.startsWith("http://localhost") || issuer.startsWith("https://localhost");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.sap.cloud.security.token.validation;

import com.sap.cloud.security.token.Token;
import com.sap.cloud.security.token.TokenHeader;

/**
* XsuaaLocalhostJkuFactory brings backward-compatibility for test credentials in consumer applications written before 2.17.0 that are used to validate java-security-test tokens.
* This is necessary for successful JKU construction when 'localhost' is defined as uaadomain in the service credentials.
* This class MUST NOT be loaded outside test scope and MUST be the ONLY implementation of {@link XsuaaJkuFactory}.
*/
public class XsuaaLocalhostJkuFactory implements XsuaaJkuFactory {

@Override
public String create(String jwt) {
Token token = Token.create(jwt);
String tokenJku = (String) token.getHeaders().get(TokenHeader.JWKS_URL);

if (tokenJku.contains("localhost") || tokenJku.contains("127.0.0.1")) {
return tokenJku;
}

throw new IllegalArgumentException("JKU is not trusted because it does not target localhost.");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.sap.cloud.security.token.validation.LocalhostIssuerValidator
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.sap.cloud.security.token.validation.XsuaaLocalhostJkuFactory
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
import com.sap.cloud.security.json.JsonParsingException;
import com.sap.cloud.security.token.Token;
import com.sap.cloud.security.token.validation.TestIssuerValidator;
import com.sap.cloud.security.token.validation.ValidationResult;
import com.sap.cloud.security.token.validation.Validator;
import org.slf4j.Logger;
Expand All @@ -17,6 +18,8 @@
import java.net.URL;
import java.util.List;
import java.util.Objects;
import java.util.ServiceConfigurationError;
import java.util.ServiceLoader;
import java.util.regex.Pattern;

import static com.sap.cloud.security.token.validation.ValidationResults.createInvalid;
Expand All @@ -38,9 +41,36 @@
* These checks are a prerequisite for using the `JwtSignatureValidator`.
*/
class JwtIssuerValidator implements Validator<Token> {
protected static final Logger LOGGER = LoggerFactory.getLogger(JwtIssuerValidator.class);

/*
* The following validator brings backward-compatibility for test credentials in consumer applications written before 2.17.0 that are used to validate java-security-test tokens.
* This is necessary for successful validation of localhost issuers that include a port when 'localhost' is defined as trusted domain without port in the service credentials.
* Implementations of this interface absolutely MUST NOT be supplied outside test scope and MUST NOT be used for any other purpose to preserve application security.
*/
static TestIssuerValidator localhostIssuerValidator;
static {
tryLoadingLocalhostIssuerValidator();
}

private static void tryLoadingLocalhostIssuerValidator() {
ServiceLoader<TestIssuerValidator> validators;
try {
validators = ServiceLoader.load(TestIssuerValidator.class);
} catch (Exception | ServiceConfigurationError e) {
LOGGER.warn("Unexpected failure while loading TestIssuerValidator service providers: {}", e.getMessage());
return;
}

for (TestIssuerValidator v : validators) {
localhostIssuerValidator = v;
break;
}
LOGGER.debug("loaded TestIssuerValidator service providers: {}. Using first one: {}.", validators, localhostIssuerValidator);
}

protected static final String HTTPS_SCHEME = "https://";
private final List<String> domains;
protected final Logger logger = LoggerFactory.getLogger(getClass());

/**
* Creates instance of Issuer validation using the given domains provided by the
Expand Down Expand Up @@ -70,6 +100,7 @@ public ValidationResult validate(Token token) {
}

String issuerUrl = issuer.startsWith(HTTPS_SCHEME) || issuer.startsWith("http://localhost") ? issuer : HTTPS_SCHEME + issuer;

try {
new URL(issuerUrl);
} catch (MalformedURLException e) {
Expand All @@ -83,6 +114,11 @@ public ValidationResult validate(Token token) {
if(Objects.equals(d, issuerDomain) || issuerDomain.matches(validSubdomainPattern)) {
return createValid();
}

if("localhost".equals(d) && localhostIssuerValidator != null && localhostIssuerValidator.isValidIssuer(issuer)) {
LOGGER.debug("Accepting {} as valid issuer on trusted domain 'localhost' for backward-compatibility with java-security-test.", issuer);
return createValid();
}
}

return createInvalid("Issuer {} was not a trusted domain or a subdomain of the trusted domains {}.", issuer, domains);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
import com.sap.cloud.security.config.ServiceConstants;
import com.sap.cloud.security.token.Token;
import com.sap.cloud.security.token.validation.XsuaaJkuFactory;
import com.sap.cloud.security.xsuaa.client.OAuth2ServiceException;
import com.sap.cloud.security.xsuaa.http.HttpHeaders;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.net.URI;
import java.security.NoSuchAlgorithmException;
import java.security.PublicKey;
import java.security.spec.InvalidKeySpecException;
import java.util.Collections;
import java.util.Map;
import java.util.*;

import static com.sap.cloud.security.config.ServiceConstants.XSUAA.UAA_DOMAIN;
import static com.sap.cloud.security.token.validation.validators.JsonWebKeyConstants.KEY_ID_VALUE_LEGACY;
Expand All @@ -21,6 +23,24 @@
* Jwt Signature validator for Access tokens issued by Xsuaa service
*/
class XsuaaJwtSignatureValidator extends JwtSignatureValidator {
public static final Logger LOGGER = LoggerFactory.getLogger(XsuaaJwtSignatureValidator.class);

/*
* The following list of factories brings backward-compatibility for test credentials in consumer applications written before 2.17.0 that are used to validate java-security-test tokens.
* This is necessary to construct the correct JKU when 'localhost' without port is defined as uaadomain in the service credentials.
* Implementations of this interface absolutely MUST NOT be supplied outside test scope and MUST NOT be used for any other purpose to preserve application security.
*/
List<XsuaaJkuFactory> jkuFactories = new ArrayList<>() {
{
try {
ServiceLoader.load(XsuaaJkuFactory.class).forEach(this::add);
LOGGER.debug("loaded XsuaaJkuFactory service providers: {}", this);
} catch (Exception | ServiceConfigurationError e) {
LOGGER.warn("Unexpected failure while loading XsuaaJkuFactory service providers: {}", e.getMessage());
}
}
};

XsuaaJwtSignatureValidator(OAuth2ServiceConfiguration configuration, OAuth2TokenKeyServiceWithCache tokenKeyService, OidcConfigurationServiceWithCache oidcConfigurationService) {
super(configuration, tokenKeyService, oidcConfigurationService);
}
Expand Down Expand Up @@ -57,7 +77,17 @@ private PublicKey fetchPublicKey(Token token, JwtSignatureAlgorithm algorithm) t
}

String zidQueryParam = composeZidQueryParameter(token);
String jwksUri = configuration.isLegacyMode() ? configuration.getUrl() + "/token_keys" : configuration.getProperty(UAA_DOMAIN) + "/token_keys" + zidQueryParam;

String jwksUri;
if (jkuFactories.isEmpty()) {
jwksUri = configuration.isLegacyMode()
? configuration.getUrl() + "/token_keys"
: configuration.getProperty(UAA_DOMAIN) + "/token_keys" + zidQueryParam;
} else {
LOGGER.info("Loaded custom JKU factory");
jwksUri = jkuFactories.get(0).create(token.getTokenValue());
}

URI uri = URI.create(jwksUri);
uri = uri.isAbsolute() ? uri : URI.create("https://" + jwksUri);
Map<String, String> params = Collections.singletonMap(HttpHeaders.X_ZID, token.getAppTid());
Expand Down
Loading