On 2024/8/14 1:37, Mae Kasza wrote:
This also fixes a bug where the set-passphrase
command failed to derive the key for disks initially
formatted with --encrypted and --no_passphrase, due to
bch_sb_crypt_init not configuring the KDF params if
the passphrase wasn't specified during formatting.

Signed-off-by: Mae Kasza <g...@badat.dev>
---
  c_src/cmd_key.c | 26 ++++++++++----------------
  c_src/crypto.c  | 43 ++++++++++++++++++++++++++++++++-----------
  c_src/crypto.h  |  3 +++
  3 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/c_src/cmd_key.c b/c_src/cmd_key.c
index adb0ac8d..ac8a94a8 100644
--- a/c_src/cmd_key.c
+++ b/c_src/cmd_key.c
@@ -104,24 +104,19 @@ int cmd_set_passphrase(int argc, char *argv[])
        if (IS_ERR(c))
                die("Error opening %s: %s", argv[1], bch2_err_str(PTR_ERR(c)));
- struct bch_sb_field_crypt *crypt = bch2_sb_field_get(c->disk_sb.sb, crypt);
+       struct bch_sb *sb = c->disk_sb.sb;
+       struct bch_sb_field_crypt *crypt = bch2_sb_field_get(sb, crypt);
        if (!crypt)
                die("Filesystem does not have encryption enabled");
- struct bch_encrypted_key new_key;
-       new_key.magic = BCH_KEY_MAGIC;
-
-       int ret = bch2_decrypt_sb_key(c, crypt, &new_key.key);
+       struct bch_key key;
+       int ret = bch2_decrypt_sb_key(c, crypt, &key);
        if (ret)
                die("Error getting current key");
char *new_passphrase = read_passphrase_twice("Enter new passphrase: ");
-       struct bch_key passphrase_key = derive_passphrase(crypt, 
new_passphrase);
- if (bch2_chacha_encrypt_key(&passphrase_key, __bch2_sb_key_nonce(c->disk_sb.sb),
-                                   &new_key, sizeof(new_key)))
-               die("error encrypting key");
-       crypt->key = new_key;
+       bch_crypt_update_passphrase(sb, crypt, &key, new_passphrase);
bch2_revoke_key(c->disk_sb.sb);
        bch2_write_super(c);
@@ -142,18 +137,17 @@ int cmd_remove_passphrase(int argc, char *argv[])
        if (IS_ERR(c))
                die("Error opening %s: %s", argv[1], bch2_err_str(PTR_ERR(c)));
- struct bch_sb_field_crypt *crypt = bch2_sb_field_get(c->disk_sb.sb, crypt);
+       struct bch_sb *sb = c->disk_sb.sb;
+       struct bch_sb_field_crypt *crypt = bch2_sb_field_get(sb, crypt);
        if (!crypt)
                die("Filesystem does not have encryption enabled");
- struct bch_encrypted_key new_key;
-       new_key.magic = BCH_KEY_MAGIC;
-
-       int ret = bch2_decrypt_sb_key(c, crypt, &new_key.key);
+       struct bch_key key;
+       int ret = bch2_decrypt_sb_key(c, crypt, &key);
        if (ret)
                die("Error getting current key");
- crypt->key = new_key;
+       bch_crypt_update_passphrase(sb, crypt, &key, NULL);
bch2_write_super(c);
        bch2_fs_stop(c);
diff --git a/c_src/crypto.c b/c_src/crypto.c
index 32671bd8..301dbebe 100644
--- a/c_src/crypto.c
+++ b/c_src/crypto.c
@@ -176,26 +176,47 @@ void bch_sb_crypt_init(struct bch_sb *sb,
                       struct bch_sb_field_crypt *crypt,
                       const char *passphrase)
  {
+       struct bch_key key;
+       get_random_bytes(&key, sizeof(key));
+
        crypt->key.magic = BCH_KEY_MAGIC;
-       get_random_bytes(&crypt->key.key, sizeof(crypt->key.key));
+       crypt->key.key = key;
- if (passphrase) {
+       bch_crypt_update_passphrase(sb, crypt, &key, passphrase);
+}
+void bch_crypt_update_passphrase(
+                       struct bch_sb *sb,
+                       struct bch_sb_field_crypt *crypt,
+                       struct bch_key *key,
+                       const char *new_passphrase)
+{
+
+       struct bch_encrypted_key new_key;
+       new_key.magic = BCH_KEY_MAGIC;
+       new_key.key = *key;
+
+       if(!new_passphrase) {
+               crypt->key = new_key;
May be the helper like this is needed. But we could assign the crypt->key outside, which seems more reasonable according to the helper name. Just in my humble opinion.

Thanks,
Hongbo

+               return;
+       }
+
+       // If crypt already has an encrypted key reuse it's encryption params
+       if (!bch2_key_is_encrypted(&crypt->key)) {
                SET_BCH_CRYPT_KDF_TYPE(crypt, BCH_KDF_SCRYPT);
                SET_BCH_KDF_SCRYPT_N(crypt, ilog2(16384));
                SET_BCH_KDF_SCRYPT_R(crypt, ilog2(8));
                SET_BCH_KDF_SCRYPT_P(crypt, ilog2(16));
+       }
- struct bch_key passphrase_key = derive_passphrase(crypt, passphrase);
-
-               assert(!bch2_key_is_encrypted(&crypt->key));
+       struct bch_key passphrase_key = derive_passphrase(crypt, 
new_passphrase);
- if (bch2_chacha_encrypt_key(&passphrase_key, __bch2_sb_key_nonce(sb),
-                                          &crypt->key, sizeof(crypt->key)))
-                       die("error encrypting key");
+       if (bch2_chacha_encrypt_key(&passphrase_key, __bch2_sb_key_nonce(sb),
+                                   &new_key, sizeof(new_key)))
+               die("error encrypting key");
- assert(bch2_key_is_encrypted(&crypt->key));
+       memzero_explicit(&passphrase_key, sizeof(passphrase_key));
- memzero_explicit(&passphrase_key, sizeof(passphrase_key));
-       }
+       crypt->key = new_key;
+       assert(bch2_key_is_encrypted(&crypt->key));
  }
diff --git a/c_src/crypto.h b/c_src/crypto.h
index baea6d86..cd3baac8 100644
--- a/c_src/crypto.h
+++ b/c_src/crypto.h
@@ -19,4 +19,7 @@ void bch2_add_key(struct bch_sb *, const char *, const char 
*, const char *);
  void bch_sb_crypt_init(struct bch_sb *sb, struct bch_sb_field_crypt *,
                       const char *);
+void bch_crypt_update_passphrase(struct bch_sb *sb, struct bch_sb_field_crypt *crypt,
+                       struct bch_key *key, const char *new_passphrase);
+
  #endif /* _CRYPTO_H */

Reply via email to