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

Argon2 the first part: Implement key derivation (was: part 0) #6468

Merged
merged 10 commits into from
Mar 21, 2022

Conversation

hexagonrecursion
Copy link
Contributor

This is the first part of #747. I have added a method to helpers.passphrase.Passphrase to derive keys via argon2.

I have chosen to add a new method instead of changing helpers.passphrase.Passphrase.kdf because:

  1. argon2 has more tunable parameters
  2. decrypt_key_file needs to be able to select pbkdf2 or argon2 based on key version
  3. I have heard that it is usually more pythonic to define two functions than to define one that does two different things based on a flag

PS. After reading the code I have reversed my original decision and started implementing this bottom-up. I'll do decrypt_key_file next
PPS. Sorry for the delay. I am back in your galaxy and ready to work on this today.

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2022

Codecov Report

Merging #6468 (1df8a0c) into master (cc3b5c0) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #6468      +/-   ##
==========================================
- Coverage   82.70%   82.68%   -0.02%     
==========================================
  Files          39       39              
  Lines       10519    10525       +6     
  Branches     2076     2076              
==========================================
+ Hits         8700     8703       +3     
- Misses       1311     1314       +3     
  Partials      508      508              
Impacted Files Coverage Δ
src/borg/helpers/passphrase.py 75.70% <50.00%> (-1.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfd7ea8...1df8a0c. Read the comment docs.

requirements.d/development.txt Outdated Show resolved Hide resolved
src/borg/helpers/passphrase.py Outdated Show resolved Hide resolved
src/borg/helpers/passphrase.py Outdated Show resolved Hide resolved
src/borg/testsuite/helpers.py Outdated Show resolved Hide resolved
src/borg/helpers/passphrase.py Show resolved Hide resolved
src/borg/helpers/passphrase.py Outdated Show resolved Hide resolved
@ThomasWaldmann ThomasWaldmann added this to the helium milestone Mar 20, 2022
src/borg/helpers/passphrase.py Outdated Show resolved Hide resolved
src/borg/testsuite/helpers.py Outdated Show resolved Hide resolved
src/borg/helpers/passphrase.py Outdated Show resolved Hide resolved
@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Mar 21, 2022

https://www.ietf.org/archive/id/draft-irtf-cfrg-argon2-03.txt

Select the type y.
If you do not know the difference between them or you consider side-channel attacks
as viable threat, choose Argon2id.

@hexagonrecursion
Copy link
Contributor Author

ci.appveyor.com:

Uploading artifacts...
Error uploading artifact to the storage: The request was aborted: Could not create SSL/TLS secure channel.

@ThomasWaldmann
Copy link
Member

seems like they have issues at appveyor (but we can just ignore that for now).

@hexagonrecursion hexagonrecursion changed the title Argon2 part 0: Implement key derivation Argon2 ~~part 0~~: Implement key derivation Mar 21, 2022
@hexagonrecursion hexagonrecursion changed the title Argon2 ~~part 0~~: Implement key derivation Argon2 the first part: Implement key derivation (was: part 0) Mar 21, 2022
Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ThomasWaldmann
Copy link
Member

OK, ready for merging?

@ThomasWaldmann
Copy link
Member

I'ld squash the commits together...

@hexagonrecursion
Copy link
Contributor Author

OK, ready for merging?

ready

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

3 participants