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: Remove 3PI config url validation #1220

Merged
merged 7 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
10 changes: 10 additions & 0 deletions docs/user-guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,16 @@ in the target project. Using `ImpersonatedCredentials` will allow the source_cre
to assume the identity of a target_principal that does have access.


Security considerations on configuration External and Impersonated credentials
BigTailWolf marked this conversation as resolved.
Show resolved Hide resolved
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

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.


Downscoped credentials
++++++++++++++++++++++

Expand Down
61 changes: 0 additions & 61 deletions google/auth/external_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import re

import six
from urllib3.util import parse_url

from google.auth import _helpers
from google.auth import credentials
Expand Down Expand Up @@ -127,14 +126,6 @@ def __init__(
self._default_scopes = default_scopes
self._workforce_pool_user_project = workforce_pool_user_project

Credentials.validate_token_url(token_url)
if token_info_url:
Credentials.validate_token_url(token_info_url, url_type="token info")
if service_account_impersonation_url:
Credentials.validate_service_account_impersonation_url(
service_account_impersonation_url
)

if self._client_id:
self._client_auth = utils.ClientAuthentication(
utils.ClientAuthType.basic, self._client_id, self._client_secret
Expand Down Expand Up @@ -434,58 +425,6 @@ def _initialize_impersonated_credentials(self):
),
)

@staticmethod
def validate_token_url(token_url, url_type="token"):
_TOKEN_URL_PATTERNS = [
"^[^\\.\\s\\/\\\\]+\\.sts(?:\\.mtls)?\\.googleapis\\.com$",
"^sts(?:\\.mtls)?\\.googleapis\\.com$",
"^sts\\.[^\\.\\s\\/\\\\]+(?:\\.mtls)?\\.googleapis\\.com$",
"^[^\\.\\s\\/\\\\]+\\-sts(?:\\.mtls)?\\.googleapis\\.com$",
"^sts\\-[^\\.\\s\\/\\\\]+\\.p(?:\\.mtls)?\\.googleapis\\.com$",
]

if not Credentials.is_valid_url(_TOKEN_URL_PATTERNS, token_url):
raise exceptions.InvalidResource(
"The provided {} URL is invalid.".format(url_type)
)

@staticmethod
def validate_service_account_impersonation_url(url):
_SERVICE_ACCOUNT_IMPERSONATION_URL_PATTERNS = [
"^[^\\.\\s\\/\\\\]+\\.iamcredentials\\.googleapis\\.com$",
"^iamcredentials\\.googleapis\\.com$",
"^iamcredentials\\.[^\\.\\s\\/\\\\]+\\.googleapis\\.com$",
"^[^\\.\\s\\/\\\\]+\\-iamcredentials\\.googleapis\\.com$",
"^iamcredentials\\-[^\\.\\s\\/\\\\]+\\.p\\.googleapis\\.com$",
]

if not Credentials.is_valid_url(
_SERVICE_ACCOUNT_IMPERSONATION_URL_PATTERNS, url
):
raise exceptions.InvalidResource(
"The provided service account impersonation URL is invalid."
)

@staticmethod
def is_valid_url(patterns, url):
"""
Returns True if the provided URL's scheme is HTTPS and the host comforms to at least one of the provided patterns.
"""
# Check specifically for whitespcaces:
# Some python3.6 will parse the space character into %20 and pass the regex check which shouldn't be passed
if not url or len(str(url).split()) > 1:
return False

try:
uri = parse_url(url)
except Exception:
return False

if not uri.scheme or uri.scheme != "https" or not uri.hostname:
return False

return any(re.compile(p).match(uri.hostname.lower()) for p in patterns)

@classmethod
def from_info(cls, info, **kwargs):
"""Creates a Credentials instance from parsed external account info.
Expand Down
34 changes: 0 additions & 34 deletions tests/test_aws.py
Original file line number Diff line number Diff line change
Expand Up @@ -1085,16 +1085,6 @@ def test_token_info_url_custom(self):

assert credentials.token_info_url == (url + "/introspect")

def test_token_info_url_bad(self):
for url in INVALID_TOKEN_URLS:
with pytest.raises(ValueError) as excinfo:
self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE.copy(),
token_info_url=(url + "/introspect"),
)

assert excinfo.match(r"The provided token info URL is invalid\.")

def test_token_info_url_negative(self):
credentials = self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE.copy(), token_info_url=None
Expand All @@ -1111,16 +1101,6 @@ def test_token_url_custom(self):

assert credentials._token_url == (url + "/token")

def test_token_url_bad(self):
for url in INVALID_TOKEN_URLS:
with pytest.raises(ValueError) as excinfo:
self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE.copy(),
token_url=(url + "/token"),
)

assert excinfo.match(r"The provided token URL is invalid\.")

def test_service_account_impersonation_url_custom(self):
for url in VALID_SERVICE_ACCOUNT_IMPERSONATION_URLS:
credentials = self.make_credentials(
Expand All @@ -1134,20 +1114,6 @@ def test_service_account_impersonation_url_custom(self):
url + SERVICE_ACCOUNT_IMPERSONATION_URL_ROUTE
)

def test_service_account_impersonation_url_bad(self):
for url in INVALID_SERVICE_ACCOUNT_IMPERSONATION_URLS:
with pytest.raises(ValueError) as excinfo:
self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE.copy(),
service_account_impersonation_url=(
url + SERVICE_ACCOUNT_IMPERSONATION_URL_ROUTE
),
)

assert excinfo.match(
r"The provided service account impersonation URL is invalid\."
)

def test_retrieve_subject_token_missing_region_url(self):
# When AWS_REGION envvar is not available, region_url is required for
# determining the current AWS region.
Expand Down
158 changes: 0 additions & 158 deletions tests/test_external_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,101 +65,6 @@
"//iam.googleapis.com/locations//workforcePool/pool-id/providers/provider-id",
]

VALID_TOKEN_URLS = [
"https://sts.googleapis.com",
"https://sts.mtls.googleapis.com",
"https://us-east-1.sts.googleapis.com",
"https://us-east-1.sts.mtls.googleapis.com",
"https://US-EAST-1.sts.googleapis.com",
"https://sts.us-east-1.googleapis.com",
"https://sts.US-WEST-1.googleapis.com",
"https://us-east-1-sts.googleapis.com",
"https://US-WEST-1-sts.googleapis.com",
"https://US-WEST-1-sts.mtls.googleapis.com",
"https://us-west-1-sts.googleapis.com/path?query",
"https://sts-us-east-1.p.googleapis.com",
"https://sts-us-east-1.p.mtls.googleapis.com",
]
INVALID_TOKEN_URLS = [
"https://iamcredentials.googleapis.com",
"https://mtls.iamcredentials.googleapis.com",
"sts.googleapis.com",
"mtls.sts.googleapis.com",
"mtls.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-east-1.mtls.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://-mtls.googleapis.com",
"https://us-east-1.sts.googleapis.com.evil.com",
"https://sts.pgoogleapis.com",
"https://p.googleapis.com",
"https://sts.p.com",
"https://sts.p.mtls.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://sts-xyz.mtls.p.foo.googleapis.com",
"https://sts-xyz.p.mtls.foo.googleapis.com",
]
VALID_SERVICE_ACCOUNT_IMPERSONATION_URLS = [
"https://iamcredentials.googleapis.com",
"https://us-east-1.iamcredentials.googleapis.com",
"https://US-EAST-1.iamcredentials.googleapis.com",
"https://iamcredentials.us-east-1.googleapis.com",
"https://iamcredentials.US-WEST-1.googleapis.com",
"https://us-east-1-iamcredentials.googleapis.com",
"https://US-WEST-1-iamcredentials.googleapis.com",
"https://us-west-1-iamcredentials.googleapis.com/path?query",
"https://iamcredentials-us-east-1.p.googleapis.com",
]
INVALID_SERVICE_ACCOUNT_IMPERSONATION_URLS = [
"https://sts.googleapis.com",
"iamcredentials.googleapis.com",
"https://",
"http://iamcredentials.googleapis.com",
"https://iamcre.dentials.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",
]


class CredentialsImpl(external_account.Credentials):
def __init__(self, **kwargs):
Expand Down Expand Up @@ -350,44 +255,6 @@ def assert_resource_manager_request_kwargs(
assert request_kwargs["headers"] == headers
assert "body" not in request_kwargs

def test_valid_token_url_shall_pass_validation(self):
valid_urls = VALID_TOKEN_URLS

for url in valid_urls:
# A valid url shouldn't throw exception and a None value should be returned
external_account.Credentials.validate_token_url(url)

def test_invalid_token_url_shall_throw_exceptions(self):
invalid_urls = INVALID_TOKEN_URLS

for url in invalid_urls:
# An invalid url should throw a ValueError exception
with pytest.raises(ValueError) as excinfo:
external_account.Credentials.validate_token_url(url)

assert excinfo.match("The provided token URL is invalid.")

def test_valid_service_account_impersonation_url_shall_pass_validation(self):
valid_urls = VALID_SERVICE_ACCOUNT_IMPERSONATION_URLS

for url in valid_urls:
# A valid url shouldn't throw exception and a None value should be returned
external_account.Credentials.validate_service_account_impersonation_url(url)

def test_invalid_service_account_impersonate_url_shall_throw_exceptions(self):
invalid_urls = INVALID_SERVICE_ACCOUNT_IMPERSONATION_URLS

for url in invalid_urls:
# An invalid url should throw a ValueError exception
with pytest.raises(ValueError) as excinfo:
external_account.Credentials.validate_service_account_impersonation_url(
url
)

assert excinfo.match(
"The provided service account impersonation URL is invalid."
)

def test_default_state(self):
credentials = self.make_credentials(
service_account_impersonation_url=self.SERVICE_ACCOUNT_IMPERSONATION_URL
Expand All @@ -409,31 +276,6 @@ def test_default_state(self):
# Token info url not set yet
assert not credentials.token_info_url

def test_invalid_token_url(self):
with pytest.raises(ValueError) as excinfo:
CredentialsImpl(
audience=self.AUDIENCE,
subject_token_type=self.SUBJECT_TOKEN_TYPE,
token_url="https:///v1/token",
credential_source=self.CREDENTIAL_SOURCE,
)

assert excinfo.match("The provided token URL is invalid.")

def test_invalid_service_account_impersonate_url(self):
with pytest.raises(ValueError) as excinfo:
CredentialsImpl(
audience=self.AUDIENCE,
subject_token_type=self.SUBJECT_TOKEN_TYPE,
token_url=self.TOKEN_URL,
credential_source=self.CREDENTIAL_SOURCE,
service_account_impersonation_url=12345, # create an exception by sending to parse url
)

assert excinfo.match(
"The provided service account impersonation URL is invalid."
)

def test_nonworkforce_with_workforce_pool_user_project(self):
with pytest.raises(ValueError) as excinfo:
CredentialsImpl(
Expand Down
34 changes: 0 additions & 34 deletions tests/test_identity_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -759,16 +759,6 @@ def test_token_info_url_custom(self):

assert credentials.token_info_url == url + "/introspect"

def test_token_info_url_bad(self):
for url in INVALID_TOKEN_URLS:
with pytest.raises(ValueError) as excinfo:
self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE_JSON.copy(),
token_info_url=(url + "/introspect"),
)

assert excinfo.match(r"The provided token info URL is invalid.")

def test_token_info_url_negative(self):
credentials = self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE_JSON.copy(), token_info_url=None
Expand All @@ -785,16 +775,6 @@ def test_token_url_custom(self):

assert credentials._token_url == (url + "/token")

def test_token_url_bad(self):
for url in INVALID_TOKEN_URLS:
with pytest.raises(ValueError) as excinfo:
self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE_JSON.copy(),
token_url=(url + "/token"),
)

assert excinfo.match(r"The provided token URL is invalid\.")

def test_service_account_impersonation_url_custom(self):
for url in VALID_SERVICE_ACCOUNT_IMPERSONATION_URLS:
credentials = self.make_credentials(
Expand All @@ -808,20 +788,6 @@ def test_service_account_impersonation_url_custom(self):
url + SERVICE_ACCOUNT_IMPERSONATION_URL_ROUTE
)

def test_service_account_impersonation_url_bad(self):
for url in INVALID_SERVICE_ACCOUNT_IMPERSONATION_URLS:
with pytest.raises(ValueError) as excinfo:
self.make_credentials(
credential_source=self.CREDENTIAL_SOURCE_JSON.copy(),
service_account_impersonation_url=(
url + SERVICE_ACCOUNT_IMPERSONATION_URL_ROUTE
),
)

assert excinfo.match(
r"The provided service account impersonation URL is invalid\."
)

def test_refresh_text_file_success_without_impersonation_ignore_default_scopes(
self,
):
Expand Down
Loading