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.

Sign up for GitHub

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

Jump to bottom

std/crypto: add argon2 kdf #9756

Merged
merged 1 commit into from Nov 15, 2021
Merged

std/crypto: add argon2 kdf #9756

merged 1 commit into from Nov 15, 2021

Conversation

x13a
Copy link
Contributor

@x13a x13a commented Sep 13, 2021

This PR adds Argon2 key derivation function. Widely used for password hashing.

/cc @jedisct1

jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
@x13a

This comment has been minimized.

Sign in to view
@x13a
Copy link
Contributor Author

x13a commented Sep 19, 2021

If anybody has any thoughts about this error, plz share..

@matu3ba
Copy link
Contributor

matu3ba commented Sep 26, 2021

@x13a Some partially educated guesses:

  1. Thread-local storage could be somehow broken on powerpc. Probably tls_image gets initialized with wrong values somewhere, since thats where the overflow happens.
  2. You stuff too much data into TLS.

Are you familiar with TLS (on powerpc) to debug this?

@x13a

This comment has been minimized.

Sign in to view
@x13a
Copy link
Contributor Author

x13a commented Oct 22, 2021

I may try next week

Sorry, but out of time for now :(

@LemonBoy LemonBoy mentioned this pull request Oct 23, 2021
Merged
@x13a
Copy link
Contributor Author

x13a commented Oct 29, 2021

Thank you very much, @LemonBoy )

@x13a
Copy link
Contributor Author

x13a commented Oct 29, 2021

Hello @jedisct1 ,

All tests passed. What do you think, is it ready for merging?

@jedisct1
Copy link
Contributor

Looking good!

I'll take a closer look over the weekend.

@x13a
Copy link
Contributor Author

x13a commented Oct 30, 2021

Thanks)

Mouvedia
Mouvedia reviewed Nov 1, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
Mouvedia
Mouvedia reviewed Nov 3, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
Mouvedia
Mouvedia reviewed Nov 3, 2021
View reviewed changes
lib/std/crypto/argon2.zig Outdated Show resolved Hide resolved
@x13a

This comment has been minimized.

Sign in to view
@x13a
Copy link
Contributor Author

x13a commented Nov 14, 2021

@jedisct1 , missing you 🙃

@jedisct1
Copy link
Contributor

jedisct1 commented Nov 14, 2021 •
edited

With the mem_limit issue fixed I think this is finally ready to be merged :)

Without vectorization, it is very slow compared to optimized implementations, but that can be addressed later.

Awesome work, thank you for this!

@jedisct1
Copy link
Contributor

Just waiting for CI to be happy :)

@jedisct1 jedisct1 merged commit c61fbe7 into ziglang:master Nov 15, 2021
@x13a x13a deleted the argon2 branch November 15, 2021 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedisct1 jedisct1 jedisct1 left review comments

@Mouvedia Mouvedia Mouvedia left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@x13a @matu3ba @jedisct1 @Mouvedia