Skip to content

Commit 32d3c4b

Browse files
authored
SNOW-890625 Do not write cache when we have no OCSP response (#1690)
Description When there is connection error, we will get None as OCSP response, which should not be written to cache. Testing unit test
1 parent 44a91a7 commit 32d3c4b

File tree

2 files changed

+29
-6
lines changed

2 files changed

+29
-6
lines changed

src/snowflake/connector/ocsp_snowflake.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,7 @@ def _validate_certificates_sequential(
11891189
ocsp_response_validation_result is None
11901190
or not ocsp_response_validation_result.validated
11911191
):
1192+
# r is a tuple of (err, issuer, subject, cert_id, ocsp_response)
11921193
r = self.validate_by_direct_connection(
11931194
issuer,
11941195
subject,
@@ -1197,12 +1198,18 @@ def _validate_certificates_sequential(
11971198
do_retry=do_retry,
11981199
cache_key=cache_key,
11991200
)
1200-
to_update_cache_dict[cache_key] = OCSPResponseValidationResult(
1201-
*r,
1202-
ts=int(time.time()),
1203-
validated=True,
1204-
)
1205-
OCSPCache.CACHE_UPDATED = True
1201+
1202+
# When OCSP server is down, the validation fails and the oscp_response will be None, and in fail open
1203+
# case, we will also reset err to None.
1204+
# In this case we don't need to write the response to cache because there is no information from a
1205+
# connection error.
1206+
if r[0] is not None or r[4] is not None:
1207+
to_update_cache_dict[cache_key] = OCSPResponseValidationResult(
1208+
*r,
1209+
ts=int(time.time()),
1210+
validated=True,
1211+
)
1212+
OCSPCache.CACHE_UPDATED = True
12061213
results.append(r)
12071214
else:
12081215
results.append(

test/unit/test_ocsp.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,22 @@ def test_ocsp_with_invalid_cache_file():
308308
assert ocsp.validate(url, connection), f"Failed to validate: {url}"
309309

310310

311+
@mock.patch(
312+
"snowflake.connector.ocsp_snowflake.SnowflakeOCSP._fetch_ocsp_response",
313+
side_effect=BrokenPipeError("fake error"),
314+
)
315+
def test_ocsp_cache_when_server_is_down(mock_fetch_ocsp_response, tmpdir):
316+
ocsp = SFOCSP()
317+
318+
"""Attempts to use outdated OCSP response cache file."""
319+
cache_file_name, target_hosts = _store_cache_in_file(tmpdir)
320+
321+
# reading cache file
322+
OCSPCache.read_ocsp_response_cache_file(ocsp, cache_file_name)
323+
cache_data = snowflake.connector.ocsp_snowflake.OCSP_RESPONSE_VALIDATION_CACHE
324+
assert not cache_data, "no cache should present because of broken pipe"
325+
326+
311327
def test_concurrent_ocsp_requests(tmpdir):
312328
"""Run OCSP revocation checks in parallel. The memory and file caches are deleted randomly."""
313329
cache_file_name = path.join(str(tmpdir), "cache_file.txt")

0 commit comments

Comments
 (0)