Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependencies on Cryptodome and oscrypto #1616

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DESCRIPTION.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
- Added thread safety in telemetry when instantiating multiple connections concurrently.
- Bumped platformdirs dependency from >=2.6.0,<3.9.0 to >=2.6.0,<4.0.0.0 and made necessary changes to allow this.
- Removed the deprecation warning from the vendored urllib3 about urllib3.contrib.pyopenssl deprecation.
- Remove dependencies on Cryptodome and oscrypto and remove the `use_openssl_only` parameter. All connections now go through OpenSSL via the cryptography library, which was already a dependency.
- Improved robustness in handling authentication response.

- v3.2.0(September 06,2023)
Expand Down
1 change: 0 additions & 1 deletion ci/test_fips_docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ user_id=$(id -u $USER)
docker run --network=host \
-e LANG=en_US.UTF-8 \
-e TERM=vt102 \
-e SF_USE_OPENSSL_ONLY=True \
-e PIP_DISABLE_PIP_VERSION_CHECK=1 \
-e LOCAL_USER_ID=$user_id \
-e CRYPTOGRAPHY_ALLOW_OPENSSL_102=1 \
Expand Down
2 changes: 0 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ install_requires =
asn1crypto>0.24.0,<2.0.0
cffi>=1.9,<2.0.0
cryptography>=3.1.0,<42.0.0
oscrypto<2.0.0
pyOpenSSL>=16.2.0,<24.0.0
pycryptodomex!=3.5.0,>=3.2,<4.0.0
pyjwt<3.0.0
pytz
requests<3.0.0
Expand Down
21 changes: 4 additions & 17 deletions src/snowflake/connector/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ def DefaultConverterClass() -> type:
"client_store_temporary_credential": (False, bool),
"client_request_mfa_token": (False, bool),
"use_openssl_only": (
False,
True,
bool,
), # only use openssl instead of python only crypto modules
), # ignored - python only crypto modules are no longer used
# whether to convert Arrow number values to decimal instead of doubles
"arrow_number_to_decimal": (False, bool),
"enable_stage_s3_privatelink_for_us_east_1": (
Expand Down Expand Up @@ -282,7 +282,6 @@ class SnowflakeConnection:
validate_default_parameters: Validate database, schema, role and warehouse used on Snowflake.
is_pyformat: Whether the current argument binding is pyformat or format.
consent_cache_id_token: Consented cache ID token.
use_openssl_only: Use OpenSSL instead of pure Python libraries for signature verification and encryption.
enable_stage_s3_privatelink_for_us_east_1: when true, clients use regional s3 url to upload files.
enable_connection_diag: when true, clients will generate a connectivity diagnostic report.
connection_diag_log_path: path to location to create diag report with enable_connection_diag.
Expand Down Expand Up @@ -564,7 +563,8 @@ def disable_request_pooling(self, value) -> None:

@property
def use_openssl_only(self) -> bool:
return self._use_openssl_only
# Deprecated, kept for backwards compatibility
return True

@property
def arrow_number_to_decimal(self):
Expand Down Expand Up @@ -1083,19 +1083,6 @@ def __config(self, **kwargs):
"CHECKED."
)

if "SF_USE_OPENSSL_ONLY" not in os.environ:
logger.info("Setting use_openssl_only mode to %s", self.use_openssl_only)
os.environ["SF_USE_OPENSSL_ONLY"] = str(self.use_openssl_only)
elif (
os.environ.get("SF_USE_OPENSSL_ONLY", "False") == "True"
) != self.use_openssl_only:
logger.warning(
"Mode use_openssl_only is already set to: %s, ignoring set request to: %s",
os.environ["SF_USE_OPENSSL_ONLY"],
self.use_openssl_only,
)
self._use_openssl_only = os.environ["SF_USE_OPENSSL_ONLY"] == "True"

def cmd_query(
self,
sql: str,
Expand Down
70 changes: 19 additions & 51 deletions src/snowflake/connector/encryption_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from logging import getLogger
from typing import IO, TYPE_CHECKING

from Cryptodome.Cipher import AES
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes

Expand Down Expand Up @@ -69,7 +68,6 @@ def encrypt_stream(
The encryption metadata.
"""
logger = getLogger(__name__)
use_openssl_only = os.getenv("SF_USE_OPENSSL_ONLY", "False") == "True"
decoded_key = base64.standard_b64decode(
encryption_material.query_stage_master_key
)
Expand All @@ -79,14 +77,9 @@ def encrypt_stream(
# Generate key for data encryption
iv_data = SnowflakeEncryptionUtil.get_secure_random(block_size)
file_key = SnowflakeEncryptionUtil.get_secure_random(key_size)
if not use_openssl_only:
data_cipher = AES.new(key=file_key, mode=AES.MODE_CBC, IV=iv_data)
else:
backend = default_backend()
cipher = Cipher(
algorithms.AES(file_key), modes.CBC(iv_data), backend=backend
)
encryptor = cipher.encryptor()
backend = default_backend()
cipher = Cipher(algorithms.AES(file_key), modes.CBC(iv_data), backend=backend)
encryptor = cipher.encryptor()

padded = False
while True:
Expand All @@ -96,30 +89,17 @@ def encrypt_stream(
elif len(chunk) % block_size != 0:
chunk = PKCS5_PAD(chunk, block_size)
padded = True
if not use_openssl_only:
out.write(data_cipher.encrypt(chunk))
else:
out.write(encryptor.update(chunk))
out.write(encryptor.update(chunk))
if not padded:
if not use_openssl_only:
out.write(
data_cipher.encrypt(block_size * chr(block_size).encode(UTF8))
)
else:
out.write(encryptor.update(block_size * chr(block_size).encode(UTF8)))
if use_openssl_only:
out.write(encryptor.finalize())
out.write(data_cipher.encrypt(block_size * chr(block_size).encode(UTF8)))
out.write(encryptor.finalize())

# encrypt key with QRMK
if not use_openssl_only:
key_cipher = AES.new(key=decoded_key, mode=AES.MODE_ECB)
enc_kek = key_cipher.encrypt(PKCS5_PAD(file_key, block_size))
else:
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()
)
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()
)

mat_desc = MaterialDescriptor(
smk_id=encryption_material.smk_id,
Expand Down Expand Up @@ -178,7 +158,6 @@ def decrypt_stream(
) -> None:
"""To read from `src` stream then decrypt to `out` stream."""

use_openssl_only = os.getenv("SF_USE_OPENSSL_ONLY", "False") == "True"
key_base64 = metadata.key
iv_base64 = metadata.iv
decoded_key = base64.standard_b64decode(
Expand All @@ -187,37 +166,26 @@ def decrypt_stream(
key_bytes = base64.standard_b64decode(key_base64)
iv_bytes = base64.standard_b64decode(iv_base64)

if not use_openssl_only:
key_cipher = AES.new(key=decoded_key, mode=AES.MODE_ECB)
file_key = PKCS5_UNPAD(key_cipher.decrypt(key_bytes))
data_cipher = AES.new(key=file_key, mode=AES.MODE_CBC, IV=iv_bytes)
else:
backend = default_backend()
cipher = Cipher(algorithms.AES(decoded_key), modes.ECB(), backend=backend)
decryptor = cipher.decryptor()
file_key = PKCS5_UNPAD(decryptor.update(key_bytes) + decryptor.finalize())
cipher = Cipher(
algorithms.AES(file_key), modes.CBC(iv_bytes), backend=backend
)
decryptor = cipher.decryptor()
backend = default_backend()
cipher = Cipher(algorithms.AES(decoded_key), modes.ECB(), backend=backend)
decryptor = cipher.decryptor()
file_key = PKCS5_UNPAD(decryptor.update(key_bytes) + decryptor.finalize())
cipher = Cipher(algorithms.AES(file_key), modes.CBC(iv_bytes), backend=backend)
decryptor = cipher.decryptor()

last_decrypted_chunk = None
chunk = src.read(chunk_size)
while len(chunk) != 0:
if last_decrypted_chunk is not None:
out.write(last_decrypted_chunk)
if not use_openssl_only:
d = data_cipher.decrypt(chunk)
else:
d = decryptor.update(chunk)
d = decryptor.update(chunk)
last_decrypted_chunk = d
chunk = src.read(chunk_size)

if last_decrypted_chunk is not None:
offset = PKCS5_OFFSET(last_decrypted_chunk)
out.write(last_decrypted_chunk[:-offset])
if use_openssl_only:
out.write(decryptor.finalize())
out.write(decryptor.finalize())

@staticmethod
def decrypt_file(
Expand Down
23 changes: 6 additions & 17 deletions src/snowflake/connector/file_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from logging import getLogger
from typing import IO

from Cryptodome.Hash import SHA256
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import hashes

Expand All @@ -33,27 +32,17 @@ def get_digest_and_size(src: IO[bytes]) -> tuple[str, int]:
Returns:
Tuple of src's digest and src's size in bytes.
"""
use_openssl_only = os.getenv("SF_USE_OPENSSL_ONLY", "False") == "True"
CHUNK_SIZE = 64 * kilobyte
if not use_openssl_only:
m = SHA256.new()
else:
backend = default_backend()
chosen_hash = hashes.SHA256()
hasher = hashes.Hash(chosen_hash, backend)
backend = default_backend()
chosen_hash = hashes.SHA256()
hasher = hashes.Hash(chosen_hash, backend)
while True:
chunk = src.read(CHUNK_SIZE)
if chunk == b"":
break
if not use_openssl_only:
m.update(chunk)
else:
hasher.update(chunk)

if not use_openssl_only:
digest = base64.standard_b64encode(m.digest()).decode(UTF8)
else:
digest = base64.standard_b64encode(hasher.finalize()).decode(UTF8)
hasher.update(chunk)

digest = base64.standard_b64encode(hasher.finalize()).decode(UTF8)

size = src.tell()
src.seek(0)
Expand Down
84 changes: 20 additions & 64 deletions src/snowflake/connector/ocsp_asn1crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@
Version,
)
from asn1crypto.x509 import Certificate
from Cryptodome.Hash import SHA1, SHA256, SHA384, SHA512
from Cryptodome.PublicKey import RSA
from Cryptodome.Signature import PKCS1_v1_5
from cryptography.exceptions import InvalidSignature
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import hashes, serialization
Expand All @@ -48,19 +45,6 @@
from snowflake.connector.errors import RevocationCheckError
from snowflake.connector.ocsp_snowflake import SnowflakeOCSP, generate_cache_key

with warnings.catch_warnings():
warnings.simplefilter("ignore")
# force versioned dylibs onto oscrypto ssl on catalina
if sys.platform == "darwin" and platform.mac_ver()[0].startswith("10.15"):
from oscrypto import _module_values, use_openssl

if _module_values["backend"] is None:
use_openssl(
libcrypto_path="/usr/lib/libcrypto.35.dylib",
libssl_path="/usr/lib/libssl.35.dylib",
)
from oscrypto import asymmetric


logger = getLogger(__name__)

Expand All @@ -70,12 +54,6 @@ class SnowflakeOCSPAsn1Crypto(SnowflakeOCSP):

# map signature algorithm name to digest class
SIGNATURE_ALGORITHM_TO_DIGEST_CLASS = {
"sha256": SHA256,
"sha384": SHA384,
"sha512": SHA512,
}

SIGNATURE_ALGORITHM_TO_DIGEST_CLASS_OPENSSL = {
"sha256": hashes.SHA256,
"sha384": hashes.SHA3_384,
"sha512": hashes.SHA3_512,
Expand Down Expand Up @@ -378,51 +356,29 @@ def process_ocsp_response(self, issuer, cert_id, ocsp_response):
raise RevocationCheckError(msg=debug_msg, errno=op_er.errno)

def verify_signature(self, signature_algorithm, signature, cert, data):
use_openssl_only = os.getenv("SF_USE_OPENSSL_ONLY", "False") == "True"
if not use_openssl_only:
pubkey = asymmetric.load_public_key(cert.public_key).unwrap().dump()
rsakey = RSA.importKey(pubkey)
signer = PKCS1_v1_5.new(rsakey)
if (
backend = default_backend()
public_key = serialization.load_der_public_key(
cert.public_key.dump(), backend=default_backend()
)
if (
signature_algorithm
in SnowflakeOCSPAsn1Crypto.SIGNATURE_ALGORITHM_TO_DIGEST_CLASS
):
chosen_hash = SnowflakeOCSPAsn1Crypto.SIGNATURE_ALGORITHM_TO_DIGEST_CLASS[
signature_algorithm
in SnowflakeOCSPAsn1Crypto.SIGNATURE_ALGORITHM_TO_DIGEST_CLASS
):
digest = SnowflakeOCSPAsn1Crypto.SIGNATURE_ALGORITHM_TO_DIGEST_CLASS[
signature_algorithm
].new()
else:
# the last resort. should not happen.
digest = SHA1.new()
digest.update(data.dump())
if not signer.verify(digest, signature):
raise RevocationCheckError(msg="Failed to verify the signature")

]()
else:
backend = default_backend()
public_key = serialization.load_der_public_key(
cert.public_key.dump(), backend=default_backend()
# the last resort. should not happen.
chosen_hash = hashes.SHA1()
hasher = hashes.Hash(chosen_hash, backend)
hasher.update(data.dump())
digest = hasher.finalize()
try:
public_key.verify(
signature, digest, padding.PKCS1v15(), utils.Prehashed(chosen_hash)
)
if (
signature_algorithm
in SnowflakeOCSPAsn1Crypto.SIGNATURE_ALGORITHM_TO_DIGEST_CLASS
):
chosen_hash = (
SnowflakeOCSPAsn1Crypto.SIGNATURE_ALGORITHM_TO_DIGEST_CLASS_OPENSSL[
signature_algorithm
]()
)
else:
# the last resort. should not happen.
chosen_hash = hashes.SHA1()
hasher = hashes.Hash(chosen_hash, backend)
hasher.update(data.dump())
digest = hasher.finalize()
try:
public_key.verify(
signature, digest, padding.PKCS1v15(), utils.Prehashed(chosen_hash)
)
except InvalidSignature:
raise RevocationCheckError(msg="Failed to verify the signature")
except InvalidSignature:
raise RevocationCheckError(msg="Failed to verify the signature")

def extract_certificate_chain(
self, connection: Connection
Expand Down
29 changes: 0 additions & 29 deletions test/integ/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,35 +681,6 @@ def mock_auth(self, auth_instance):
assert cnx


@pytest.mark.skipolddriver
def test_use_openssl_only(db_parameters):
cnx = snowflake.connector.connect(
user=db_parameters["user"],
password=db_parameters["password"],
host=db_parameters["host"],
port=db_parameters["port"],
account=db_parameters["account"],
protocol=db_parameters["protocol"],
use_openssl_only=True,
)
assert cnx
assert "SF_USE_OPENSSL_ONLY" in os.environ
# Note during testing conftest will default this value to False, so if testing this we need to manually clear it
# Let's test it again, after clearing it
del os.environ["SF_USE_OPENSSL_ONLY"]
cnx = snowflake.connector.connect(
user=db_parameters["user"],
password=db_parameters["password"],
host=db_parameters["host"],
port=db_parameters["port"],
account=db_parameters["account"],
protocol=db_parameters["protocol"],
use_openssl_only=True,
)
assert cnx
assert os.environ["SF_USE_OPENSSL_ONLY"] == "True"


def test_dashed_url(db_parameters):
"""Test whether dashed URLs get created correctly."""
with mock.patch(
Expand Down
Loading