-
-
Notifications
You must be signed in to change notification settings - Fork 98
Fix manual prompt in pyopenssl adapter for private key password #798
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,10 +149,18 @@ def make_tls_http_server(bind_addr, ssl_adapter, request): | |
| return httpserver | ||
|
|
||
|
|
||
| def get_key_password(): | ||
| """Return a predefined password string. | ||
|
|
||
| It is to be used for decrypting private keys. | ||
| """ | ||
| return 'ΠΊΡΠΈΡΠ²ΠΊΠ°' | ||
|
|
||
|
|
||
| @pytest.fixture(scope='session') | ||
| def private_key_password(): | ||
| """Provide hardcoded password for private key.""" | ||
| return 'ΠΊΡΠΈΡΠ²ΠΊΠ°' | ||
| return get_key_password() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
|
|
@@ -900,9 +908,17 @@ def test_http_over_https_ssl_handshake( | |
| ids=('encrypted-key', 'unencrypted-key'), | ||
| ) | ||
| @pytest.mark.parametrize( | ||
| 'password_as_bytes', | ||
| (True, False), | ||
| ids=('with-bytes-password', 'with-str-password'), | ||
| 'transform_password_arg', | ||
| ( | ||
| lambda pass_factory: pass_factory().encode('utf-8'), | ||
| lambda pass_factory: pass_factory(), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jatalahd so I replaced the deprecated The CI now fails on that specific VM under PyPy 3.11 with a SEGFAULT in The last successful PR CI run wasn't running pypy under macos: https://github.com/cherrypy/cheroot/actions/runs/20009127282. I just forgot to skip it when changing VMs. This means that the pathc in this PR isn't necessarily related to the problem but if you have a minute, it might be a good idea to look closer in case this is something that might start happening under classic CPython too...
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although.. The surrounding comments suggest pre-existing issues with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With 41a44f1, the CI should be fine again.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, hopefully it is fine now. I did not see any test issues when running the unit tests locally with Python 3.11 in MacOS15.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this was just PyPy, not CPython.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this was just PyPy, not CPython. |
||
| lambda pass_factory: pass_factory, | ||
| ), | ||
| ids=( | ||
| 'with-bytes-password', | ||
| 'with-str-password', | ||
| 'with-callable-password-provider', | ||
| ), | ||
| ) | ||
| # pylint: disable-next=too-many-positional-arguments | ||
| def test_ssl_adapters_with_private_key_password( | ||
|
|
@@ -915,25 +931,21 @@ def test_ssl_adapters_with_private_key_password( | |
| tls_certificate_private_key_pem_path, | ||
| adapter_type, | ||
| encrypted_key, | ||
| password_as_bytes, | ||
| transform_password_arg, | ||
| ): | ||
| """Check server decrypts private TLS keys with password as bytes or str.""" | ||
| key_file = ( | ||
| tls_certificate_passwd_private_key_pem_path | ||
| if encrypted_key | ||
| else tls_certificate_private_key_pem_path | ||
| ) | ||
| key_pass = ( | ||
| private_key_password.encode('utf-8') | ||
| if password_as_bytes | ||
| else private_key_password | ||
| ) | ||
| private_key_password = transform_password_arg(get_key_password) | ||
|
|
||
| tls_adapter_cls = get_ssl_adapter_class(name=adapter_type) | ||
| tls_adapter = tls_adapter_cls( | ||
| certificate=tls_certificate_chain_pem_path, | ||
| private_key=key_file, | ||
| private_key_password=key_pass, | ||
| private_key_password=private_key_password, | ||
| ) | ||
|
|
||
| interface, _host, port = _get_conn_data( | ||
|
|
@@ -1015,6 +1027,76 @@ def test_openssl_adapter_with_false_key_password( | |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| 'adapter_type', | ||
| ('pyopenssl', 'builtin'), | ||
| ) | ||
| def test_ssl_adapter_with_none_key_password( | ||
| tls_certificate_chain_pem_path, | ||
| tls_certificate_passwd_private_key_pem_path, | ||
| private_key_password, | ||
| adapter_type, | ||
| mocker, | ||
| ): | ||
| """Check that TLS-adapters prompt for password when set as ``None``.""" | ||
| tls_adapter_cls = get_ssl_adapter_class(name=adapter_type) | ||
| mocker.patch( | ||
| 'cheroot.ssl._ask_for_password_interactively', | ||
| return_value=private_key_password, | ||
| ) | ||
| tls_adapter = tls_adapter_cls( | ||
| certificate=tls_certificate_chain_pem_path, | ||
| private_key=tls_certificate_passwd_private_key_pem_path, | ||
| ) | ||
|
|
||
| assert tls_adapter.context is not None | ||
|
|
||
|
|
||
| class PasswordCallbackHelper: | ||
| """Collects helper methods for mocking password callback.""" | ||
|
|
||
| def __init__(self, adapter: Adapter): | ||
| """Initialize helper variables.""" | ||
| self.counter = 0 | ||
| self.callback = adapter._password_callback | ||
|
|
||
| def get_password(self): | ||
| """Provide correct password on first call, wrong on other calls.""" | ||
| self.counter += 1 | ||
| return get_key_password() * self.counter | ||
|
|
||
| def verify_twice_callback(self, max_length, _verify_twice, userdata): | ||
| """Establish a mock callback for testing two-factor password prompt.""" | ||
| return self.callback(self, max_length, True, userdata) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize('adapter_type', ('pyopenssl',)) | ||
| def test_openssl_adapter_verify_twice_callback( | ||
| tls_certificate_chain_pem_path, | ||
| tls_certificate_passwd_private_key_pem_path, | ||
| adapter_type, | ||
| mocker, | ||
| ): | ||
| """Check that two-time password verification fails with correct error.""" | ||
| tls_adapter_cls = get_ssl_adapter_class(name=adapter_type) | ||
| helper = PasswordCallbackHelper(tls_adapter_cls) | ||
|
|
||
| mocker.patch( | ||
| 'cheroot.ssl.pyopenssl.pyOpenSSLAdapter._password_callback', | ||
| side_effect=helper.verify_twice_callback, | ||
| ) | ||
|
|
||
| with pytest.raises( | ||
| ValueError, | ||
| match='Verification failed: entered passwords do not match', | ||
| ): | ||
| tls_adapter_cls( | ||
| certificate=tls_certificate_chain_pem_path, | ||
| private_key=tls_certificate_passwd_private_key_pem_path, | ||
| private_key_password=helper.get_password, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def dummy_adapter(monkeypatch): | ||
| """Provide a dummy SSL adapter instance.""" | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good (short of the callable link): https://cheroot--798.org.readthedocs.build/en/798/history/#to-be-included-in-the-next-release
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hopefully ok in the latest push.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, it's now clickable! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| Fixed prompting for the encrypted private key password interactively, | ||
| when the password in not set in the :py:attr:`private_key_password attribute | ||
| <cheroot.ssl.pyopenssl.pyOpenSSLAdapter.private_key_password>` in the | ||
| :py:class:`pyOpenSSL TLS adapter <cheroot.ssl.pyopenssl.pyOpenSSLAdapter>`. | ||
| Also improved the private key password to accept the :py:class:`~collections.abc.Callable` type. | ||
|
|
||
| -- by :user:`jatalahd` | ||
jatalahd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Uh oh!
There was an error while loading. Please reload this page.