#11826 Conditionally skip tests that require METHOD_CRYPT #11827
Conversation
mweinelt
force-pushed
the
skipif-crypt_method_crypt
branch
from
eb3b944
to
59886b5
Compare
mweinelt
force-pushed
the
skipif-crypt_method_crypt
branch
from
59886b5
to
6ae4f1a
Compare
The advent of libxcrypt offers new password hashing algorithms to linux distributions, which we are slowly rolling out to our users. At the same time we are removing support for legacy ciphers, that should not be used any longer in a security-context. One of these is the traditional crypt method, which the python documentation¹ describes aptly as the "weakest method" it supports. This change adds a skip condition to the relevant tests, that checks for the presence of crypt.METHOD_CRYPT in the list of supported hashes. [1] https://docs.python.org/3/library/crypt.html#crypt.METHOD_CRYPT Closes: twisted#11826
mweinelt
force-pushed
the
skipif-crypt_method_crypt
branch
from
6ae4f1a
to
e69e652
Compare
|
please review |
|
Thanks for the PR. I don't know what to say, as we don't have a GHA test environment for this. I guess that we can merge this. At the same time, I think that crypt will be removed in Python 3.13, so we should look into deprecating this code. |
There was a problem hiding this comment.
Thanks for the PR.
I guess that we can merge this.
It looks like we don't have the resources to setup an nixOS testing environment for GitHub Action.
But since we need to remove the usage for crypt library for Python 3.13, maybe this can help a bit to identify the places where crypt is used.
| else: | ||
| cryptSkip = None | ||
| has_method_crypt = getattr(crypt, "METHOD_CRYPT", None) in crypt.methods |
There was a problem hiding this comment.
| has_method_crypt = getattr(crypt, "METHOD_CRYPT", None) in crypt.methods | |
| # We have the crypt main library. | |
| # But some Python builds might now have the the | |
| # weakest `crypt.crypt()` method. | |
| has_method_crypt = getattr(crypt, "METHOD_CRYPT", None) in crypt.methods |
|
|
||
| cryptSkip: Optional[str] | ||
| try: | ||
| import crypt | ||
| except ImportError: | ||
| cryptSkip = "cannot run without crypt module" | ||
| has_method_crypt = False |
There was a problem hiding this comment.
Should be hasCryptMethod to follow the Twisted naming convention
| @@ -64,6 +67,7 @@ class HelperTests(TestCase): | |||
| def setUp(self): | |||
| self.mockos = MockOS() | |||
|
|
|||
| @skipIf(not has_method_crypt, "Required crypt method is unavailable: METHOD_CRYPT") | |||
There was a problem hiding this comment.
This line could be duplicated a lot less.
|
Looks like the 3.13 builder has covered the lines that were previously skipped! |
Scope and purpose
Fixes #11826
The advent of libxcrypt offers new password hashing algorithms to linux distributions, which we are slowly rolling out to our users.
At the same time we are removing support for legacy ciphers, that should not be used any longer in a security-context.
One of these is the traditional crypt method, which the python documentation¹ describes aptly as the "weakest method" it supports.
This change adds a skip condition to the relevant tests, that checks for the presence of crypt.METHOD_CRYPT in the list of supported hashes.
[1] https://docs.python.org/3/library/crypt.html#crypt.METHOD_CRYPT
Contributor Checklist:
This process applies to all pull requests - no matter how small.
Have a look at our developer documentation before submitting your Pull Request.
Below is a non-exhaustive list (as a reminder):
please review.Our bot will trigger the review process, by applying the pending review label
and requesting a review from the Twisted dev team.