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 id_key, enc_key and enc_hmac_key with a single field #6611

Closed
hexagonrecursion opened this issue Apr 17, 2022 · 3 comments
Closed

Replace id_key, enc_key and enc_hmac_key with a single field #6611

hexagonrecursion opened this issue Apr 17, 2022 · 3 comments
Assignees
Milestone

Comments

@hexagonrecursion
Copy link
Contributor

hexagonrecursion commented Apr 17, 2022

If we choose to break with the past #6602, I propose we simplify our key format. Our keys currently store 800 secret bits split into 4 fields: id_key (256 bits), enc_key (256 bits), enc_hmac_key (256 bits) and chunk_seed (32 bits). The fields were once created each for a specific purpose, but some were later repurposed:

  1. We needed a new key to authenticate the manifest. We currently derive it via a key derivation function from a concatenation of id_key, enc_key and enc_hmac_key.
  2. In new AEAD crypto with session keys #6463 we needed a session key. We currently derive it via a key derivation function from a concatenation of enc_key and enc_hmac_key.

I propose we future-proof our key format by having a single secret field - a seed. We can then derive any number of keys we need from it via a cryptographically secure pseudorandom number generator.

P.S. I myself do not currently have an opinion on whether breaking with the past (#6602) is a good idea or not.
P.P.S. We might want to future-proof the format by generating a bigger seed than we need and truncating it to the length a given PRNG wants. We can then upgrade to a PRNG with a bigger seed without changing the format

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Jun 1, 2022

You're right, having a big random seed and derive all we need from that would be more elegant.

But, for borg transfer to be able to efficiently transfer compressed chunks from an old repo to a borg2 repo (or from one new borg2 repo to another borg2 repo), we need to use the same chunker seed and id_key, so the current chunks will dedup with the future chunks from same / similar files - thus these must be kept separate and not be derived from crypt_key.

The enc_key and enc_hmac_key are not needed separately any more for the AEAD ciphers, so these could be concatenated and written to a new key field. borg2 still needs to be able to read the old keys for reading old repos with old encryption though.

@ThomasWaldmann ThomasWaldmann added this to the 2.0.0b1 milestone Jul 4, 2022
@ThomasWaldmann ThomasWaldmann self-assigned this Aug 1, 2022
@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Aug 1, 2022

idea:

  • have a new attr crypt_key that contains what used to be in enc_key + enc_hmac_key
  • instead of enc_key we can just use crypt_key[:32] (AES key always has 256 bits)
  • instead of enc_hmac_key we can just use crypt_key[32:] (mac key can be 256 bits for hmac-sha256 or 1024 bits for blake2b)
  • load: deal with old and new keys, but feed into crypt_key attr
  • save: only save new keys

@ThomasWaldmann
Copy link
Member

Old borg can not read/process the new keys that have crypt_key instead of enc_key and enc_hmac_key, thus increasing the key version to 2.

ThomasWaldmann added a commit that referenced this issue Aug 3, 2022
Key: crypt_key instead of enc_key + enc_hmac_key, fixes #6611
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

No branches or pull requests

2 participants