Skip to content

Conversation

@sfc-gh-turbaszek
Copy link
Contributor

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:

k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******"
for (k, v) in body["data"].items()
},
{k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" for (k, v) in body["data"].items()},

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.

Copilot Autofix

AI 15 days ago

To prevent leaking sensitive information in logs, we should always mask known sensitive keys such as 'PASSCODE', 'PASSWORD', and similar. For authentication data, it is safest to explicitly blacklist known secret keys, overriding any whitelist mechanism.
The best solution for the problematic lines (149) is to ensure that sensitive keys are always masked, regardless of whitelist configuration. This means instead of using AUTHENTICATION_REQUEST_KEY_WHITELIST to decide what may be shown, we should create a blacklist set (e.g. SENSITIVE_AUTH_KEYS) for keys that must never be logged in clear text.
In the scope allowed, we can define a small set of keys at the top of the file (or with the function), for example:

SENSITIVE_AUTH_KEYS = {"PASSCODE", "PASSWORD", ...}

Then, in the debug logging, we mask any key in SENSITIVE_AUTH_KEYS regardless of other conditions:

{k: ("******" if k in SENSITIVE_AUTH_KEYS else v) for k, v in body["data"].items()}

This change affects only the debug logging statement in src/snowflake/connector/aio/auth/_auth.py, lines 147-150.
No dependency additions are required; all required functionality is available in standard Python.


Suggested changeset 1
src/snowflake/connector/aio/auth/_auth.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/aio/auth/_auth.py b/src/snowflake/connector/aio/auth/_auth.py
--- a/src/snowflake/connector/aio/auth/_auth.py
+++ b/src/snowflake/connector/aio/auth/_auth.py
@@ -46,7 +46,10 @@
 
 logger = logging.getLogger(__name__)
 
+# List of sensitive authentication keys that must never be logged in clear text.
+SENSITIVE_AUTH_KEYS = {"PASSCODE", "PASSWORD"}
 
+
 class Auth(AuthSync):
     async def authenticate(
         self,
@@ -146,7 +148,7 @@
 
         logger.debug(
             "body['data']: %s",
-            {k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" for (k, v) in body["data"].items()},
+            {k: ("******" if k in SENSITIVE_AUTH_KEYS else v) for (k, v) in body["data"].items()},
         )
 
         try:
EOF
@@ -46,7 +46,10 @@

logger = logging.getLogger(__name__)

# List of sensitive authentication keys that must never be logged in clear text.
SENSITIVE_AUTH_KEYS = {"PASSCODE", "PASSWORD"}


class Auth(AuthSync):
async def authenticate(
self,
@@ -146,7 +148,7 @@

logger.debug(
"body['data']: %s",
{k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" for (k, v) in body["data"].items()},
{k: ("******" if k in SENSITIVE_AUTH_KEYS else v) for (k, v) in body["data"].items()},
)

try:
Copilot is powered by AI and may make mistakes. Always verify output.
k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******"
for (k, v) in body["data"].items()
},
{k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" for (k, v) in body["data"].items()},

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.

Copilot Autofix

AI 15 days ago

The fix is to ensure that no sensitive data (such as passwords, passcodes, or tokens) is ever logged, regardless of the contents of AUTHENTICATION_REQUEST_KEY_WHITELIST. The logging block at line 245 should always redact known sensitive fields (e.g., "PASSCODE", "password", "access_token", etc.), using a hardcoded denylist of these keys. For all other keys, the current whitelist approach can be maintained if desired, but the denylist should override the whitelist.

Specifically, in src/snowflake/connector/auth/_auth.py, replace the log statement so that values for any sensitive key (from a hardcoded denylist) are always replaced with "******", regardless of their status in the whitelist. For flexibility and future-proofing, define the denylist (e.g., as AUTH_LOG_REDACT_KEYS) near the logging call or at module-level (as appropriate). Only true non-sensitive parameters may be logged for debugging.

No changes are required in the other files (src/snowflake/connector/auth/_oauth_base.py, src/snowflake/connector/connection.py) since the analysis only detects the data flow to the problematic log at line 245 in _auth.py (unless there are similar logging calls there; none shown in the provided snippets).


Suggested changeset 1
src/snowflake/connector/auth/_auth.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/auth/_auth.py b/src/snowflake/connector/auth/_auth.py
--- a/src/snowflake/connector/auth/_auth.py
+++ b/src/snowflake/connector/auth/_auth.py
@@ -240,9 +240,11 @@
         if session_parameters:
             body["data"]["SESSION_PARAMETERS"] = session_parameters
 
+        # Always redact sensitive fields, regardless of whitelist
+        AUTH_LOG_REDACT_KEYS = {"PASSCODE", "PASSWORD", "ACCESS_TOKEN", "REFRESH_TOKEN", "TOKEN", "ID_TOKEN", "SECRET", "CLIENT_SECRET"}
         logger.debug(
             "body['data']: %s",
-            {k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" for (k, v) in body["data"].items()},
+            {k: "******" if k.upper() in AUTH_LOG_REDACT_KEYS else (v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******") for (k, v) in body["data"].items()},
         )
 
         try:
EOF
@@ -240,9 +240,11 @@
if session_parameters:
body["data"]["SESSION_PARAMETERS"] = session_parameters

# Always redact sensitive fields, regardless of whitelist
AUTH_LOG_REDACT_KEYS = {"PASSCODE", "PASSWORD", "ACCESS_TOKEN", "REFRESH_TOKEN", "TOKEN", "ID_TOKEN", "SECRET", "CLIENT_SECRET"}
logger.debug(
"body['data']: %s",
{k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" for (k, v) in body["data"].items()},
{k: "******" if k.upper() in AUTH_LOG_REDACT_KEYS else (v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******") for (k, v) in body["data"].items()},
)

try:
Copilot is powered by AI and may make mistakes. Always verify output.
enc_kek = (
encryptor.update(PKCS5_PAD(file_key, block_size)) + encryptor.finalize()
)
enc_kek = encryptor.update(PKCS5_PAD(file_key, block_size)) + encryptor.finalize()

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic algorithm High

The block mode ECB
is broken or weak, and should not be used.

Copilot Autofix

AI 15 days ago

How to fix the problem:
Replace the use of AES in ECB mode to encrypt the file key with a secure key wrapping algorithm, such as AES Key Wrap (RFC 3394). Python's cryptography library provides standard-conforming key wrap/unwrap algorithms in cryptography.hazmat.primitives.keywrap.

Detailed fix:

  • Import aes_key_wrap from cryptography.hazmat.primitives.keywrap.
  • At the relevant line, replace the construction of a raw AES-ECB cipher and the manual encryption + padding with a call to aes_key_wrap, passing the KEK (decoded_key) and the key to be wrapped (file_key).
  • The output, enc_kek, will be the wrapped key (there is no need for manual padding, cipher setup, or cipher finalization).
  • Remove any code related to the ECB cipher used for this purpose.

Which files/lines to change:

  • In src/snowflake/connector/encryption_util.py, add the import.
  • In the body of encrypt_stream, replace both the Cipher(algorithms.AES(...), modes.ECB(), ...) block and associated padding/manipulation with a call to aes_key_wrap.
  • Ensure the remainder of the function uses the output as before (returning base64-encoded wrapped keys).
Suggested changeset 1
src/snowflake/connector/encryption_util.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/encryption_util.py b/src/snowflake/connector/encryption_util.py
--- a/src/snowflake/connector/encryption_util.py
+++ b/src/snowflake/connector/encryption_util.py
@@ -11,6 +11,7 @@
 
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
+from cryptography.hazmat.primitives.keywrap import aes_key_wrap
 
 from .compat import PKCS5_OFFSET, PKCS5_PAD, PKCS5_UNPAD
 from .constants import UTF8, EncryptionMetadata, MaterialDescriptor, kilobyte
@@ -91,10 +92,8 @@
             out.write(encryptor.update(block_size * chr(block_size).encode(UTF8)))
         out.write(encryptor.finalize())
 
-        # encrypt key with QRMK
-        cipher = Cipher(algorithms.AES(decoded_key), modes.ECB(), backend=backend)
-        encryptor = cipher.encryptor()
-        enc_kek = encryptor.update(PKCS5_PAD(file_key, block_size)) + encryptor.finalize()
+        # encrypt key with QRMK using AES Key Wrap (RFC 3394)
+        enc_kek = aes_key_wrap(decoded_key, file_key, backend=backend)
 
         mat_desc = MaterialDescriptor(
             smk_id=encryption_material.smk_id,
EOF
@@ -11,6 +11,7 @@

from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
from cryptography.hazmat.primitives.keywrap import aes_key_wrap

from .compat import PKCS5_OFFSET, PKCS5_PAD, PKCS5_UNPAD
from .constants import UTF8, EncryptionMetadata, MaterialDescriptor, kilobyte
@@ -91,10 +92,8 @@
out.write(encryptor.update(block_size * chr(block_size).encode(UTF8)))
out.write(encryptor.finalize())

# encrypt key with QRMK
cipher = Cipher(algorithms.AES(decoded_key), modes.ECB(), backend=backend)
encryptor = cipher.encryptor()
enc_kek = encryptor.update(PKCS5_PAD(file_key, block_size)) + encryptor.finalize()
# encrypt key with QRMK using AES Key Wrap (RFC 3394)
enc_kek = aes_key_wrap(decoded_key, file_key, backend=backend)

mat_desc = MaterialDescriptor(
smk_id=encryption_material.smk_id,
Copilot is powered by AI and may make mistakes. Always verify output.
@sfc-gh-fpawlowski sfc-gh-fpawlowski changed the title NO-SNOW: Introduce ruff formatting SNOW-762783: Introduce ruff formatting Dec 13, 2025
@sfc-gh-fpawlowski sfc-gh-fpawlowski changed the title SNOW-762783: Introduce ruff formatting SNOW-762783: Introduce ruff formatting + improve logging formatting Dec 13, 2025
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