Skip to content

Commit

Permalink
fix: Removed url pattern validation for google urls in external accou…
Browse files Browse the repository at this point in the history 
…nt credential configurations (#1150)

* fix: Removed url pattern validation for google urls, added readme change to explain risk.

* fix: formatting
  • Loading branch information
@aeitzman
aeitzman committed Feb 11, 2023
1 parent 5bf606b commit 35495b1
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 72 deletions.
7 changes: 7 additions & 0 deletions README.md
View file
Expand Up @@ -728,6 +728,13 @@ ExternalAccountCredentials credentials =
ExternalAccountCredentials.fromStream(new FileInputStream("/path/to/credentials.json"));
```

##### Security Considerations
Note that this library does not perform any validation on the token_url, token_info_url,
or service_account_impersonation_url fields of the credential configuration.
It is not recommended to use a credential configuration that you did not
generate with the gcloud CLI unless you verify that the URL fields point to a
googleapis.com domain.

### Downscoping with Credential Access Boundaries

[Downscoping with Credential Access Boundaries](https://cloud.google.com/iam/docs/downscoping-short-lived-credentials)
Expand Down
34 changes: 5 additions & 29 deletions oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java
View file
Expand Up @@ -54,7 +54,6 @@
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.Executor;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -589,37 +588,20 @@ public boolean isWorkforcePoolConfiguration() {
}

static void validateTokenUrl(String tokenUrl) {
List<Pattern> patterns = new ArrayList<>();
patterns.add(Pattern.compile("^[^\\.\\s\\/\\\\]+\\.sts\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^sts\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^sts\\.[^\\.\\s\\/\\\\]+\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^[^\\.\\s\\/\\\\]+\\-sts\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^sts\\-[^\\.\\s\\/\\\\]+\\.p\\.googleapis\\.com$"));

if (!isValidUrl(patterns, tokenUrl)) {
if (!isValidUrl(tokenUrl)) {
throw new IllegalArgumentException("The provided token URL is invalid.");
}
}

static void validateServiceAccountImpersonationInfoUrl(String serviceAccountImpersonationUrl) {
List<Pattern> patterns = new ArrayList<>();
patterns.add(Pattern.compile("^[^\\.\\s\\/\\\\]+\\.iamcredentials\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^iamcredentials\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^iamcredentials\\.[^\\.\\s\\/\\\\]+\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^[^\\.\\s\\/\\\\]+\\-iamcredentials\\.googleapis\\.com$"));
patterns.add(Pattern.compile("^iamcredentials-[^\\.\\s\\/\\\\]+\\.p\\.googleapis\\.com$"));

if (!isValidUrl(patterns, serviceAccountImpersonationUrl)) {
if (!isValidUrl(serviceAccountImpersonationUrl)) {
throw new IllegalArgumentException(
"The provided service account impersonation URL is invalid.");
}
}

/**
* Returns true if the provided URL's scheme is HTTPS and the host comforms to at least one of the
* provided patterns.
*/
private static boolean isValidUrl(List<Pattern> patterns, String url) {
/** Returns true if the provided URL's scheme is valid and is HTTPS. */
private static boolean isValidUrl(String url) {
URI uri;

try {
Expand All @@ -635,13 +617,7 @@ private static boolean isValidUrl(List<Pattern> patterns, String url) {
return false;
}

for (Pattern pattern : patterns) {
Matcher match = pattern.matcher(uri.getHost().toLowerCase(Locale.US));
if (match.matches()) {
return true;
}
}
return false;
return true;
}

/**
Expand Down
46 changes: 3 additions & 43 deletions oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java
View file
Expand Up @@ -980,34 +980,14 @@ public void validateTokenUrl_validUrls() {
public void validateTokenUrl_invalidUrls() {
List<String> invalidUrls =
Arrays.asList(
"https://iamcredentials.googleapis.com",
"sts.googleapis.com",
"https://",
"http://sts.googleapis.com",
"https://st.s.googleapis.com",
"https://us-eas\\t-1.sts.googleapis.com",
"https:/us-east-1.sts.googleapis.com",
"https://US-WE/ST-1-sts.googleapis.com",
"https://sts-us-east-1.googleapis.com",
"https://sts-US-WEST-1.googleapis.com",
"testhttps://us-east-1.sts.googleapis.com",
"https://us-east-1.sts.googleapis.comevil.com",
"https://us-east-1.us-east-1.sts.googleapis.com",
"https://us-ea.s.t.sts.googleapis.com",
"https://sts.googleapis.comevil.com",
"hhttps://us-east-1.sts.googleapis.com",
"https://us- -1.sts.googleapis.com",
"https://-sts.googleapis.com",
"https://us-east-1.sts.googleapis.com.evil.com",
"https://sts.pgoogleapis.com",
"https://p.googleapis.com",
"https://sts.p.com",
"http://sts.p.googleapis.com",
"https://xyz-sts.p.googleapis.com",
"https://sts-xyz.123.p.googleapis.com",
"https://sts-xyz.p1.googleapis.com",
"https://sts-xyz.p.foo.com",
"https://sts-xyz.p.foo.googleapis.com");
"https://us- -1.sts.googleapis.com");

for (String url : invalidUrls) {
try {
Expand Down Expand Up @@ -1046,34 +1026,14 @@ public void validateServiceAccountImpersonationUrls_validUrls() {
public void validateServiceAccountImpersonationUrls_invalidUrls() {
List<String> invalidUrls =
Arrays.asList(
"https://sts.googleapis.com",
"iamcredentials.googleapis.com",
"https://",
"http://iamcredentials.googleapis.com",
"https://iamcre.dentials.googleapis.com",
"https:/iamcredentials.googleapis.com",
"https://us-eas\t-1.iamcredentials.googleapis.com",
"https:/us-east-1.iamcredentials.googleapis.com",
"https://US-WE/ST-1-iamcredentials.googleapis.com",
"https://iamcredentials-us-east-1.googleapis.com",
"https://iamcredentials-US-WEST-1.googleapis.com",
"testhttps://us-east-1.iamcredentials.googleapis.com",
"https://us-east-1.iamcredentials.googleapis.comevil.com",
"https://us-east-1.us-east-1.iamcredentials.googleapis.com",
"https://us-ea.s.t.iamcredentials.googleapis.com",
"https://iamcredentials.googleapis.comevil.com",
"hhttps://us-east-1.iamcredentials.googleapis.com",
"https://us- -1.iamcredentials.googleapis.com",
"https://-iamcredentials.googleapis.com",
"https://us-east-1.iamcredentials.googleapis.com.evil.com",
"https://iamcredentials.pgoogleapis.com",
"https://p.googleapis.com",
"https://iamcredentials.p.com",
"http://iamcredentials.p.googleapis.com",
"https://xyz-iamcredentials.p.googleapis.com",
"https://iamcredentials-xyz.123.p.googleapis.com",
"https://iamcredentials-xyz.p1.googleapis.com",
"https://iamcredentials-xyz.p.foo.com",
"https://iamcredentials-xyz.p.foo.googleapis.com");
"https://us- -1.iamcredentials.googleapis.com");

for (String url : invalidUrls) {
try {
Expand Down

0 comments on commit 35495b1

Please sign in to comment.