Re: [PATCH v3] security/keys: rewrite big_key crypto to use library interface
On Tue, May 12, 2020 at 4:03 PM David Howells wrote: > > Jason A. Donenfeld wrote: > > > So long as that ->update function: > > 1. Deletes the old on-disk data. > > 2. Deletes the old key from the inode. > > 3. Generates a new key using get_random_bytes. > > 4. Stores that new key in the inode. > > 5. Encrypts the updated data afresh with the new key. > > 6. Puts the updated data onto disk, > > > > then this is fine with me, and feel free to have my Acked-by if you > > want. But if it doesn't do that -- i.e. if it tries to reuse the old > > key or similar -- then this isn't fine. But it sounds like from what > > you've described that things are actually fine, in which case, I guess > > it makes sense to apply your patch ontop of mine and commit these. > > Yep. It calls big_key_destroy(), which clears away the old stuff just as when > a key is being destroyed, then generic_key_instantiate() just as when a key is > being set up. > > The key ID and the key metadata (ownership, perms, expiry) are maintained, but > the payload is just completely replaced. Okay, in that case, take my: Acked-by: Jason A. Donenfeld And then perhaps you can take both my patch and your addendum into keys-next. Jason
Re: [PATCH v3] security/keys: rewrite big_key crypto to use library interface
Jason A. Donenfeld wrote: > So long as that ->update function: > 1. Deletes the old on-disk data. > 2. Deletes the old key from the inode. > 3. Generates a new key using get_random_bytes. > 4. Stores that new key in the inode. > 5. Encrypts the updated data afresh with the new key. > 6. Puts the updated data onto disk, > > then this is fine with me, and feel free to have my Acked-by if you > want. But if it doesn't do that -- i.e. if it tries to reuse the old > key or similar -- then this isn't fine. But it sounds like from what > you've described that things are actually fine, in which case, I guess > it makes sense to apply your patch ontop of mine and commit these. Yep. It calls big_key_destroy(), which clears away the old stuff just as when a key is being destroyed, then generic_key_instantiate() just as when a key is being set up. The key ID and the key metadata (ownership, perms, expiry) are maintained, but the payload is just completely replaced. David
Re: [PATCH v3] security/keys: rewrite big_key crypto to use library interface
Hi David, So long as that ->update function: 1. Deletes the old on-disk data. 2. Deletes the old key from the inode. 3. Generates a new key using get_random_bytes. 4. Stores that new key in the inode. 5. Encrypts the updated data afresh with the new key. 6. Puts the updated data onto disk, then this is fine with me, and feel free to have my Acked-by if you want. But if it doesn't do that -- i.e. if it tries to reuse the old key or similar -- then this isn't fine. But it sounds like from what you've described that things are actually fine, in which case, I guess it makes sense to apply your patch ontop of mine and commit these. Jason
Re: [PATCH v3] security/keys: rewrite big_key crypto to use library interface
Jason A. Donenfeld wrote:
> - /* no ->update(); don't add it without changing big_key_crypt() nonce */
> + /* no ->update(); don't add it without changing chacha20poly1305's nonce
Note that ->update() doesn't have to modify the contents of the key, but can
just rather replace them entirely. See attached. The actual work is done in
big_key_preparse(); all big_key_update() has to do is apply it and dispose of
the old payload.
David
---
commit 724e76c185d517f35ead4f72f9958850c9218f4d
Author: David Howells
Date: Tue May 12 14:03:53 2020 +0100
keys: Implement update for the big_key type
Implement the ->update op for the big_key type.
Signed-off-by: David Howells
diff --git a/include/keys/big_key-type.h b/include/keys/big_key-type.h
index 3fee04f81439..988d90d77f53 100644
--- a/include/keys/big_key-type.h
+++ b/include/keys/big_key-type.h
@@ -18,5 +18,6 @@ extern void big_key_revoke(struct key *key);
extern void big_key_destroy(struct key *key);
extern void big_key_describe(const struct key *big_key, struct seq_file *m);
extern long big_key_read(const struct key *key, char *buffer, size_t buflen);
+extern int big_key_update(struct key *key, struct key_preparsed_payload *prep);
#endif /* _KEYS_BIG_KEY_TYPE_H */
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index d43f3daab2b8..dd708e8f13c0 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -47,7 +47,7 @@ struct key_type key_type_big_key = {
.destroy= big_key_destroy,
.describe = big_key_describe,
.read = big_key_read,
- /* no ->update(); don't add it without changing chacha20poly1305's
nonce */
+ .update = big_key_update,
};
/*
@@ -191,6 +191,23 @@ void big_key_destroy(struct key *key)
key->payload.data[big_key_data] = NULL;
}
+/*
+ * Update a big key
+ */
+int big_key_update(struct key *key, struct key_preparsed_payload *prep)
+{
+ int ret;
+
+ ret = key_payload_reserve(key, prep->datalen);
+ if (ret < 0)
+ return ret;
+
+ if (key_is_positive(key))
+ big_key_destroy(key);
+
+ return generic_key_instantiate(key, prep);
+}
+
/*
* describe the big_key key
*/
