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

Replace crypt module with passlib #2108

Conversation

Chris-Peterson444
Copy link
Contributor

@Chris-Peterson444 Chris-Peterson444 commented Nov 1, 2024

In python3.13, the crypt submodule will be removed from the standard library. This won't affect snap builds until core26, but makes dryrun testing difficult. This PR introduces some logic to shell out to perl to compute the hash in dry-run mode only.

Replace crypt module usage with the passlib module, which is provided by the python3-passlib package. The package is already in Main and doesn't appear to have any open CVEs. This will fix the issue when trying to import the crypt module in python3.13 or later, where it was finally removed.

@Chris-Peterson444 Chris-Peterson444 marked this pull request as draft November 1, 2024 10:57
@Chris-Peterson444 Chris-Peterson444 force-pushed the crypt-alternative-for-dryrun branch 2 times, most recently from 5cba20c to 6869fb0 Compare November 2, 2024 17:36
@Chris-Peterson444 Chris-Peterson444 changed the title Crypt alternative for dryrun Replace crypt module with passlib Nov 2, 2024
subiquity/ui/views/identity.py Outdated Show resolved Hide resolved
subiquitycore/utils.py Outdated Show resolved Hide resolved
@Chris-Peterson444 Chris-Peterson444 marked this pull request as ready for review November 7, 2024 01:38
@Chris-Peterson444
Copy link
Contributor Author

I think reviewing by commit makes sense still, but we can probably just squash all three commits now once we're happy with the changes.

Copy link
Collaborator

@dbungert dbungert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please give it a VM test if you haven't yet.

@Chris-Peterson444
Copy link
Contributor Author

Just tested this on an Oracular install and I was able to login as expected. Based on some out-of-band discussion, I'm going to keep all three commits for historical accuracy but I'll update the commit messages so they're a little more interesting.

The `crypt` module was deprecated in python3.11 and officially removed
in python3.13. This commit introduces equivalent functionality using
the `passlib` module instead. The `passlib` module is listed directly in
the python documentation as a replacement for the `crypt` module[0].
Additionally, this module is already provided by the `python3-passlib`
package in main.

The main functionality of `crypt_password` is not replaced in this commit.
Instead, this commit acts more as a transient commit to demonstrate
that, given the right parameters, we can use `passlib` to provide
equivalent functionality compared with what we expected from the
`crypt`-based implementation.

[0] https://docs.python.org/3.12/library/crypt.html
Replace usage of the `crypt` module from the stdlib with `passlib`,
which is provided by the python3-passlib (main) package. Follow-up
to the previous commit which introduced the `passlib`-based
functionality, and now switches `crypt_password` over to using it.

The equivalence between the two implementations can be further
demonstrated by the canary test passing with just the previous commit
and also passing with this one.
@Chris-Peterson444 Chris-Peterson444 merged commit 64236de into canonical:main Nov 8, 2024
12 checks passed
@Chris-Peterson444 Chris-Peterson444 deleted the crypt-alternative-for-dryrun branch November 8, 2024 01:08
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.

3 participants