Skip to content

Commit

Permalink
Verify challenge resource matches request domain (#31045)
Browse files Browse the repository at this point in the history
* Updated KeyVaultCredentialPolicy to verify the challenge resource matches the request domain.

* Updated JavaDoc.

* Updated CHANGELOGs, READMEs and POMs.

* Fixed CheckStyle issue.

* Removed typo from CHANGELOG.

* Added test for when `verifyChallengeResource` is set to `false`.

* Added error message.

* Changed `verifyChallengeResource()` (defaulted to `true`) to `disableChallengeResourceVerification()` (defaults to `false`) in client builders. Functionality remains unchanged.

* Fixed JavaDoc error.

* Changed `disableChallengeResourceVerification()` to take no args instead of a `boolean`.

* Updated CHANGELOGs.

* Updated CHANGELOG dates.

* Removed JavaDoc references to KeyVaultCredentialPolicy.
  • Loading branch information
vcolin7 authored Sep 20, 2022
1 parent 6e9d6ac commit 141d2a6
Show file tree
Hide file tree
Showing 41 changed files with 607 additions and 322 deletions.
8 changes: 4 additions & 4 deletions eng/jacoco-test-coverage/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,12 @@
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-administration</artifactId>
<version>4.1.6</version> <!-- {x-version-update;com.azure:azure-security-keyvault-administration;current} -->
<version>4.2.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-administration;current} -->
</dependency>
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-certificates</artifactId>
<version>4.3.6</version> <!-- {x-version-update;com.azure:azure-security-keyvault-certificates;current} -->
<version>4.4.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-certificates;current} -->
</dependency>
<dependency>
<groupId>com.azure</groupId>
Expand All @@ -298,12 +298,12 @@
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-keys</artifactId>
<version>4.4.7</version> <!-- {x-version-update;com.azure:azure-security-keyvault-keys;current} -->
<version>4.5.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-keys;current} -->
</dependency>
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-secrets</artifactId>
<version>4.4.7</version> <!-- {x-version-update;com.azure:azure-security-keyvault-secrets;current} -->
<version>4.5.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-secrets;current} -->
</dependency>
<dependency>
<groupId>com.azure</groupId>
Expand Down
8 changes: 4 additions & 4 deletions eng/versioning/version_client.txt
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ com.azure:azure-search-documents;11.5.0;11.6.0-beta.2
com.azure:azure-search-perf;1.0.0-beta.1;1.0.0-beta.1
com.azure:azure-security-attestation;1.1.6;1.2.0-beta.1
com.azure:azure-security-confidentialledger;1.0.1;1.1.0-beta.1
com.azure:azure-security-keyvault-administration;4.1.5;4.1.6
com.azure:azure-security-keyvault-certificates;4.3.5;4.3.6
com.azure:azure-security-keyvault-administration;4.1.5;4.2.0
com.azure:azure-security-keyvault-certificates;4.3.5;4.4.0
com.azure:azure-security-keyvault-jca;2.7.0;2.8.0-beta.1
com.azure:azure-security-test-keyvault-jca;1.0.0;1.0.0
com.azure:azure-security-keyvault-keys;4.4.6;4.4.7
com.azure:azure-security-keyvault-secrets;4.4.6;4.4.7
com.azure:azure-security-keyvault-keys;4.4.6;4.5.0
com.azure:azure-security-keyvault-secrets;4.4.6;4.5.0
com.azure:azure-security-keyvault-perf;1.0.0-beta.1;1.0.0-beta.1
com.azure:azure-sdk-template;1.1.1234;1.2.2-beta.1
com.azure:azure-sdk-template-two;1.0.0-beta.1;1.0.0-beta.1
Expand Down
6 changes: 3 additions & 3 deletions sdk/e2e/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,17 @@
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-keys</artifactId>
<version>4.4.7</version> <!-- {x-version-update;com.azure:azure-security-keyvault-keys;current} -->
<version>4.5.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-keys;current} -->
</dependency>
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-secrets</artifactId>
<version>4.4.7</version> <!-- {x-version-update;com.azure:azure-security-keyvault-secrets;current} -->
<version>4.5.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-secrets;current} -->
</dependency>
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-certificates</artifactId>
<version>4.3.6</version> <!-- {x-version-update;com.azure:azure-security-keyvault-certificates;current} -->
<version>4.4.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-certificates;current} -->
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Release History

## 4.1.6 (2022-09-19)
## 4.2.0 (2022-09-20)

### Breaking Changes
- Made it so that we verify that the challenge resource matches the vault domain by default. This should affect few customers who can use the `disableChallengeResourceVerification()` method in client builders to disable this functionality. See https://aka.ms/azsdk/blog/vault-uri for more information.

### Other Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ If you want to take dependency on a particular version of the library that is no
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-administration</artifactId>
<version>4.1.5</version>
<version>4.2.0</version>
</dependency>
```
[//]: # ({x-version-update-end})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

<groupId>com.azure</groupId>
<artifactId>azure-security-keyvault-administration</artifactId>
<version>4.1.6</version> <!-- {x-version-update;com.azure:azure-security-keyvault-administration;current} -->
<version>4.2.0</version> <!-- {x-version-update;com.azure:azure-security-keyvault-administration;current} -->

<name>Microsoft Azure client library for KeyVault Administration</name>
<description>This module contains client library for Microsoft Azure KeyVault Administration.</description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public final class KeyVaultAccessControlClientBuilder implements
private Configuration configuration;
private ClientOptions clientOptions;
private KeyVaultAdministrationServiceVersion serviceVersion;
private boolean disableChallengeResourceVerification = false;

/**
* Creates a {@link KeyVaultAccessControlClientBuilder} instance that is able to configure and construct
Expand Down Expand Up @@ -183,7 +184,7 @@ public KeyVaultAccessControlAsyncClient buildAsyncClient() {
// Add retry policy.
policies.add(ClientBuilderUtil.validateAndGetRetryPolicy(retryPolicy, retryOptions));

policies.add(new KeyVaultCredentialPolicy(credential));
policies.add(new KeyVaultCredentialPolicy(credential, disableChallengeResourceVerification));

// Add per retry additional policies.
policies.addAll(perRetryPolicies);
Expand All @@ -200,7 +201,9 @@ public KeyVaultAccessControlAsyncClient buildAsyncClient() {
}

/**
* Sets the URL to the Key Vault on which the client operates. Appears as "DNS Name" in the Azure portal.
* Sets the URL to the Key Vault on which the client operates. Appears as "DNS Name" in the Azure portal. You should
* validate that this URL references a valid Key Vault or Managed HSM resource.
* Refer to the following <a href=https://aka.ms/azsdk/blog/vault-uri>documentation</a> for details.
*
* @param vaultUrl The vault URL is used as destination on Azure to send requests to.
*
Expand Down Expand Up @@ -438,6 +441,18 @@ public KeyVaultAccessControlClientBuilder serviceVersion(KeyVaultAdministrationS
return this;
}

/**
* Disables verifying if the authentication challenge resource matches the Key Vault or Managed HSM domain. This
* verification is performed by default.
*
* @return The updated {@link KeyVaultAccessControlClientBuilder} object.
*/
public KeyVaultAccessControlClientBuilder disableChallengeResourceVerification() {
this.disableChallengeResourceVerification = true;

return this;
}

private URL getBuildEndpoint(Configuration configuration) {
if (vaultUrl != null) {
return vaultUrl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public final class KeyVaultBackupClientBuilder implements
private Configuration configuration;
private ClientOptions clientOptions;
private KeyVaultAdministrationServiceVersion serviceVersion;
private boolean disableChallengeResourceVerification = false;

/**
* Creates a {@link KeyVaultBackupClientBuilder} instance that is able to configure and construct instances of
Expand Down Expand Up @@ -182,7 +183,7 @@ public KeyVaultBackupAsyncClient buildAsyncClient() {
// Add retry policy.
policies.add(ClientBuilderUtil.validateAndGetRetryPolicy(retryPolicy, retryOptions));

policies.add(new KeyVaultCredentialPolicy(credential));
policies.add(new KeyVaultCredentialPolicy(credential, disableChallengeResourceVerification));

// Add per retry additional policies.
policies.addAll(perRetryPolicies);
Expand All @@ -199,7 +200,9 @@ public KeyVaultBackupAsyncClient buildAsyncClient() {
}

/**
* Sets the URL to the Key Vault on which the client operates. Appears as "DNS Name" in the Azure portal.
* Sets the URL to the Key Vault on which the client operates. Appears as "DNS Name" in the Azure portal. You should
* validate that this URL references a valid Key Vault or Managed HSM resource.
* Refer to the following <a href=https://aka.ms/azsdk/blog/vault-uri>documentation</a> for details.
*
* @param vaultUrl The vault URL is used as destination on Azure to send requests to.
*
Expand Down Expand Up @@ -437,6 +440,18 @@ public KeyVaultBackupClientBuilder serviceVersion(KeyVaultAdministrationServiceV
return this;
}

/**
* Disables verifying if the authentication challenge resource matches the Key Vault or Managed HSM domain. This
* verification is performed by default.
*
* @return The updated {@link KeyVaultBackupClientBuilder} object.
*/
public KeyVaultBackupClientBuilder disableChallengeResourceVerification() {
this.disableChallengeResourceVerification = true;

return this;
}

private URL getBuildEndpoint(Configuration configuration) {
if (vaultUrl != null) {
return vaultUrl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.util.BinaryData;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.logging.ClientLogger;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

Expand All @@ -32,21 +33,25 @@
* @see TokenCredential
*/
public class KeyVaultCredentialPolicy extends BearerTokenAuthenticationPolicy {
private static final ClientLogger LOGGER = new ClientLogger(KeyVaultCredentialPolicy.class);
private static final String BEARER_TOKEN_PREFIX = "Bearer ";
private static final String CONTENT_LENGTH_HEADER = "Content-Length";
private static final String KEY_VAULT_STASHED_CONTENT_KEY = "KeyVaultCredentialPolicyStashedBody";
private static final String KEY_VAULT_STASHED_CONTENT_LENGTH_KEY = "KeyVaultCredentialPolicyStashedContentLength";
private static final String WWW_AUTHENTICATE = "WWW-Authenticate";
private static final ConcurrentMap<String, ChallengeParameters> CHALLENGE_CACHE = new ConcurrentHashMap<>();
private ChallengeParameters challenge;
private final boolean disableChallengeResourceVerification;

/**
* Creates a {@link KeyVaultCredentialPolicy}.
*
* @param credential The token credential to authenticate the request.
*/
public KeyVaultCredentialPolicy(TokenCredential credential) {
public KeyVaultCredentialPolicy(TokenCredential credential, boolean disableChallengeResourceVerification) {
super(credential);

this.disableChallengeResourceVerification = disableChallengeResourceVerification;
}

/**
Expand Down Expand Up @@ -90,6 +95,7 @@ private static boolean isBearerChallenge(String authenticateHeader, String authC
return (!CoreUtils.isNullOrEmpty(authenticateHeader)
&& authenticateHeader.toLowerCase(Locale.ROOT).startsWith(authChallengePrefix.toLowerCase(Locale.ROOT)));
}

@Override
public Mono<Void> authorizeRequest(HttpPipelineCallContext context) {
return Mono.defer(() -> {
Expand Down Expand Up @@ -161,6 +167,17 @@ public Mono<Boolean> authorizeRequestOnChallenge(HttpPipelineCallContext context
return Mono.just(false);
}
} else {
if (!disableChallengeResourceVerification) {
if (!isChallengeResourceValid(request, scope)) {
throw LOGGER.logExceptionAsError(
new RuntimeException(String.format(
"The challenge resource '%s' does not match the requested domain. If you wish to "
+ "disable this check for your client, pass 'true' to the SecretClientBuilder"
+ ".disableChallengeResourceVerification() method when building it. See "
+ "https://aka.ms/azsdk/blog/vault-uri for more information.", scope)));
}
}

String authorization = challengeAttributes.get("authorization");

if (authorization == null) {
Expand All @@ -172,8 +189,9 @@ public Mono<Boolean> authorizeRequestOnChallenge(HttpPipelineCallContext context
try {
authorizationUri = new URI(authorization);
} catch (URISyntaxException e) {
// The challenge authorization URI is invalid.
return Mono.just(false);
throw LOGGER.logExceptionAsError(
new RuntimeException(
String.format("The challenge authorization URI '%s' is invalid.", authorization), e));
}

this.challenge = new ChallengeParameters(authorizationUri, new String[] {scope});
Expand Down Expand Up @@ -257,6 +275,17 @@ public boolean authorizeRequestOnChallengeSync(HttpPipelineCallContext context,
return false;
}
} else {
if (!disableChallengeResourceVerification) {
if (!isChallengeResourceValid(request, scope)) {
throw LOGGER.logExceptionAsError(
new RuntimeException(String.format(
"The challenge resource '%s' does not match the requested domain. If you wish to disable "
+ "this check for your client, pass 'true' to the SecretClientBuilder"
+ ".disableChallengeResourceVerification() method when building it. See "
+ "https://aka.ms/azsdk/blog/vault-uri for more information.", scope)));
}
}

String authorization = challengeAttributes.get("authorization");

if (authorization == null) {
Expand All @@ -268,8 +297,9 @@ public boolean authorizeRequestOnChallengeSync(HttpPipelineCallContext context,
try {
authorizationUri = new URI(authorization);
} catch (URISyntaxException e) {
// The challenge authorization URI is invalid.
return false;
throw LOGGER.logExceptionAsError(
new RuntimeException(
String.format("The challenge authorization URI '%s' is invalid.", authorization), e));
}

this.challenge = new ChallengeParameters(authorizationUri, new String[] {scope});
Expand All @@ -285,10 +315,6 @@ public boolean authorizeRequestOnChallengeSync(HttpPipelineCallContext context,
return true;
}

public static void clearCache() {
CHALLENGE_CACHE.clear();
}

private static class ChallengeParameters {
private final URI authorizationUri;
private final String tenantId;
Expand Down Expand Up @@ -323,10 +349,15 @@ public String getTenantId() {
}
}

public static void clearCache() {
CHALLENGE_CACHE.clear();
}

/**
* Gets the host name and port of the Key Vault or Managed HSM endpoint.
*
* @param request The {@link HttpRequest} to extract the host name and port from.
*
* @return The host name and port of the Key Vault or Managed HSM endpoint.
*/
private static String getRequestAuthority(HttpRequest request) {
Expand All @@ -341,4 +372,20 @@ private static String getRequestAuthority(HttpRequest request) {

return authority;
}

private static boolean isChallengeResourceValid(HttpRequest request, String scope) {
final URI scopeUri;

try {
scopeUri = new URI(scope);
} catch (URISyntaxException e) {
throw LOGGER.logExceptionAsError(
new RuntimeException(
String.format("The challenge resource '%s' is not a valid URI.", scope), e));
}

// Returns false if the host specified in the scope does not match the requested domain.
return request.getUrl().getHost().toLowerCase(Locale.ROOT)
.endsWith("." + scopeUri.getHost().toLowerCase(Locale.ROOT));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ protected List<HttpPipelinePolicy> getPolicies() {
policies.add(new RetryPolicy(strategy));

if (credential != null) {
policies.add(new KeyVaultCredentialPolicy(credential));
policies.add(new KeyVaultCredentialPolicy(credential, false));
}

HttpPolicyProviders.addAfterRetryPolicies(policies);
Expand Down
Loading

0 comments on commit 141d2a6

Please sign in to comment.