search

inejge / pwhash

A collection of password hashing routines in pure Rust
MIT License
61 stars 11 forks source link

Update dependencies, update edition #16

Closed sundhaug92 closed 2 years ago

sundhaug92 commented 2 years ago

This doesn't update all dependencies, and some are only partially updated. This is the most I could update without needing code-changes (and thus the most I felt comfortable doing without risking potential API issues). I think a future idea would be to break unix_crypt into a separate file, and then make it and the various hashing algorithms features (so you could for example say "I only need to verify md5crypt hashes, so don't include stuff like blowfish and rand").

inejge commented 2 years ago

Thanks for the PR! I'll be mostly offline for at least another week, so I won't be able to check and merge it, sorry. In the meantime, please remove the VS Code config from the PR, I try not to mix local preferences with the library code.

sundhaug92 commented 2 years ago

Ok, should I list it in .gitignore? It's just to make "F5 to run tests" work.

inejge commented 2 years ago

I did the rest of the updates up to the latest crypto crate versions, including adaptations due to API changes. (Which were quite modest, I suppose you could've done them too. As long as the tests pass and the crate's public API remains the same, implementation details don't matter much.) I rearranged and squashed the commits at the same time.

I don't think I'm going to make unix_crypt()'s algorithm collection configurable. Its raison d'être is to be a one-stop shop for anything one may encounter on a Unix system.

sundhaug92 commented 2 years ago

Ok, ty, wasn't familiar enough with rust or the library to update the crates and got a bit distracted with other stuff to fix the vscode thing. The reason I'm interested in making unix_crypt()'s algorithm-collection configurable is that I had a use-case that only needs a small part of the library. I have a working implementation of that, I think I'll be able to merge that with your updates, in a way that should both be backwards compatibility and add in flexibility.

inejge commented 2 years ago

It's no doubt possible to place each algorithm behind its own feature flag. The reason I want to avoid it is that every flag complicates building, testing and documentation, which is acceptable if there's a large delta in the number of dependencies and/or library size, but I don't see it here. E.g., cargo bloat reports puny sizes for the crypto algorithms (< 15 KiB for the largest ones). I don't think it's worth the bother.

If you absolutely need the smallest possible code size, I believe that your approach with the private fork is the correct one in this case.