Skip to content

Commit 8969d6d

Browse files
SNOW-3675579: verify TLS certificate hostname on every HTTPS connection
1 parent 2b1bf9e commit 8969d6d

6 files changed

Lines changed: 592 additions & 8 deletions

File tree

DESCRIPTION.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
88

99
# Release Notes
1010
- v3.18.1(July 15,2026)
11-
- N/A
11+
- Improved verification of TLS connections (SNOW-3675579).
1212

1313
- v3.18.0(October 03,2025)
1414
- Added support for pandas conversion for Day-time and Year-Month Interval types

src/snowflake/connector/ssl_wrap_socket.py

Lines changed: 126 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from .vendored.urllib3 import connection as connection_
2929
from .vendored.urllib3.contrib.pyopenssl import PyOpenSSLContext, WrappedSocket
3030
from .vendored.urllib3.util import ssl_ as ssl_
31+
from .vendored.urllib3.util.ssl_match_hostname import CertificateError, match_hostname
3132

3233
DEFAULT_OCSP_MODE: OCSPMode = OCSPMode.FAIL_OPEN
3334
FEATURE_OCSP_MODE: OCSPMode = DEFAULT_OCSP_MODE
@@ -76,13 +77,79 @@ def _ensure_partial_chain_on_context(ctx: PyOpenSSLContext, cafile: str | None)
7677
pass
7778

7879

79-
def _build_context_with_partial_chain(cafile: str | None) -> PyOpenSSLContext:
80-
"""Create PyOpenSSL context configured for CERT_REQUIRED and partial-chain trust."""
80+
def _nonnegative_options(value: int) -> int:
81+
"""Return *value* as the non-negative bitmask pyOpenSSL/cryptography expects.
82+
83+
``ssl.SSLContext.options`` is exposed through a signed, platform-width C
84+
``long``. On Windows that type is 32 bits, so the common default mask (which
85+
has bit 31 set, e.g. ``0x82520050``) is returned as a *negative* Python int.
86+
cryptography's binding marshals the value into an unsigned parameter and
87+
rejects negatives with ``OverflowError: can't convert negative number to
88+
unsigned`` -- which previously aborted every Windows TLS handshake the
89+
moment we carried these options onto the substituted ``PyOpenSSLContext``.
90+
Recover the intended unsigned 32-bit mask; values that are already
91+
non-negative (every other platform) pass through unchanged.
92+
"""
93+
return value & 0xFFFFFFFF if value < 0 else value
94+
95+
96+
def _apply_stdlib_hardening(dst: PyOpenSSLContext, src: ssl.SSLContext | None) -> None:
97+
"""Carry TLS hardening from a stdlib ``SSLContext`` onto ``dst``.
98+
99+
The connector replaces the stdlib ``ssl.SSLContext`` that urllib3 builds
100+
(or that a caller supplied) with a ``PyOpenSSLContext``. Without copying the
101+
original context's hardening forward, the substitution silently drops the
102+
TLS-version floor and ``OP_NO_*`` options urllib3 configured and
103+
any hardening a caller set on a supplied context. Copy the
104+
settings we can read back; fall back to urllib3's default floor when there
105+
is no source context to mirror.
106+
107+
Limitation: cipher restrictions and pinned CA material (``cadata`` /
108+
``load_verify_locations``) cannot be read back out of an ``ssl.SSLContext``,
109+
so they cannot be transferred here. Honoring caller-supplied pinning needs a
110+
dedicated, supported channel and is tracked as a follow-up.
111+
"""
112+
if isinstance(src, ssl.SSLContext):
113+
# Mirror the protocol-version floor/ceiling and OpenSSL options the
114+
# original context carried (e.g. TLS 1.2 minimum, OP_NO_SSLv3,
115+
# OP_NO_COMPRESSION) plus any caller hardening (e.g. VERIFY_X509_STRICT).
116+
for attr in ("minimum_version", "maximum_version", "verify_flags"):
117+
try:
118+
setattr(dst, attr, getattr(src, attr))
119+
except (ValueError, OSError, OpenSSL.SSL.Error):
120+
# Best-effort; an unsupported value must not break the handshake.
121+
pass
122+
try:
123+
dst.options |= _nonnegative_options(src.options)
124+
except (ValueError, OSError, OpenSSL.SSL.Error, OverflowError, TypeError):
125+
# Best-effort; carrying options forward must never break the
126+
# handshake even if a value can't be marshalled into pyOpenSSL.
127+
pass
128+
else:
129+
# No source context to mirror (no ssl_context was supplied): restore the
130+
# hardening urllib3's create_urllib3_context() would have applied.
131+
try:
132+
dst.minimum_version = ssl.TLSVersion.TLSv1_2
133+
dst.options |= ssl.OP_NO_COMPRESSION
134+
except (ValueError, OSError, OpenSSL.SSL.Error, OverflowError, TypeError):
135+
pass
136+
137+
138+
def _build_context_with_partial_chain(
139+
cafile: str | None, src_context: ssl.SSLContext | None = None
140+
) -> PyOpenSSLContext:
141+
"""Create PyOpenSSL context configured for CERT_REQUIRED and partial-chain trust.
142+
143+
When ``src_context`` is the stdlib context being replaced, its TLS hardening
144+
(version floor, options, verify flags) is carried forward so the
145+
substitution does not weaken the connection.
146+
"""
81147
ctx = PyOpenSSLContext(ssl_.PROTOCOL_TLS_CLIENT)
82148
try:
83149
ctx.verify_mode = ssl.CERT_REQUIRED
84150
except Exception:
85151
pass
152+
_apply_stdlib_hardening(ctx, src_context)
86153
_ensure_partial_chain_on_context(ctx, cafile)
87154
return ctx
88155

@@ -144,6 +211,54 @@ def inject_into_urllib3() -> None:
144211
connection_.ssl_wrap_socket = ssl_wrap_socket_with_ocsp
145212

146213

214+
def _verify_hostname_after_handshake(
215+
wrapped_socket: WrappedSocket,
216+
server_hostname: str | None,
217+
ssl_context: Any,
218+
) -> None:
219+
"""Match the peer certificate against *server_hostname*."""
220+
# Honor explicitly-disabled certificate verification (CERT_NONE) first; when
221+
# verification is off there is nothing to assert about server identity, so a
222+
# missing hostname is acceptable. Check this before the hostname so we only
223+
# skip when the caller genuinely opted out of verification.
224+
verify_mode = getattr(ssl_context, "verify_mode", ssl.CERT_REQUIRED)
225+
if verify_mode == ssl.CERT_NONE:
226+
return
227+
228+
# Verification is required but there is no host to match against (e.g. TLS
229+
# without SNI). Fail closed rather than accepting the peer: without a
230+
# hostname we cannot assert server identity.
231+
if not server_hostname:
232+
raise CertificateError(
233+
"no server hostname supplied to match against the peer certificate; "
234+
"cannot verify server identity"
235+
)
236+
237+
# Normalize bracketed / scoped IPv6 literals the same way urllib3 does:
238+
# strip the brackets and drop any "%scope" suffix before testing for an IP,
239+
# since Python's ssl module treats scoped addresses as DNS hostnames.
240+
normalized = server_hostname.strip("[]")
241+
if "%" in normalized:
242+
normalized = normalized[: normalized.rfind("%")]
243+
if ssl_.is_ipaddress(normalized):
244+
server_hostname = normalized
245+
246+
cert = wrapped_socket.getpeercert()
247+
try:
248+
match_hostname(cert, server_hostname)
249+
except CertificateError as e:
250+
log.warning(
251+
"Certificate did not match expected hostname: %s. Certificate: %s",
252+
server_hostname,
253+
cert,
254+
)
255+
# Attach the cert so callers catching CertificateError can inspect it,
256+
# matching urllib3's own _match_hostname behavior.
257+
e._peer_cert = cert
258+
wrapped_socket.close()
259+
raise
260+
261+
147262
@wraps(ssl_.ssl_wrap_socket)
148263
def ssl_wrap_socket_with_ocsp(*args: Any, **kwargs: Any) -> WrappedSocket:
149264
# Bind passed args/kwargs to the underlying signature to support both positional and keyword calls
@@ -158,15 +273,21 @@ def ssl_wrap_socket_with_ocsp(*args: Any, **kwargs: Any) -> WrappedSocket:
158273

159274
# Ensure PyOpenSSL context with partial-chain is used if none or wrong type provided
160275
provided_ctx = params.get("ssl_context")
276+
cafile_for_ctx = _resolve_cafile(params)
161277
if not isinstance(provided_ctx, PyOpenSSLContext):
162-
cafile_for_ctx = _resolve_cafile(params)
163-
params["ssl_context"] = _build_context_with_partial_chain(cafile_for_ctx)
278+
# Carry the replaced stdlib context's TLS hardening forward so the
279+
# substitution doesn't silently weaken the connection.
280+
params["ssl_context"] = _build_context_with_partial_chain(
281+
cafile_for_ctx, src_context=provided_ctx
282+
)
164283
else:
165284
# If a PyOpenSSLContext is provided, ensure it trusts the provided CA and partial-chain is enabled
166-
_ensure_partial_chain_on_context(provided_ctx, _resolve_cafile(params))
285+
_ensure_partial_chain_on_context(provided_ctx, cafile_for_ctx)
167286

168287
ret = ssl_.ssl_wrap_socket(**params)
169288

289+
_verify_hostname_after_handshake(ret, server_hostname, params.get("ssl_context"))
290+
170291
log.debug(
171292
"OCSP Mode: %s, OCSP response cache file name: %s",
172293
FEATURE_OCSP_MODE.name,

test/integ/test_connection.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -812,9 +812,13 @@ def test_invalid_connection_parameters_turned_off(conn_cnx):
812812
# TODO: SNOW-2114216 remove filtering once the root cause for deprecation warning is fixed
813813
# Filter out the deprecation warning
814814
filtered_w = [
815-
warning for warning in w if warning.category != DeprecationWarning
815+
warning
816+
for warning in w
817+
if str(warning.category).endswith("DeprecationWarning")
816818
]
817-
assert len(filtered_w) == 0
819+
assert (
820+
len(filtered_w) == 0
821+
), f"Unexpected warnings {[f.message for f in filtered_w]}"
818822

819823

820824
def test_invalid_connection_parameters_only_warns(conn_cnx):

0 commit comments

Comments
 (0)