Skip to content

std/crypto: add argon2 kdf#9756

Merged
jedisct1 merged 1 commit into
ziglang:masterfrom
x13a:argon2
Nov 15, 2021
Merged

std/crypto: add argon2 kdf#9756
jedisct1 merged 1 commit into
ziglang:masterfrom
x13a:argon2

Conversation

@x13a
Copy link
Copy Markdown
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
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
jedisct1
jedisct1 reviewed Sep 13, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
@x13a

This comment has been minimized.

Sign in to view
@x13a
Copy link
Copy Markdown
Contributor Author

x13a commented Sep 19, 2021

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

@matu3ba
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

x13a commented Oct 29, 2021

Thank you very much, @LemonBoy )

@x13a
Copy link
Copy Markdown
Contributor Author

x13a commented Oct 29, 2021

Hello @jedisct1 ,

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

@jedisct1
Copy link
Copy Markdown
Contributor

jedisct1 commented Oct 29, 2021

Looking good!

I'll take a closer look over the weekend.

@x13a
Copy link
Copy Markdown
Contributor Author

x13a commented Oct 30, 2021

Thanks)

Mouvedia
Mouvedia reviewed Nov 1, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
Mouvedia
Mouvedia reviewed Nov 3, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
Mouvedia
Mouvedia reviewed Nov 3, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig Outdated
@x13a

This comment has been minimized.

Sign in to view
@x13a
Copy link
Copy Markdown
Contributor Author

x13a commented Nov 14, 2021

@jedisct1 , missing you 🙃

jedisct1
jedisct1 reviewed Nov 14, 2021
View reviewed changes
Comment thread lib/std/crypto/argon2.zig
@jedisct1
Copy link
Copy Markdown
Contributor

jedisct1 commented Nov 14, 2021 •
edited
Loading

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
Copy Markdown
Contributor

jedisct1 commented Nov 14, 2021

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

+1 more reviewer

@Mouvedia Mouvedia Mouvedia left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@x13a @matu3ba @jedisct1 @Mouvedia