Skip to content

Commit 0b0c168

Browse files
authored
Add bounded request timeouts to Fulcio client (#1917)
* Add bounded timeouts to Fulcio client Signed-off-by: ECD5A <111098024+ECD5A@users.noreply.github.com> * Keep Fulcio timeout handling narrowly scoped Signed-off-by: ECD5A <111098024+ECD5A@users.noreply.github.com> --------- Signed-off-by: ECD5A <111098024+ECD5A@users.noreply.github.com>
1 parent 233dbd7 commit 0b0c168

3 files changed

Lines changed: 54 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ All versions prior to 0.9.0 are untracked.
2626

2727
### Fixed
2828

29+
* Fulcio certificate and trust-bundle requests now use finite connection and
30+
read inactivity timeouts.
2931
* Parsing a malformed in-toto statement now includes the underlying validation
3032
error, instead of discarding it. `StatementBuilder.build()` already did this;
3133
`Statement(contents=...)` did not, so a rejected digest algorithm, a missing

‎sigstore/_internal/fulcio/client.py‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@
4242
SIGNING_CERT_ENDPOINT = "/api/v2/signingCert"
4343
TRUST_BUNDLE_ENDPOINT = "/api/v2/trustBundle"
4444

45+
# Use a short connect timeout and a finite read timeout for Fulcio requests.
46+
# Certificate issuance and trust-bundle retrieval should not block the caller
47+
# indefinitely when the configured endpoint accepts a connection but stalls.
48+
DEFAULT_FULCIO_TIMEOUT: tuple[int, int] = (5, 30)
49+
4550

4651
class ExpiredCertificate(Exception):
4752
"""An error raised when the Certificate is expired."""
@@ -101,7 +106,10 @@ def post(
101106
"Accept": "application/pem-certificate-chain",
102107
}
103108
resp: requests.Response = self.session.post(
104-
url=self.url, data=_serialize_cert_request(req), headers=headers
109+
url=self.url,
110+
data=_serialize_cert_request(req),
111+
headers=headers,
112+
timeout=DEFAULT_FULCIO_TIMEOUT,
105113
)
106114
try:
107115
resp.raise_for_status()
@@ -139,7 +147,9 @@ class FulcioTrustBundle(_Endpoint):
139147

140148
def get(self) -> FulcioTrustBundleResponse:
141149
"""Get the certificate chains from Fulcio"""
142-
resp: requests.Response = self.session.get(self.url)
150+
resp: requests.Response = self.session.get(
151+
self.url, timeout=DEFAULT_FULCIO_TIMEOUT
152+
)
143153
try:
144154
resp.raise_for_status()
145155
except requests.HTTPError as http_error:

‎test/unit/internal/fulcio/test_client.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,43 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import unittest.mock
16+
17+
import pytest
18+
import requests
19+
20+
from sigstore._internal.fulcio.client import (
21+
DEFAULT_FULCIO_TIMEOUT,
22+
FulcioClient,
23+
FulcioClientError,
24+
)
25+
26+
27+
def _client_with_mock_session():
28+
client = FulcioClient("http://fake")
29+
session = unittest.mock.MagicMock()
30+
client.session = session
31+
return client, session
32+
33+
34+
def test_signing_cert_post_passes_timeout():
35+
client, session = _client_with_mock_session()
36+
37+
request = unittest.mock.MagicMock()
38+
request.public_bytes.return_value = b"csr"
39+
response = session.post.return_value
40+
response.raise_for_status.side_effect = requests.HTTPError
41+
42+
with pytest.raises(FulcioClientError):
43+
client.signing_cert.post(request, "token")
44+
45+
assert session.post.call_args.kwargs["timeout"] == DEFAULT_FULCIO_TIMEOUT
46+
47+
48+
def test_trust_bundle_get_passes_timeout():
49+
client, session = _client_with_mock_session()
50+
session.get.return_value.json.return_value = {"chains": []}
51+
52+
client.trust_bundle.get()
53+
54+
assert session.get.call_args.kwargs["timeout"] == DEFAULT_FULCIO_TIMEOUT

0 commit comments

Comments
 (0)