Re: [PATCH] cmd_set_passphrase: initialize KDF parameters

Kent Overstreet
2024-08-12T01:41:07Z

On Sun, Aug 11, 2024 at 11:40:38PM GMT, Mae Kasza wrote:
> From: mae <git@badat.dev>
>
> The set-passphrase command failed to derive the key for disks initially
> formatted with --encrypted and --no_passphrase.
>
> This happened because bch_sb_crypt_init only configures the KDF params
> if a passphrase is specified.
> This commit makes the command initialize the KDF with the same parameters
> as bch_sb_crypt_init if the key wasn't encrypted before.
>
> Signed-off-by: Mae Kasza <git@badat.dev>

This looks good to me, but I'm in the middle of a 20 hour drive and not
feeling super confident to review crypto stuff tonight, so maybe someone
else can go over it too before I pull it in..

> ---
> c_src/cmd_key.c | 9 +++++++--
> c_src/crypto.c | 13 ++++++++-----
> c_src/crypto.h | 1 +
> 3 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/c_src/cmd_key.c b/c_src/cmd_key.c
> index adb0ac8d..2da83758 100644
> --- a/c_src/cmd_key.c
> +++ b/c_src/cmd_key.c
> @@ -104,7 +104,8 @@ 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");
>
> @@ -116,9 +117,13 @@ int cmd_set_passphrase(int argc, char *argv[])
> die("Error getting current key");
>
> char *new_passphrase = read_passphrase_twice("Enter new passphrase: ");
> + if (!bch2_key_is_encrypted(&crypt->key)) {
> + bch_crypt_default_kdf_init(crypt);
> + }
> +
> 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),
> + if (bch2_chacha_encrypt_key(&passphrase_key, __bch2_sb_key_nonce(sb),
> &new_key, sizeof(new_key)))
> die("error encrypting key");
> crypt->key = new_key;
> diff --git a/c_src/crypto.c b/c_src/crypto.c
> index 32671bd8..30ad92d4 100644
> --- a/c_src/crypto.c
> +++ b/c_src/crypto.c
> @@ -180,11 +180,7 @@ void bch_sb_crypt_init(struct bch_sb *sb,
> get_random_bytes(&crypt->key.key, sizeof(crypt->key.key));
>
> if (passphrase) {
> -
> - 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));
> + bch_crypt_default_kdf_init(crypt);
>
> struct bch_key passphrase_key = derive_passphrase(crypt, passphrase);
>
> @@ -199,3 +195,10 @@ void bch_sb_crypt_init(struct bch_sb *sb,
> memzero_explicit(&passphrase_key, sizeof(passphrase_key));
> }
> }
> +
> +void bch_crypt_default_kdf_init(struct bch_sb_field_crypt *crypt) {
> + 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));
> +}
> diff --git a/c_src/crypto.h b/c_src/crypto.h
> index baea6d86..846a8931 100644
> --- a/c_src/crypto.h
> +++ b/c_src/crypto.h
> @@ -18,5 +18,6 @@ void bch2_passphrase_check(struct bch_sb *, const char *,
> 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_default_kdf_init(struct bch_sb_field_crypt *);
>
> #endif /* _CRYPTO_H */
> --
> 2.45.2
>