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 pbkdf2 by Argon2 in registrar #81

Closed
wants to merge 4 commits into from

Conversation

thespooler
Copy link
Contributor

@thespooler thespooler commented Apr 9, 2020

This change replaces the current PBKDF2 password by Argon2 from the argonautica crate.

The tests looks good, so I did not find anything to add.
The only thing that I recon might be a point of concern is that store and check handles [u8], while Hasher operates on String, so I handle the utf8 conversion.
I'm not quite sure that no security implications are hidden in there.

@thespooler
Copy link
Contributor Author

Argonautica seems to require clang, wich is missing from CI.
I don't know how to fix that tho.

oxide-auth/Cargo.toml Outdated Show resolved Hide resolved
oxide-auth/src/primitives/registrar.rs Outdated Show resolved Hide resolved
oxide-auth/src/primitives/registrar.rs Outdated Show resolved Hide resolved
oxide-auth/src/primitives/registrar.rs Outdated Show resolved Hide resolved
oxide-auth/src/primitives/registrar.rs Outdated Show resolved Hide resolved
@HeroicKatora
Copy link
Owner

HeroicKatora commented Apr 10, 2020

Argonautica seems to require clang, wich is missing from CI.
I don't know how to fix that tho.

It should be possible to apt install the necessary packages in a script task. I'm not quite sure why clang in particular is required.

@thespooler thespooler force-pushed the argon2 branch 2 times, most recently from 0d91110 to 8cb4861 Compare April 10, 2020 14:00
@thespooler
Copy link
Contributor Author

It seems to work with the reference implementation of phc-winner-argon2

@thespooler thespooler force-pushed the argon2 branch 2 times, most recently from eb3cf1f to 1e01b43 Compare April 10, 2020 14:16
@thespooler
Copy link
Contributor Author

Ok, I think I got this. Except the two tasks that were Signaled to exit!.
I guess some kind of resource limiting on the CI host.

@thespooler
Copy link
Contributor Author

This PR is obsolete with #85 merged and #86.

@thespooler thespooler closed this Apr 13, 2020
@thespooler thespooler deleted the argon2 branch April 13, 2020 16:06
@thespooler thespooler restored the argon2 branch April 27, 2020 21:45
@thespooler thespooler deleted the argon2 branch April 27, 2020 21:46
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.

None yet

2 participants