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

Avoid module-level imports of oscrypt #36

Closed
wants to merge 1 commit into from

Conversation

AdrianVollmer
Copy link

@AdrianVollmer AdrianVollmer commented Dec 5, 2023

Let me take this opportunity to thank you for important work on many of your projects, some of which have become a corner stone of the OST community. Your efforts are very appreciated.

Today I'd like to discuss the oscrypto dependency. As you are aware, there are issues in some of your projects which depend on minikerberos (skelsec/pypykatz#136, skelsec/msldap#37) due to a bad regex if openssl has a double digit patch number. Currently, openssl-3.0.10 is the current version on Kali and Debian unstable and probably more. This is discussed here: wbond/oscrypto#78

I'm not sure why exactly the oscrypto dependency is needed, as cryptography is already a dependency due to the asysocks dependency. I think it could be replaced by cryptography, but unfortunately I lack the skills, time and motivation to do this.

To alleviate the issue, I propose to avoid the module-level imports of oscrypto and move them to the functions where they are needed by merging this PR. This would allow us to use projects like LdapRelayScan which need minikerberos, but not the pkinit module. Note that minikerberos has over 900 dependents (including forks, not sure how accurate this information is though), even though only 26 files (also including forks and identical files) actually import minikerberos.pkinit.

Still, if you manage to remove the oscrypto dependency, I'd count that as a win. What do you think?

The original commit message follows. Please note that I could not yet test this change.


Many dependents of minikerberos don't need PKINIT, so it makes sense to import oscrypto only when needed. Especially because oscrypto<=1.3.0 does not work when openssl>=3.0.10.

See: wbond/oscrypto#78

Many dependents of minikerberos don't need `PKINIT`, so it makes sense
to import `oscrypt` only when needed. Especially because `oscrypt<=1.3.0` does
not work when `openssl>=3.0.10`.

See: wbond/oscrypto#78
@skelsec
Copy link
Owner

skelsec commented Dec 8, 2023

Please check the new version of asyauth it should fix this issue.

@AdrianVollmer
Copy link
Author

Fantastic. Thank you!

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.

2 participants