Skip to content

Commit b8c8990

Browse files
committed
Update the retry logic and refactor cert handling.
1 parent 4b628b9 commit b8c8990

File tree

8 files changed

+196
-105
lines changed

8 files changed

+196
-105
lines changed

google/auth/_agent_identity_utils.py

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
import hashlib
1919
import os
2020
import re
21+
import time
2122

22-
from google.auth import _exponential_backoff
2323
from google.auth import environment_vars
2424
from google.auth import exceptions
2525

@@ -51,11 +51,10 @@ def get_agent_identity_certificate_path():
5151
if not cert_config_path:
5252
return None
5353

54-
# Use exponential backoff to retry loading the certificate config file.
55-
# This is to handle the race condition where the env var is set before the file exists.
56-
backoff = _exponential_backoff.ExponentialBackoff(total_attempts=5)
57-
58-
for _ in backoff:
54+
# Poll for the config file and the certificate file to be available.
55+
# Phase 1: Poll rapidly for 5 seconds (50 * 0.1s).
56+
# Phase 2: Slow down polling for the next 25 seconds (50 * 0.5s).
57+
for i in range(100):
5958
if os.path.exists(cert_config_path):
6059
with open(cert_config_path, "r") as f:
6160
cert_config = json.load(f)
@@ -66,20 +65,41 @@ def get_agent_identity_certificate_path():
6665
)
6766
if cert_path and os.path.exists(cert_path):
6867
return cert_path
68+
if i < 50:
69+
time.sleep(0.1)
70+
else:
71+
time.sleep(0.5)
6972

7073
raise exceptions.RefreshError(
71-
"Certificate config or certificate file not found after multiple retries."
74+
"Certificate config or certificate file not found after multiple retries. "
75+
f"If you are using Agent Engine, you can export "
76+
f"{environment_vars.GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES} to false to "
77+
"disable cert bound tokens to fall back to unbound tokens."
7278
)
7379

7480

75-
def _is_agent_identity_certificate(cert_bytes):
81+
def parse_certificate(cert_bytes):
82+
"""Parses a PEM-encoded certificate.
83+
84+
Args:
85+
cert_bytes (bytes): The PEM-encoded certificate bytes.
86+
87+
Returns:
88+
cryptography.x509.Certificate: The parsed certificate object.
89+
"""
90+
from cryptography import x509
91+
92+
return x509.load_pem_x509_certificate(cert_bytes)
93+
94+
95+
def _is_agent_identity_certificate(cert):
7696
"""Checks if a certificate is an Agent Identity certificate.
7797
7898
This is determined by checking the Subject Alternative Name (SAN) for a
7999
SPIFFE ID with a trust domain matching Agent Identity patterns.
80100
81101
Args:
82-
cert_bytes (bytes): The PEM-encoded certificate bytes.
102+
cert (cryptography.x509.Certificate): The parsed certificate object.
83103
84104
Returns:
85105
bool: True if the certificate is an Agent Identity certificate,
@@ -88,7 +108,6 @@ def _is_agent_identity_certificate(cert_bytes):
88108
from cryptography import x509
89109
from cryptography.x509.oid import ExtensionOID
90110

91-
cert = x509.load_pem_x509_certificate(cert_bytes)
92111
try:
93112
ext = cert.extensions.get_extension_for_oid(
94113
ExtensionOID.SUBJECT_ALTERNATIVE_NAME
@@ -107,37 +126,39 @@ def _is_agent_identity_certificate(cert_bytes):
107126
return False
108127

109128

110-
def calculate_certificate_fingerprint(cert_bytes):
129+
def calculate_certificate_fingerprint(cert):
111130
"""Calculates the base64-encoded SHA256 hash of a DER-encoded certificate.
112131
113132
Args:
114-
cert_bytes (bytes): The PEM-encoded certificate bytes.
133+
cert (cryptography.x509.Certificate): The parsed certificate object.
115134
116135
Returns:
117136
str: The base64-encoded SHA256 fingerprint.
118137
"""
119-
from cryptography import x509
120138
from cryptography.hazmat.primitives import serialization
121139

122-
cert = x509.load_pem_x509_certificate(cert_bytes)
123140
der_cert = cert.public_bytes(serialization.Encoding.DER)
124141
fingerprint = hashlib.sha256(der_cert).digest()
125-
return base64.b64encode(fingerprint).decode("utf-8")
142+
return base64.urlsafe_b64encode(fingerprint).rstrip(b"=").decode("utf-8")
126143

127144

128-
def should_request_bound_token(cert_bytes):
145+
def should_request_bound_token(cert):
129146
"""Determines if a bound token should be requested.
130147
131148
This is based on the GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES
132149
environment variable and whether the certificate is an agent identity cert.
133150
151+
Args:
152+
cert (cryptography.x509.Certificate): The parsed certificate object.
153+
134154
Returns:
135155
bool: True if a bound token should be requested, False otherwise.
136156
"""
137-
is_agent_cert = _is_agent_identity_certificate(cert_bytes)
157+
is_agent_cert = _is_agent_identity_certificate(cert)
138158
is_opted_in = (
139159
os.environ.get(
140-
"GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES", "true"
160+
environment_vars.GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES,
161+
"true",
141162
).lower()
142163
== "true"
143164
)

google/auth/compute_engine/_metadata.py

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -368,18 +368,14 @@ def get_service_account_token(request, service_account="default", scopes=None):
368368
scopes = ",".join(scopes)
369369
params["scopes"] = scopes
370370

371-
try:
372-
cert_path = _agent_identity_utils.get_agent_identity_certificate_path()
373-
if cert_path:
374-
with open(cert_path, "rb") as cert_file:
375-
cert_bytes = cert_file.read()
376-
if _agent_identity_utils.should_request_bound_token(cert_bytes):
377-
fingerprint = _agent_identity_utils.calculate_certificate_fingerprint(
378-
cert_bytes
379-
)
380-
params["bindCertificateFingerprint"] = fingerprint
381-
except exceptions.RefreshError as e:
382-
_LOGGER.warning("Could not load agent identity certificate: %s", e)
371+
cert_path = _agent_identity_utils.get_agent_identity_certificate_path()
372+
if cert_path:
373+
with open(cert_path, "rb") as cert_file:
374+
cert_bytes = cert_file.read()
375+
cert = _agent_identity_utils.parse_certificate(cert_bytes)
376+
if _agent_identity_utils.should_request_bound_token(cert):
377+
fingerprint = _agent_identity_utils.calculate_certificate_fingerprint(cert)
378+
params["bindCertificateFingerprint"] = fingerprint
383379

384380
metrics_header = {
385381
metrics.API_CLIENT_HEADER: metrics.token_request_access_token_mds()

google/auth/environment_vars.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,8 @@
9090
GOOGLE_API_CERTIFICATE_CONFIG = "GOOGLE_API_CERTIFICATE_CONFIG"
9191
"""Environment variable defining the location of Google API certificate config
9292
file."""
93+
94+
GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES = (
95+
"GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES"
96+
)
97+
"""Environment variable to prevent agent token sharing for GCP services."""

google/auth/identity_pool.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,9 +564,10 @@ def refresh(self, request):
564564
# Check if the credential is X.509 based.
565565
if self._credential_source_certificate is not None:
566566
cert_bytes = self._get_cert_bytes()
567-
if _agent_identity_utils.should_request_bound_token(cert_bytes):
567+
cert = _agent_identity_utils.parse_certificate(cert_bytes)
568+
if _agent_identity_utils.should_request_bound_token(cert):
568569
cert_fingerprint = _agent_identity_utils.calculate_certificate_fingerprint(
569-
cert_bytes
570+
cert
570571
)
571572

572573
self._refresh_token(request, cert_fingerprint=cert_fingerprint)

tests/compute_engine/test__metadata.py

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import mock
2222
import pytest # type: ignore
2323

24+
from google.auth import _agent_identity_utils
2425
from google.auth import _helpers
2526
from google.auth import environment_vars
2627
from google.auth import exceptions
@@ -38,6 +39,29 @@
3839
DATA_DIR, "smbios_product_name_non_google"
3940
)
4041

42+
# A mock PEM-encoded certificate without an Agent Identity SPIFFE ID.
43+
NON_AGENT_IDENTITY_CERT_BYTES = (
44+
b"-----BEGIN CERTIFICATE-----\n"
45+
b"MIIDIzCCAgugAwIBAgIJAMfISuBQ5m+5MA0GCSqGSIb3DQEBBQUAMBUxEzARBgNV\n"
46+
b"BAMTCnVuaXQtdGVzdHMwHhcNMTExMjA2MTYyNjAyWhcNMjExMjAzMTYyNjAyWjAV\n"
47+
b"MRMwEQYDVQQDEwp1bml0LXRlc3RzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\n"
48+
b"CgKCAQEA4ej0p7bQ7L/r4rVGUz9RN4VQWoej1Bg1mYWIDYslvKrk1gpj7wZgkdmM\n"
49+
b"7oVK2OfgrSj/FCTkInKPqaCR0gD7K80q+mLBrN3PUkDrJQZpvRZIff3/xmVU1Wer\n"
50+
b"uQLFJjnFb2dqu0s/FY/2kWiJtBCakXvXEOb7zfbINuayL+MSsCGSdVYsSliS5qQp\n"
51+
b"gyDap+8b5fpXZVJkq92hrcNtbkg7hCYUJczt8n9hcCTJCfUpApvaFQ18pe+zpyl4\n"
52+
b"+WzkP66I28hniMQyUlA1hBiskT7qiouq0m8IOodhv2fagSZKjOTTU2xkSBc//fy3\n"
53+
b"ZpsL7WqgsZS7Q+0VRK8gKfqkxg5OYQIDAQABo3YwdDAdBgNVHQ4EFgQU2RQ8yO+O\n"
54+
b"gN8oVW2SW7RLrfYd9jEwRQYDVR0jBD4wPIAU2RQ8yO+OgN8oVW2SW7RLrfYd9jGh\n"
55+
b"GaQXMBUxEzARBgNVBAMTCnVuaXQtdGVzdHOCCQDHyErgUOZvuTAMBgNVHRMEBTAD\n"
56+
b"AQH/MA0GCSqGSIb3DQEBBQUAA4IBAQBRv+M/6+FiVu7KXNjFI5pSN17OcW5QUtPr\n"
57+
b"odJMlWrJBtynn/TA1oJlYu3yV5clc/71Vr/AxuX5xGP+IXL32YDF9lTUJXG/uUGk\n"
58+
b"+JETpKmQviPbRsvzYhz4pf6ZIOZMc3/GIcNq92ECbseGO+yAgyWUVKMmZM0HqXC9\n"
59+
b"ovNslqe0M8C1sLm1zAR5z/h/litE7/8O2ietija3Q/qtl2TOXJdCA6sgjJX2WUql\n"
60+
b"ybrC55ct18NKf3qhpcEkGQvFU40rVYApJpi98DiZPYFdx1oBDp/f4uZ3ojpxRVFT\n"
61+
b"cDwcJLfNRCPUhormsY7fDS9xSyThiHsW9mjJYdcaKQkwYZ0F11yB\n"
62+
b"-----END CERTIFICATE-----\n"
63+
)
64+
4165
ACCESS_TOKEN_REQUEST_METRICS_HEADER_VALUE = (
4266
"gl-python/3.7 auth/1.1 auth-request-type/at cred-type/mds"
4367
)
@@ -641,21 +665,40 @@ def test_get_service_account_token_with_scopes_string(
641665
assert expiry == utcnow() + datetime.timedelta(seconds=ttl)
642666

643667

644-
@mock.patch("google.auth.compute_engine._metadata.get")
645-
@mock.patch("google.auth.compute_engine._metadata._LOGGER")
646-
@mock.patch("google.auth._agent_identity_utils.get_agent_identity_certificate_path")
647-
def test_get_service_account_token_agent_identity_refresh_error(
648-
mock_get_agent_path, mock_logger, mock_get
649-
):
650-
mock_get_agent_path.side_effect = exceptions.RefreshError("Test error")
651-
mock_get.return_value = {"access_token": "token", "expires_in": 500}
668+
@mock.patch(
669+
"google.auth._agent_identity_utils._is_agent_identity_certificate",
670+
return_value=True,
671+
)
672+
def test_get_service_account_token_with_bound_token(mock_is_agent, tmpdir, monkeypatch):
673+
# Create a mock certificate and a config file that points to it.
674+
cert_path = tmpdir.join("cert.pem")
675+
cert_path.write_binary(NON_AGENT_IDENTITY_CERT_BYTES)
676+
config_path = tmpdir.join("config.json")
677+
config_path.write(
678+
json.dumps({"cert_configs": {"workload": {"cert_path": str(cert_path)}}})
679+
)
680+
monkeypatch.setenv(environment_vars.GOOGLE_API_CERTIFICATE_CONFIG, str(config_path))
681+
monkeypatch.setenv(
682+
environment_vars.GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES, "true"
683+
)
652684

653-
_metadata.get_service_account_token(mock.sentinel.request)
685+
# Create a mock request to simulate the metadata server response.
686+
token_response = json.dumps({"access_token": "token", "expires_in": 3600})
687+
request = make_request(token_response, headers={"content-type": "application/json"})
654688

655-
mock_logger.warning.assert_called_once_with(
656-
"Could not load agent identity certificate: %s", mock.ANY
657-
)
658-
mock_get.assert_called_once()
689+
# Call the function under test.
690+
_metadata.get_service_account_token(request)
691+
692+
# Verify the request URL contains the correct fingerprint.
693+
request.assert_called_once()
694+
_, kwargs = request.call_args
695+
url = kwargs["url"]
696+
697+
# Calculate the expected fingerprint.
698+
cert = _agent_identity_utils.parse_certificate(NON_AGENT_IDENTITY_CERT_BYTES)
699+
expected_fingerprint = _agent_identity_utils.calculate_certificate_fingerprint(cert)
700+
701+
assert f"bindCertificateFingerprint={expected_fingerprint}" in url
659702

660703

661704
def test_get_service_account_info():

tests/compute_engine/test_credentials.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,7 @@ def test_build_trust_boundary_lookup_url_no_email(
660660

661661
@mock.patch("google.auth.compute_engine._metadata.get")
662662
@mock.patch("google.auth._agent_identity_utils.get_agent_identity_certificate_path")
663+
@mock.patch("google.auth._agent_identity_utils.parse_certificate")
663664
@mock.patch(
664665
"google.auth._agent_identity_utils.should_request_bound_token",
665666
return_value=True,
@@ -672,6 +673,7 @@ def test_refresh_with_agent_identity(
672673
self,
673674
mock_calculate_fingerprint,
674675
mock_should_request,
676+
mock_parse_certificate,
675677
mock_get_path,
676678
mock_metadata_get,
677679
tmpdir,
@@ -688,7 +690,8 @@ def test_refresh_with_agent_identity(
688690
self.credentials.refresh(None)
689691

690692
assert self.credentials.token == "token"
691-
mock_should_request.assert_called_once_with(b"cert_content")
693+
mock_parse_certificate.assert_called_once_with(b"cert_content")
694+
mock_should_request.assert_called_once_with(mock_parse_certificate.return_value)
692695
kwargs = mock_metadata_get.call_args[1]
693696
assert kwargs["params"] == {
694697
"scopes": "one,two",
@@ -697,12 +700,18 @@ def test_refresh_with_agent_identity(
697700

698701
@mock.patch("google.auth.compute_engine._metadata.get")
699702
@mock.patch("google.auth._agent_identity_utils.get_agent_identity_certificate_path")
703+
@mock.patch("google.auth._agent_identity_utils.parse_certificate")
700704
@mock.patch(
701705
"google.auth._agent_identity_utils.should_request_bound_token",
702706
return_value=False,
703707
)
704708
def test_refresh_with_agent_identity_opt_out_or_not_agent(
705-
self, mock_should_request, mock_get_path, mock_metadata_get, tmpdir
709+
self,
710+
mock_should_request,
711+
mock_parse_certificate,
712+
mock_get_path,
713+
mock_metadata_get,
714+
tmpdir,
706715
):
707716
cert_path = tmpdir.join("cert.pem")
708717
cert_path.write(b"cert_content")
@@ -716,7 +725,8 @@ def test_refresh_with_agent_identity_opt_out_or_not_agent(
716725
self.credentials.refresh(None)
717726

718727
assert self.credentials.token == "token"
719-
mock_should_request.assert_called_once_with(b"cert_content")
728+
mock_parse_certificate.assert_called_once_with(b"cert_content")
729+
mock_should_request.assert_called_once_with(mock_parse_certificate.return_value)
720730
kwargs = mock_metadata_get.call_args[1]
721731
assert "bindCertificateFingerprint" not in kwargs.get("params", {})
722732

0 commit comments

Comments
 (0)