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

crypto: use a one-step kdf for session keys, fixes #7953 #7955

Conversation

ThomasWaldmann
Copy link
Member

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (6a511fd) 83.65% compared to head (74c34ba) 83.65%.
Report is 2 commits behind head on master.

Files Patch % Lines
src/borg/crypto/key.py 83.33% 0 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7955      +/-   ##
==========================================
- Coverage   83.65%   83.65%   -0.01%     
==========================================
  Files          66       66              
  Lines       11860    11861       +1     
  Branches     2149     2150       +1     
==========================================
  Hits         9922     9922              
- Misses       1363     1364       +1     
  Partials      575      575              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ThomasWaldmann
Copy link
Member Author

ThomasWaldmann commented Nov 30, 2023

Nice:

  • faster key derivation (relevant for existing content)
  • get slightly faster startup time (less self-tests)
  • looks like we can get rid of quite some crypto code
  • no "own crypto implementations" (was: HKDF-HMAC-SHA512) any more

@marti4d
Copy link

marti4d commented Dec 1, 2023

Just a thought, but I know I used the word flavour (spelled a certain way 🇨🇦 🇨🇦 🇨🇦) when I was talking about the string detailing what encryption suite you are using, but that's just kind-of a cheeky word I used for it. I think most organizations like FIPS would use the more standard term domain for it, since "domain separation" is the reason it's there.

@ThomasWaldmann ThomasWaldmann force-pushed the improve-session-key-gen-master branch 2 times, most recently from e1b975e to ceb6456 Compare December 2, 2023 19:21
@ThomasWaldmann
Copy link
Member Author

@enkore can you review, please? You added the hkdf code 7y ago for the TAM stuff.

In master branch, i removed all the TAM stuff because we have typed repo objects now, so borg actually knows what's metadata and what's user data and verifies that it gets the type of object it requested from the repo.

So the session key computation was the last user of that hkdf and after discussion in #7953 a way simpler "one-step kdf" was found.

@ThomasWaldmann ThomasWaldmann changed the title crypto: fix and simplify _get_session_key, fixes #7953 crypto: use a one-step kdf for session keys, fixes #7953 Dec 2, 2023
also:
- fixes, simplifies, speeds up _get_session_key
- convert sessionid memoryview to bytes before calling _get_cipher,
to avoid TypeError in (crypt_key + sessionid + domain) operation.
- add docstring and comments
@ThomasWaldmann ThomasWaldmann force-pushed the improve-session-key-gen-master branch from ceb6456 to 74c34ba Compare December 2, 2023 19:35
@ThomasWaldmann ThomasWaldmann merged commit 812eb35 into borgbackup:master Jan 4, 2024
13 checks passed
@ThomasWaldmann ThomasWaldmann deleted the improve-session-key-gen-master branch January 4, 2024 17:42
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.

3 participants