Skip to content

Conversation

sfc-gh-mmishchenko
Copy link
Collaborator

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes #NNNN

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am modifying authorization mechanisms
    • I am adding new credentials
    • I am modifying OCSP code
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

    Please write a short description of how your code change solves the related issue.

  4. (Optional) PR for stored-proc connector:

@sfc-gh-mmishchenko sfc-gh-mmishchenko requested a review from a team as a code owner August 25, 2025 13:17
return ssl_context.wrap_socket(sock, server_hostname=server_hostname)
else:
return ssl_context.wrap_socket(sock)
return ssl_context.wrap_socket(sock, server_hostname=server_hostname)

Check failure

Code scanning / CodeQL

Use of insecure SSL/TLS version High

Insecure SSL/TLS protocol version TLSv1 allowed by
call to SSLContext
.
Insecure SSL/TLS protocol version TLSv1_1 allowed by
call to SSLContext
.

Copilot Autofix

AI 23 days ago

To fix this problem, ensure that the SSL context always enforces TLSv1.2 or higher, regardless of caller-supplied arguments.

  • If the caller or upstream code attempts to set ssl_minimum_version (or underlying ssl_version) to a value lower than TLSVersion.TLSv1_2, raise a ValueError—prevent creation of a context allowing insecure protocols.
  • The check should happen after resolving the minimum version, before setting context.minimum_version.
  • Only edits in src/snowflake/connector/vendored/urllib3/util/ssl_.py in the shown region are required.
  • No new imports or dependencies are necessary, but add an explicit check before setting context.minimum_version.
  • Edits are to lines after line 302, just before setting context.minimum_version, to add an additional check.

Suggested changeset 1
src/snowflake/connector/vendored/urllib3/util/ssl_.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/snowflake/connector/vendored/urllib3/util/ssl_.py b/src/snowflake/connector/vendored/urllib3/util/ssl_.py
--- a/src/snowflake/connector/vendored/urllib3/util/ssl_.py
+++ b/src/snowflake/connector/vendored/urllib3/util/ssl_.py
@@ -298,6 +298,12 @@
     context = SSLContext(PROTOCOL_TLS_CLIENT)
 
     if ssl_minimum_version is not None:
+        # Prevent insecure protocols from being allowed
+        if ssl_minimum_version < TLSVersion.TLSv1_2:
+            raise ValueError(
+                "Weak or insecure SSL/TLS protocols (TLSv1/TLSv1.1) are not supported. "
+                "Please use TLSv1_2 or higher."
+            )
         context.minimum_version = ssl_minimum_version
     else:  # Python <3.10 defaults to 'MINIMUM_SUPPORTED' so explicitly set TLSv1.2 here
         context.minimum_version = TLSVersion.TLSv1_2
EOF
@@ -298,6 +298,12 @@
context = SSLContext(PROTOCOL_TLS_CLIENT)

if ssl_minimum_version is not None:
# Prevent insecure protocols from being allowed
if ssl_minimum_version < TLSVersion.TLSv1_2:
raise ValueError(
"Weak or insecure SSL/TLS protocols (TLSv1/TLSv1.1) are not supported. "
"Please use TLSv1_2 or higher."
)
context.minimum_version = ssl_minimum_version
else: # Python <3.10 defaults to 'MINIMUM_SUPPORTED' so explicitly set TLSv1.2 here
context.minimum_version = TLSVersion.TLSv1_2
Copilot is powered by AI and may make mistakes. Always verify output.
@sfc-gh-mmishchenko sfc-gh-mmishchenko force-pushed the SNOW-896926-upgrade-vendored-urllib3-and-requests branch from 9f3d417 to 2e84e1b Compare September 5, 2025 10:18
Copy link
Contributor

@sfc-gh-fpawlowski sfc-gh-fpawlowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume tests can be failing for this branch - without our adjustments (to be added in PR) - so this one looks good 👍

@sfc-gh-dszmolka sfc-gh-dszmolka requested review from a team and sfc-gh-pczajka September 11, 2025 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants