-
Notifications
You must be signed in to change notification settings - Fork 524
SNOW-2176203: Support intermediates in trust stores #2520
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
69c8755
SNOW-2176203: Support intermediates in trust stores
sfc-gh-snoonan 817bcc8
Narrow exceptions and add helpers
sfc-gh-snoonan d2a4d29
VERIFY_X509_PARTIAL_CHAIN
sfc-gh-snoonan 7604784
Merge branch 'main' into SNOW-2176203-partial-chains
sfc-gh-snoonan 344b7eb
Update DESCRIPTION.md
sfc-gh-snoonan 009d2b8
Update test/unit/test_ssl_partial_chain.py
sfc-gh-snoonan de34cd9
Merge branch 'main' into SNOW-2176203-partial-chains
sfc-gh-snoonan d7f5260
black run
sfc-gh-snoonan 1e9f9b1
safer arg handling for our ssl_wrap_socket
sfc-gh-snoonan c63560a
Use cryptography module rather than deprecated x509 module
sfc-gh-snoonan 4539153
Fix spelling error in intermediate certificates entry
sfc-gh-snoonan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| """Unit tests for SSL wrapper partial-chain context injection.""" | ||
|
|
||
| import types | ||
|
|
||
| import pytest | ||
|
|
||
| import snowflake.connector.ssl_wrap_socket as ssw # pylint: disable=import-error | ||
| from snowflake.connector.constants import OCSPMode # pylint: disable=import-error | ||
| from snowflake.connector.vendored.urllib3.contrib.pyopenssl import ( # pylint: disable=import-error | ||
| PyOpenSSLContext, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def disable_ocsp_checks(): | ||
| """Disable OCSP checks for offline unit testing.""" | ||
| # Ensure wrapper doesn't perform OCSP to keep this unit test offline | ||
| orig = ssw.FEATURE_OCSP_MODE | ||
| ssw.FEATURE_OCSP_MODE = OCSPMode.DISABLE_OCSP_CHECKS | ||
| try: | ||
| yield | ||
| finally: | ||
| ssw.FEATURE_OCSP_MODE = orig | ||
|
|
||
|
|
||
| def test_wrapper_injects_pyopenssl_context(monkeypatch): | ||
| """Wrapper should inject a PyOpenSSLContext when none is given.""" | ||
| captured = {} | ||
|
|
||
| def fake_ssl_wrap_socket( # pylint: disable=unused-argument,too-many-arguments,too-many-positional-arguments | ||
| sock, | ||
| keyfile=None, | ||
| certfile=None, | ||
| cert_reqs=None, | ||
| ca_certs=None, | ||
| server_hostname=None, | ||
| ssl_version=None, | ||
| ciphers=None, | ||
| ssl_context=None, | ||
| ca_cert_dir=None, | ||
| key_password=None, | ||
| ca_cert_data=None, | ||
| tls_in_tls=False, | ||
| ): | ||
| # Assert that our wrapper provided a PyOpenSSLContext | ||
| captured["ctx_is_pyopenssl"] = isinstance(ssl_context, PyOpenSSLContext) | ||
| # Return a minimal object with a 'connection' attribute expected by wrapper | ||
| return types.SimpleNamespace(connection=None) | ||
|
|
||
| # Patch underlying urllib3 ssl_wrap_socket used by our wrapper | ||
| monkeypatch.setattr(ssw.ssl_, "ssl_wrap_socket", fake_ssl_wrap_socket) | ||
|
|
||
| # Call our wrapper without providing ssl_context; it should inject one | ||
| ssw.ssl_wrap_socket_with_ocsp( | ||
| sock=None, | ||
| keyfile=None, | ||
| certfile=None, | ||
| cert_reqs=None, | ||
| ca_certs=None, | ||
| server_hostname="localhost", | ||
| ssl_version=None, | ||
| ciphers=None, | ||
| ssl_context=None, | ||
| ca_cert_dir=None, | ||
| key_password=None, | ||
| ca_cert_data=None, | ||
| tls_in_tls=False, | ||
| ) | ||
|
|
||
| assert captured.get("ctx_is_pyopenssl") is True | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,176 @@ | ||
| """Integration-style unit test for partial-chain TLS handshake.""" | ||
|
|
||
| import socket | ||
| import ssl | ||
| import tempfile as _tempfile | ||
| import threading | ||
|
|
||
| import OpenSSL | ||
| import pytest | ||
|
|
||
| import snowflake.connector.ssl_wrap_socket as ssw # pylint: disable=import-error | ||
| from snowflake.connector.constants import OCSPMode # pylint: disable=import-error | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def disable_ocsp_checks(): | ||
| """Disable OCSP checks for offline unit testing.""" | ||
| orig = ssw.FEATURE_OCSP_MODE | ||
| ssw.FEATURE_OCSP_MODE = OCSPMode.DISABLE_OCSP_CHECKS | ||
| try: | ||
| yield | ||
| finally: | ||
| ssw.FEATURE_OCSP_MODE = orig | ||
|
|
||
|
|
||
| def _create_key(): | ||
| """Create a new RSA key for certificate generation.""" | ||
| k = OpenSSL.crypto.PKey() | ||
| k.generate_key(OpenSSL.crypto.TYPE_RSA, 2048) | ||
| return k | ||
|
|
||
|
|
||
| def _create_cert(subject_cn, issuer_cert, issuer_key, is_ca, subject_key, ca=False): | ||
| """Create a certificate signed by issuer or self-signed if issuer is None.""" | ||
| cert = OpenSSL.crypto.X509() | ||
| cert.set_version(2) | ||
| cert.set_serial_number(1) | ||
| subj = cert.get_subject() | ||
| subj.CN = subject_cn | ||
| cert.gmtime_adj_notBefore(0) | ||
| cert.gmtime_adj_notAfter(60 * 60) | ||
| cert.set_pubkey(subject_key) | ||
| if issuer_cert is None: | ||
| issuer = subj | ||
| else: | ||
| issuer = issuer_cert.get_subject() | ||
| cert.set_issuer(issuer) | ||
| if is_ca: | ||
| cert.add_extensions( | ||
| [ | ||
| OpenSSL.crypto.X509Extension( | ||
| b"basicConstraints", True, b"CA:TRUE, pathlen:1" | ||
| ), | ||
| OpenSSL.crypto.X509Extension( | ||
| b"keyUsage", True, b"keyCertSign, cRLSign" | ||
| ), | ||
| OpenSSL.crypto.X509Extension( | ||
| b"subjectKeyIdentifier", False, b"hash", subject=cert | ||
| ), | ||
| ] | ||
| ) | ||
| else: | ||
| cert.add_extensions( | ||
| [ | ||
| OpenSSL.crypto.X509Extension(b"basicConstraints", True, b"CA:FALSE"), | ||
| OpenSSL.crypto.X509Extension(b"extendedKeyUsage", False, b"serverAuth"), | ||
| OpenSSL.crypto.X509Extension( | ||
| b"subjectAltName", False, b"DNS:localhost,IP:127.0.0.1" | ||
| ), | ||
| ] | ||
| ) | ||
| if issuer_cert is not None: | ||
| cert.add_extensions( | ||
| [ | ||
| OpenSSL.crypto.X509Extension( | ||
| b"authorityKeyIdentifier", | ||
| False, | ||
| b"keyid:always,issuer:always", | ||
| issuer=issuer_cert, | ||
| ), | ||
| ] | ||
| ) | ||
| cert.sign(issuer_key if issuer_key is not None else subject_key, "sha256") | ||
| return cert | ||
|
|
||
|
|
||
| def _pem(obj, is_key=False): | ||
| """Return PEM-encoded certificate or key.""" | ||
| if is_key: | ||
| return OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, obj) | ||
| return OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM, obj) | ||
|
|
||
|
|
||
| def _run_tls_server(server_cert_pem, server_key_pem, chain_pem, ready_evt, addr_holder): | ||
| """Run a minimal TLS server presenting server+intermediate chain.""" | ||
| # Minimal TLS server using Python ssl to present server+intermediate chain | ||
| ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) | ||
| # Combine server and intermediate into one PEM for certfile | ||
| with _tempfile.NamedTemporaryFile(delete=False) as cert_chain_file: | ||
| cert_chain_file.write(server_cert_pem) | ||
| cert_chain_file.write(chain_pem) | ||
| cert_chain_file.flush() | ||
| certfile_path = cert_chain_file.name | ||
| with _tempfile.NamedTemporaryFile(delete=False) as key_file: | ||
| key_file.write(server_key_pem) | ||
| key_file.flush() | ||
| keyfile_path = key_file.name | ||
| ctx.load_cert_chain(certfile=certfile_path, keyfile=keyfile_path) | ||
|
|
||
| s = socket.socket() | ||
| s.bind(("127.0.0.1", 0)) | ||
| s.listen(1) | ||
| addr = s.getsockname() | ||
| addr_holder.append(addr) | ||
| ready_evt.set() | ||
| with ctx.wrap_socket(s, server_side=True) as ssock: | ||
|
||
| conn, _ = ssock.accept() | ||
| conn.close() | ||
| s.close() | ||
|
|
||
|
|
||
| def test_partial_chain_handshake_succeeds_with_intermediate_as_anchor(): | ||
| """Client should handshake trusting only the intermediate as anchor.""" | ||
| # Generate Root -> Intermediate -> Server | ||
| root_key = _create_key() | ||
| root_cert = _create_cert("Root", None, None, True, root_key) | ||
|
|
||
| inter_key = _create_key() | ||
| inter_cert = _create_cert("Intermediate", root_cert, root_key, True, inter_key) | ||
|
|
||
| server_key = _create_key() | ||
| server_cert = _create_cert("Server", inter_cert, inter_key, False, server_key) | ||
|
|
||
| # Start TLS server presenting server + intermediate chain | ||
| ready_evt = threading.Event() | ||
| addr_holder = [] | ||
| t = threading.Thread( | ||
| target=_run_tls_server, | ||
| args=( | ||
| _pem(server_cert), | ||
| _pem(server_key, True), | ||
| _pem(inter_cert), | ||
| ready_evt, | ||
| addr_holder, | ||
| ), | ||
| ) | ||
| t.daemon = True | ||
| t.start() | ||
| ready_evt.wait(5) | ||
| host, port = addr_holder[0] | ||
|
|
||
| # Build PyOpenSSL context with only intermediate as trust anchor | ||
| ctx = ssw._build_pyopenssl_context_with_ca_and_partial_chain( | ||
| None | ||
| ) # pylint: disable=protected-access | ||
| # Load intermediate into store via PEM file path by reusing helper | ||
| with _tempfile.NamedTemporaryFile(delete=False) as caf: | ||
| caf.write(_pem(inter_cert)) | ||
| caf.flush() | ||
| ctx.load_verify_locations(cafile=caf.name) | ||
|
|
||
| # Wrap a socket with our wrapper specifying our context | ||
| s = socket.socket() | ||
| s.settimeout(5) | ||
| s.connect((host, port)) | ||
|
|
||
| # The wrapper expects kwargs similar to urllib3; use provided context | ||
| ws = ssw.ssl_wrap_socket_with_ocsp( | ||
| sock=s, | ||
| server_hostname="localhost", | ||
| ssl_context=ctx, | ||
| ) | ||
| # If we reached here without SSL error, TLS handshake succeeded with | ||
| # intermediate-only trust; access attribute to assert presence | ||
| assert hasattr(ws, "connection") | ||
| s.close() | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.