client: Do not expose ceph_fscrypt_key_identifier in api

The libcephfs API call add_fscrypt_key exposes an internal fscrypt
data structure. This is because a hash keyid (of the master key) is used
for calls such as remove_fscrypt_key. Instead of using this structure,
use a char array to obtain keyid.

Fixes: https://tracker.ceph.com/issues/63293
Signed-off-by: Christopher Hoffman <choffman@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 28aa5565951cfd867fee924125b8917fb4c8e340..de3626862a4818af1f45bc0bbbba203ce1344e6f 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -7325,11 +7325,11 @@ void Client::abort_conn()
 
 int Client::fscrypt_dummy_encryption() {
     // get add key
-    char key[20];
+    char key[FSCRYPT_KEY_IDENTIFIER_SIZE];
     memset(key, 0, sizeof(key));
-    ceph_fscrypt_key_identifier kid;
 
-    int r = add_fscrypt_key(key, FSCRYPT_KEY_IDENTIFIER_SIZE, &kid);
+    char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+    int r = add_fscrypt_key(key, sizeof(key), keyid);
     if (r < 0) {
       goto err;
     }
@@ -7342,7 +7342,7 @@ int Client::fscrypt_dummy_encryption() {
     policy.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
     policy.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
     policy.flags = FSCRYPT_POLICY_FLAGS_PAD_32;
-    memcpy(policy.master_key_identifier, kid.raw, FSCRYPT_KEY_IDENTIFIER_SIZE);
+    memcpy(policy.master_key_identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
     r = ll_set_fscrypt_policy_v2(root.get(), policy);
     if (r < 0) {
       goto err;
@@ -7354,7 +7354,7 @@ int Client::fscrypt_dummy_encryption() {
     fscrypt_key_specifier key_spec;
     key_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
     key_spec.__reserved = 0;
-    memcpy(key_spec.u.identifier, kid.raw, 16);
+    memcpy(key_spec.u.identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
     arg.removal_status_flags = 0;
     arg.key_spec = key_spec;
     r = remove_fscrypt_key(&arg);
@@ -18321,7 +18321,7 @@ void Client::set_uuid(const std::string& uuid)
 }
 
 int Client::add_fscrypt_key(const char *key_data, int key_len,
-                            ceph_fscrypt_key_identifier *kid, int user)
+                            char* keyid, int user)
 {
   auto& key_store = fscrypt->get_key_store();
 
@@ -18334,8 +18334,8 @@ int Client::add_fscrypt_key(const char *key_data, int key_len,
   }
 
   auto& k = kh->get_key();
-  if (kid) {
-    *kid = k->get_identifier();
+  if (keyid) {
+    memcpy(keyid, &k->get_identifier().raw, FSCRYPT_KEY_IDENTIFIER_SIZE);
   }
 
   return 0;

diff --git a/src/client/Client.h b/src/client/Client.h
index 28be5bb36842ca82ba23339b41293e8e718b93fc..14e61830b77841cc19f2a1da05fc966c84ce077b 100644 (file)
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -382,7 +382,7 @@ public:
   }
 
   /* fscrypt */
-  int add_fscrypt_key(const char *key_data, int key_len, ceph_fscrypt_key_identifier *kid, int user = 0);
+  int add_fscrypt_key(const char *key_data, int key_len, char* keyid, int user = 0);
   int remove_fscrypt_key(fscrypt_remove_key_arg* kid, int user = 0);
   int get_fscrypt_key_status(fscrypt_get_key_status_arg* arg);
 

diff --git a/src/client/FSCrypt.h b/src/client/FSCrypt.h
index 038775eb7a5fb1f4e50955d7285a6a34ec511a22..30b5a0300a3d77afe7499161f2ac23174f17db37 100644 (file)
--- a/src/client/FSCrypt.h
+++ b/src/client/FSCrypt.h
@@ -68,8 +68,7 @@ int fscrypt_calc_hkdf(char hkdf_context,
 
 
 struct ceph_fscrypt_key_identifier {
-#define FSCRYPT_KEY_IDENTIFIER_LEN 16
-  char raw[FSCRYPT_KEY_IDENTIFIER_LEN];
+  char raw[FSCRYPT_KEY_IDENTIFIER_SIZE];
 
   int init(const char *k, int klen);
   int init(const struct fscrypt_key_specifier& k);

diff --git a/src/client/fuse_ll.cc b/src/client/fuse_ll.cc
index 2346de122f47017d5ccaa4326e3ed523ff014f2c..1b1e51106d9419b111054b18762538b4ebbaed2d 100644 (file)
--- a/src/client/fuse_ll.cc
+++ b/src/client/fuse_ll.cc
@@ -1015,15 +1015,15 @@ static void fuse_ll_ioctl(fuse_req_t req, fuse_ino_t ino,
         break;
       }
 
-      ceph_fscrypt_key_identifier kid;
-      int r = cfuse->client->add_fscrypt_key((const char *)arg->raw, arg->raw_size, &kid, ctx->uid);
+      char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+      int r = cfuse->client->add_fscrypt_key((const char *)arg->raw, arg->raw_size, keyid, ctx->uid);
       if (r < 0) {
         generic_dout(0) << __FILE__ << ":" << __LINE__ << ": failed to create a new key: r=" << r << dendl;
         fuse_reply_err(req, -r);
         break;
       }
 
-      memcpy(&arg->key_spec.u.identifier, &kid.raw, 16);
+      memcpy(&arg->key_spec.u.identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
       fuse_reply_ioctl(req, 0, arg, sizeof(*arg));
       break;
     }

diff --git a/src/include/cephfs/libcephfs.h b/src/include/cephfs/libcephfs.h
index 106785fe611bbc0e5e45e7a7ddb604d44c66b729..12c202ae12eabb5f3596bfd59f694e96e19b5bd8 100644 (file)
--- a/src/include/cephfs/libcephfs.h
+++ b/src/include/cephfs/libcephfs.h
@@ -2008,14 +2008,13 @@ int ceph_debug_get_file_caps(struct ceph_mount_info *cmount, const char *path);
  * @param cmount the ceph mount handle to use.
  * @param key_data key data
  * @param key_len key data length
- * @param kid to hold the returned key identifier
+ * @param out_keyid to hold the hashed key identifier, FSCRYPT_KEY_IDENTIFIER_SIZE bytes in length
  * @param user user id
  * @returns zero on success, other returns a negative error code.
  */
 int ceph_add_fscrypt_key(struct ceph_mount_info *cmount,
                          const char *key_data, int key_len,
-                        struct ceph_fscrypt_key_identifier *kid,
-                        int user);
+                         char* out_keyid, int user);
 
 /**
  * Remove fscrypt encryption key from the in-memory key manager

diff --git a/src/libcephfs.cc b/src/libcephfs.cc
index 95d6c755d19caf970fee4efb9c9f8a4d292bba8b..e80976de25524de99659fbe520ba032d780e7520 100644 (file)
--- a/src/libcephfs.cc
+++ b/src/libcephfs.cc
@@ -2506,18 +2506,18 @@ extern "C" void ceph_finish_reclaim(class ceph_mount_info *cmount)
 
 extern "C" int ceph_add_fscrypt_key(struct ceph_mount_info *cmount,
                                     const char *key_data, int key_len,
-                                   struct ceph_fscrypt_key_identifier *kid,
+                                   char* out_keyid,
                                    int user)
 {
   if (!cmount->is_mounted())
     return -ENOTCONN;
 
-  return cmount->get_client()->add_fscrypt_key(key_data, key_len, kid, user);
+  return cmount->get_client()->add_fscrypt_key(key_data, key_len, out_keyid, user);
 }
 
 extern "C" int ceph_remove_fscrypt_key(struct ceph_mount_info *cmount,
                                        struct fscrypt_remove_key_arg *kid,
-                                      int user)
+                                       int user)
 {
   if (!cmount->is_mounted())
     return -ENOTCONN;

diff --git a/src/test/client/TestClient.h b/src/test/client/TestClient.h
index d9d4344354b65881a574fdc9f9b0c420971657db..4e0fd6c103574ddd40bc6855b2677a455b59c3c2 100644 (file)
--- a/src/test/client/TestClient.h
+++ b/src/test/client/TestClient.h
@@ -168,9 +168,8 @@ public:
 
       close(key_fd);
 
-      struct ceph_fscrypt_key_identifier kid;
-
-      r = add_fscrypt_key(fse->key, sizeof(fse->key), &kid);
+      char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+      r = add_fscrypt_key(fse->key, sizeof(fse->key), keyid);
       if (r < 0) {
         std::clog << __func__ << "(): add_fscrypt_key() r=" << r << std::endl;
         throw std::runtime_error("add_fscrypt_key() returned error");
@@ -181,7 +180,7 @@ public:
       policy.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
       policy.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
       policy.flags = FSCRYPT_POLICY_FLAGS_PAD_32;
-      memcpy(policy.master_key_identifier, kid.raw, FSCRYPT_KEY_IDENTIFIER_SIZE);
+      memcpy(policy.master_key_identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
 
       int fd = open(fse->name.c_str(), O_DIRECTORY, myperm, 0);
       if (fd < 0) {
@@ -216,9 +215,8 @@ public:
         return r;
       }
 
-      struct ceph_fscrypt_key_identifier kid;
-
-      r = add_fscrypt_key(fse->key, sizeof(fse->key), &kid);
+      char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+      r = add_fscrypt_key(fse->key, sizeof(fse->key), keyid);
       if (r < 0) {
         std::clog << __func__ << "() ceph_mount add_fscrypt_key r=" << r << std::endl;
         return r;

diff --git a/src/test/libcephfs/fscrypt.cc b/src/test/libcephfs/fscrypt.cc
index 764131aa8aecdada98ee833498ce4beeaf3b9268..5534dea95f0d70e4f55401dba1c8c28c87d15aa7 100644 (file)
--- a/src/test/libcephfs/fscrypt.cc
+++ b/src/test/libcephfs/fscrypt.cc
@@ -71,9 +71,8 @@ int do_fscrypt_mount(struct ceph_mount_info *cmount, const char *root)
     return r;
   }
 
-  struct ceph_fscrypt_key_identifier kid;
-
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 0);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 0);
   if (r < 0) {
     std::clog << __func__ << "() ceph_mount add_fscrypt_key r=" << r << std::endl;
     return r;
@@ -88,22 +87,22 @@ string get_unique_dir_name()
   return string("ceph_test_libcephfs_fscrypt.") + stringify(mypid) + "." + stringify(rand());
 }
 
-void generate_remove_key_arg(ceph_fscrypt_key_identifier kid, fscrypt_remove_key_arg* arg){
+void generate_remove_key_arg(char *keyid, fscrypt_remove_key_arg* arg){
   fscrypt_key_specifier key_spec;
   key_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
   key_spec.__reserved = 0;
-  memcpy(key_spec.u.identifier, kid.raw, 16);
+  memcpy(key_spec.u.identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
   arg->removal_status_flags = 0;
   arg->key_spec = key_spec;
 }
 
-void populate_policy(struct ceph_fscrypt_key_identifier kid, struct fscrypt_policy_v2* policy) {
+void populate_policy(char *keyid, struct fscrypt_policy_v2* policy) {
   memset(policy, 0, sizeof(*policy));
   policy->version = 2;
   policy->contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
   policy->filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
   policy->flags = FSCRYPT_POLICY_FLAGS_PAD_32;
-  memcpy(policy->master_key_identifier, kid.raw, FSCRYPT_KEY_IDENTIFIER_SIZE);
+  memcpy(policy->master_key_identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
 }
 
 int init_mount(struct ceph_mount_info** cmount){
@@ -195,9 +194,8 @@ int fscrypt_encrypt(const string& dir_path)
 
   ceph_close(cmount, key_fd);
 
-  struct ceph_fscrypt_key_identifier kid;
-
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 0);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 0);
   if (r < 0) {
     std::clog << __func__ << "(): ceph_add_fscrypt_key() r=" << r << std::endl;
     return r;
@@ -208,7 +206,7 @@ int fscrypt_encrypt(const string& dir_path)
   policy.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
   policy.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
   policy.flags = FSCRYPT_POLICY_FLAGS_PAD_32;
-  memcpy(policy.master_key_identifier, kid.raw, FSCRYPT_KEY_IDENTIFIER_SIZE);
+  memcpy(policy.master_key_identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
 
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
   if (fd < 0) {
@@ -266,17 +264,15 @@ out:
 }
 
 TEST(FSCrypt, MultipleUnlockLockClaims) {
-  struct ceph_fscrypt_key_identifier kid;
-  struct ceph_fscrypt_key_identifier kid2;
-
   struct ceph_mount_info *cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1091);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1091);
   ASSERT_EQ(0, r);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid2, 1299);
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
   ASSERT_EQ(0, r);
   if (r < 0) {
     std::clog << __func__ << "() 1ceph_mount add_fscrypt_key r=" << r << std::endl;
@@ -284,7 +280,7 @@ TEST(FSCrypt, MultipleUnlockLockClaims) {
 
   //remove user 1 of 2, should return 0, but 0x2 status_flag
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -292,7 +288,7 @@ TEST(FSCrypt, MultipleUnlockLockClaims) {
 
    //remove suser 2 of 2, ret 0, 0x0 status_flag
   fscrypt_remove_key_arg arg2;
-  generate_remove_key_arg(kid2, &arg2);
+  generate_remove_key_arg(keyid, &arg2);
 
   r = ceph_remove_fscrypt_key(cmount, &arg2, 1091);
   ASSERT_EQ(0, r);
@@ -301,17 +297,16 @@ TEST(FSCrypt, MultipleUnlockLockClaims) {
 }
 
 TEST(FSCrypt, UnlockKeyUserDNE) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info *cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1091);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1091);
   ASSERT_EQ(0, r);
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -320,17 +315,16 @@ TEST(FSCrypt, UnlockKeyUserDNE) {
 }
 
 TEST(FSCrypt, UnlockKeyDNE) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info *cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
   ASSERT_EQ(0, r);
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -342,20 +336,19 @@ TEST(FSCrypt, UnlockKeyDNE) {
   ceph_shutdown(cmount);
 }
 
-#warning key_remove todo: 'EINVAL: invalid key specifier type, or reserved bits were set' case
+//#warning key_remove todo: 'EINVAL: invalid key specifier type, or reserved bits were set' case
 
 TEST(FSCrypt, SetPolicyEmptyDir) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
   ASSERT_EQ(0, r);
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -364,8 +357,6 @@ TEST(FSCrypt, SetPolicyEmptyDir) {
 }
 
 TEST(FSCrypt, SetPolicyNotEmptyDir) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -382,10 +373,11 @@ TEST(FSCrypt, SetPolicyNotEmptyDir) {
 
   int fd2 = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd2, &policy);
   ASSERT_EQ(-ENOTEMPTY, r);
@@ -394,7 +386,7 @@ TEST(FSCrypt, SetPolicyNotEmptyDir) {
   ASSERT_EQ(0, ceph_rmdir(cmount, dir2_path.c_str()));
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -405,8 +397,6 @@ TEST(FSCrypt, SetPolicyNotEmptyDir) {
 }
 
 TEST(FSCrypt, SetPolicyAlreadyExistSamePolicy) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -416,10 +406,11 @@ TEST(FSCrypt, SetPolicyAlreadyExistSamePolicy) {
 
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
   ASSERT_EQ(0, r);
@@ -432,8 +423,6 @@ TEST(FSCrypt, SetPolicyAlreadyExistSamePolicy) {
 }
 
 TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicy) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -443,10 +432,11 @@ TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicy) {
 
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
   ASSERT_EQ(0, r);
@@ -455,11 +445,11 @@ TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicy) {
   for (int i = 0; i < (int)sizeof(fscrypt_key2); ++i) {
     fscrypt_key2[i] = (char)rand();
   }
-  struct ceph_fscrypt_key_identifier kid2;
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key2, sizeof(fscrypt_key2), &kid2, 1299);
+  char keyid2[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key2, sizeof(fscrypt_key2), keyid2, 1299);
 
   struct fscrypt_policy_v2 policy2;
-  populate_policy(kid2, &policy2);
+  populate_policy(keyid2, &policy2);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy2);
   ASSERT_EQ(-EEXIST, r);
@@ -470,15 +460,15 @@ TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicy) {
 
 TEST(FSCrypt, SetPolicyNonDir) {
   //can be file, symlink, device file etc
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
 
   //setup policy
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  memset(keyid, 0, sizeof(keyid));
+  populate_policy(keyid, &policy);
 
   //file
   string file_path = "file1";
@@ -512,8 +502,6 @@ TEST(FSCrypt, SetPolicyNonDir) {
 }
 
 TEST(FSCrypt, SetPolicyNotSupported) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -523,7 +511,8 @@ TEST(FSCrypt, SetPolicyNotSupported) {
 
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
   policy.version = 2;
@@ -531,7 +520,7 @@ TEST(FSCrypt, SetPolicyNotSupported) {
   policy.filenames_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
   policy.flags = FSCRYPT_POLICY_FLAGS_PAD_32;
   memset(policy.__reserved, 0, sizeof(policy.__reserved));
-  memcpy(policy.master_key_identifier, kid.raw, FSCRYPT_KEY_IDENTIFIER_SIZE);
+  memcpy(policy.master_key_identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
   ASSERT_EQ(-EINVAL, r);
@@ -541,7 +530,7 @@ TEST(FSCrypt, SetPolicyNotSupported) {
   policy.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
   policy.flags = FSCRYPT_POLICY_FLAGS_PAD_32;
   memset(policy.__reserved, 0, sizeof(policy.__reserved));
-  memcpy(policy.master_key_identifier, kid.raw, FSCRYPT_KEY_IDENTIFIER_SIZE);
+  memcpy(policy.master_key_identifier, keyid, FSCRYPT_KEY_IDENTIFIER_SIZE);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
   ASSERT_EQ(-EINVAL, r);
@@ -552,8 +541,6 @@ TEST(FSCrypt, SetPolicyNotSupported) {
 
 
 TEST(FSCrypt, LockedListDir) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -564,10 +551,11 @@ TEST(FSCrypt, LockedListDir) {
   string file_path = "dir1/file5";
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
 
@@ -588,7 +576,7 @@ TEST(FSCrypt, LockedListDir) {
     }
   }
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -618,8 +606,6 @@ done:
 }
 
 TEST(FSCrypt, ReadLockedDir) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -630,10 +616,11 @@ TEST(FSCrypt, ReadLockedDir) {
   string file_path = "dir1/file5";
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
 
@@ -654,7 +641,7 @@ TEST(FSCrypt, ReadLockedDir) {
     }
   }
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -681,8 +668,6 @@ read:
 }
 
 TEST(FSCrypt, WriteLockedDir) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -694,10 +679,11 @@ TEST(FSCrypt, WriteLockedDir) {
   string file_path = "dir1/file5";
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
 
@@ -718,7 +704,7 @@ TEST(FSCrypt, WriteLockedDir) {
     }
   }
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -747,8 +733,6 @@ write:
 }
 
 TEST(FSCrypt, LockedCreateSnap) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -759,10 +743,11 @@ TEST(FSCrypt, LockedCreateSnap) {
   string file_path = "dir1/file5";
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
 
@@ -776,7 +761,7 @@ TEST(FSCrypt, LockedCreateSnap) {
   ASSERT_EQ(0, ceph_unlink(cmount, file_path.c_str()));
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -790,8 +775,6 @@ TEST(FSCrypt, LockedCreateSnap) {
 }
 
 TEST(FSCrypt, RenameLockedSource) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -802,10 +785,11 @@ TEST(FSCrypt, RenameLockedSource) {
   string src_path = "dir1/file5";
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
 
@@ -827,7 +811,7 @@ TEST(FSCrypt, RenameLockedSource) {
   }
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -854,8 +838,6 @@ TEST(FSCrypt, RenameLockedSource) {
 }
 
 TEST(FSCrypt, RenameLockedDest) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -867,10 +849,11 @@ TEST(FSCrypt, RenameLockedDest) {
   string dest_path = "dir1/file_dest";
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
 
@@ -882,7 +865,7 @@ TEST(FSCrypt, RenameLockedDest) {
   ceph_close(cmount, fd2);
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -896,8 +879,6 @@ TEST(FSCrypt, RenameLockedDest) {
 }
 
 TEST(FSCrypt, RemoveBusyFile) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info *cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -907,10 +888,11 @@ TEST(FSCrypt, RemoveBusyFile) {
 
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
   ceph_close(cmount, fd);
@@ -925,7 +907,7 @@ TEST(FSCrypt, RemoveBusyFile) {
   ASSERT_EQ(32, r);
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -935,7 +917,7 @@ TEST(FSCrypt, RemoveBusyFile) {
   ASSERT_EQ(0, ceph_unlink(cmount, path.c_str()));
 
   //actually remove the key
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
   ASSERT_EQ(0, arg.removal_status_flags);
@@ -946,8 +928,6 @@ TEST(FSCrypt, RemoveBusyFile) {
 }
 
 TEST(FSCrypt, RemoveBusyCreate) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info *cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -956,10 +936,11 @@ TEST(FSCrypt, RemoveBusyCreate) {
   ASSERT_EQ(0, ceph_mkdir(cmount, dir_path.c_str(), 0777));
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
   ceph_close(cmount, fd);
@@ -974,7 +955,7 @@ TEST(FSCrypt, RemoveBusyCreate) {
   ASSERT_EQ(32, r);
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -994,7 +975,7 @@ TEST(FSCrypt, RemoveBusyCreate) {
   ASSERT_EQ(0, ceph_unlink(cmount, path.c_str()));
 
   //actually remove the key
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
   ASSERT_EQ(0, arg.removal_status_flags);
@@ -1009,7 +990,6 @@ TEST(FSCrypt, RemoveBusyCreate) {
 // if this test fails, it means that these ops has been impleneted AND we must reject these ops for encrypted files
 // see https://www.kernel.org/doc/html/v4.18/filesystems/fscrypt.html Access Semantics section
 TEST(FSCrypt, FallocateNotImplemented) {
-  struct ceph_fscrypt_key_identifier kid;
   struct ceph_mount_info *cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -1019,9 +999,10 @@ TEST(FSCrypt, FallocateNotImplemented) {
   ASSERT_EQ(0, ceph_mkdir(cmount, dir_path.c_str(), 0777));
 
   int fd = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd, &policy);
   ceph_close(cmount, fd);
@@ -1053,8 +1034,6 @@ TEST(FSCrypt, FallocateNotImplemented) {
 }
 
 TEST(FSCrypt, SetPolicyAlreadyExistSamePolicyNotEmpty) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -1064,10 +1043,11 @@ TEST(FSCrypt, SetPolicyAlreadyExistSamePolicyNotEmpty) {
 
   int fd2 = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd2, &policy);
 
@@ -1085,7 +1065,7 @@ TEST(FSCrypt, SetPolicyAlreadyExistSamePolicyNotEmpty) {
   ASSERT_EQ(0, ceph_unlink(cmount, file_path.c_str()));
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);
@@ -1096,8 +1076,6 @@ TEST(FSCrypt, SetPolicyAlreadyExistSamePolicyNotEmpty) {
 }
 
 TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicyNotEmpty) {
-  struct ceph_fscrypt_key_identifier kid;
-
   struct ceph_mount_info* cmount;
   int r = init_mount(&cmount);
   ASSERT_EQ(0, r);
@@ -1107,10 +1085,11 @@ TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicyNotEmpty) {
 
   int fd2 = ceph_open(cmount, dir_path.c_str(), O_DIRECTORY, 0);
 
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), &kid, 1299);
+  char keyid[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key, sizeof(fscrypt_key), keyid, 1299);
 
   struct fscrypt_policy_v2 policy;
-  populate_policy(kid, &policy);
+  populate_policy(keyid, &policy);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd2, &policy);
 
@@ -1125,11 +1104,11 @@ TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicyNotEmpty) {
   for (int i = 0; i < (int)sizeof(fscrypt_key2); ++i) {
     fscrypt_key2[i] = (char)rand();
   }
-  struct ceph_fscrypt_key_identifier kid2;
-  r = ceph_add_fscrypt_key(cmount, fscrypt_key2, sizeof(fscrypt_key2), &kid2, 1299);
+  char keyid2[FSCRYPT_KEY_IDENTIFIER_SIZE];
+  r = ceph_add_fscrypt_key(cmount, fscrypt_key2, sizeof(fscrypt_key2), keyid2, 1299);
 
   struct fscrypt_policy_v2 policy2;
-  populate_policy(kid2, &policy2);
+  populate_policy(keyid2, &policy2);
 
   r = ceph_set_fscrypt_policy_v2(cmount, fd2, &policy2);
 
@@ -1138,7 +1117,7 @@ TEST(FSCrypt, SetPolicyAlreadyExistDifferentPolicyNotEmpty) {
   ASSERT_EQ(0, ceph_unlink(cmount, file_path.c_str()));
 
   fscrypt_remove_key_arg arg;
-  generate_remove_key_arg(kid, &arg);
+  generate_remove_key_arg(keyid, &arg);
 
   r = ceph_remove_fscrypt_key(cmount, &arg, 1299);
   ASSERT_EQ(0, r);