Skip to contentSkip to conversation

f2fs: support access control via key management

PATCH NAK Jaegeuk Kim 3 participants 6 messages Mar 10, 2016 – Mar 21, 2016

Subsystems (2)
VFS F2FS file system
lore
Share & tools
lore

Jaegeuk Kim

author maintainer
#0 lore raw

PATCH 1/1 f2fs: support access control via key management

Through this patch, user can assign its key into a specific normal files. Then, other users who do not have that key cannot open the files. Later, owner can drop its key from the files for other users to access the files again.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/f2fs.h  | 20 +++++++++++++++++
 fs/f2fs/file.c  | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/f2fs/xattr.c |  7 ++++++
 fs/f2fs/xattr.h |  1 +
 4 files changed, 97 insertions(+)
fs/f2fs/f2fs.h +20 -0
index bbe2cd1..cbc115b 100644
@@ -212,6 +212,8 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_IOC_WRITE_CHECKPOINT	_IO(F2FS_IOCTL_MAGIC, 7)
 #define F2FS_IOC_DEFRAGMENT		_IO(F2FS_IOCTL_MAGIC, 8)
 
+#define F2FS_IOC_KEYCTL		_IOW(F2FS_IOCTL_MAGIC, 9, struct f2fs_key)
+
 #define F2FS_IOC_SET_ENCRYPTION_POLICY	FS_IOC_SET_ENCRYPTION_POLICY
 #define F2FS_IOC_GET_ENCRYPTION_POLICY	FS_IOC_GET_ENCRYPTION_POLICY
 #define F2FS_IOC_GET_ENCRYPTION_PWSALT	FS_IOC_GET_ENCRYPTION_PWSALT

@@ -235,6 +237,20 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_IOC32_GETVERSION		FS_IOC32_GETVERSION
 #endif
 
+#define F2FS_KEY_SIZE	8
+#define F2FS_KEY_DESC_PREFIX		"f2fs:"
+#define F2FS_KEY_DESC_PREFIX_SIZE	5
+
+enum f2fs_key_mode {
+	F2FS_SET_KEY,
+	F2FS_DROP_KEY,
+};
+
+struct f2fs_key {
+	u8 key[F2FS_KEY_SIZE];
+	u8 mode;
+};
+
 struct f2fs_defragment {
 	u64 start;
 	u64 len;

@@ -358,6 +374,7 @@ struct f2fs_map_blocks {
 #define FADVISE_LOST_PINO_BIT	0x02
 #define FADVISE_ENCRYPT_BIT	0x04
 #define FADVISE_ENC_NAME_BIT	0x08
+#define FADVISE_KEY_BIT		0x10
 
 #define file_is_cold(inode)	is_file(inode, FADVISE_COLD_BIT)
 #define file_wrong_pino(inode)	is_file(inode, FADVISE_LOST_PINO_BIT)

@@ -370,6 +387,9 @@ struct f2fs_map_blocks {
 #define file_clear_encrypt(inode) clear_file(inode, FADVISE_ENCRYPT_BIT)
 #define file_enc_name(inode)	is_file(inode, FADVISE_ENC_NAME_BIT)
 #define file_set_enc_name(inode) set_file(inode, FADVISE_ENC_NAME_BIT)
+#define file_has_key(inode)	is_file(inode, FADVISE_KEY_BIT)
+#define file_set_key(inode)	set_file(inode, FADVISE_KEY_BIT)
+#define file_clear_key(inode)	clear_file(inode, FADVISE_KEY_BIT)
 
 #define DEF_DIR_LEVEL		0
fs/f2fs/file.c +69 -0

99 lines not on this page · raw message

fs/f2fs/xattr.c +7 -0
index 06a72dc..5b02b31 100644
@@ -550,6 +550,13 @@ static int __f2fs_setxattr(struct inode *inode, int index,
 	if (index == F2FS_XATTR_INDEX_ENCRYPTION &&
 			!strcmp(name, F2FS_XATTR_NAME_ENCRYPTION_CONTEXT))
 		f2fs_set_encrypted_inode(inode);
+	if (index == F2FS_XATTR_INDEX_KEY &&
+			!strcmp(name, F2FS_XATTR_NAME_ENCRYPTION_CONTEXT)) {
+		if (value)
+			file_set_key(inode);
+		else
+			file_clear_key(inode);
+	}
 
 	if (ipage)
 		update_inode(inode, ipage);
fs/f2fs/xattr.h +1 -0
index 79dccc8..cb5282c 100644
@@ -37,6 +37,7 @@
 #define F2FS_XATTR_INDEX_ADVISE			7
 /* Should be same as EXT4_XATTR_INDEX_ENCRYPTION */
 #define F2FS_XATTR_INDEX_ENCRYPTION		9
+#define F2FS_XATTR_INDEX_KEY			10
 
 #define F2FS_XATTR_NAME_ENCRYPTION_CONTEXT	"c"
Signature · 1 line
2.6.3
build bot
#1 lore raw
reproduce:

Hi Jaegeuk,

[auto build test ERROR on f2fs/dev]
[also build test ERROR on next-20160309]
[cannot apply to v4.5-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jaegeuk-Kim/f2fs-support-access-control-via-key-management/20160310-085442
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs dev
config: x86_64-randconfig-i0-03091236 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64

All error/warnings (new ones prefixed by >>):

In file included from include/uapi/linux/posix_types.h:4:0,

                    from include/uapi/linux/types.h:13,
                    from include/linux/compiler.h:199,
                    from include/linux/linkage.h:4,
                    from include/linux/fs.h:4,
                    from fs/f2fs/file.c:11:
   fs/f2fs/file.c: In function 'f2fs_ioc_keyctl':
>> include/linux/stddef.h:7:14: warning: passing argument 6 of 'f2fs_setxattr' makes integer from pointer without a cast [-Wint-conversion]
    #define NULL ((void *)0)
                 ^
>> fs/f2fs/file.c:1599:27: note: in expansion of macro 'NULL'
        value, F2FS_KEY_SIZE, NULL, type);
                              ^
   In file included from fs/f2fs/file.c:29:0:
   fs/f2fs/xattr.h:129:19: note: expected 'int' but argument is of type 'void *'
    static inline int f2fs_setxattr(struct inode *inode, int index,
                      ^
>> fs/f2fs/file.c:1597:9: error: too many arguments to function 'f2fs_setxattr'

return f2fs_setxattr(inode, F2FS_XATTR_INDEX_KEY,
^
In file included from fs/f2fs/file.c:29:0:
fs/f2fs/xattr.h:129:19: note: declared here
static inline int f2fs_setxattr(struct inode *inode, int index,
^

vim +/f2fs_setxattr +1597 fs/f2fs/file.c

1591
1592 value = NULL;
1593 type = XATTR_REPLACE;
1594 }
1595
1596 f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);

> 1597		return f2fs_setxattr(inode, F2FS_XATTR_INDEX_KEY,
  1598					F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
> 1599					value, F2FS_KEY_SIZE, NULL, type);

1600 }
1601
1602 static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

build bot
#2 lore raw
In reply to Jaegeuk Kim
reproduce:

Hi Jaegeuk,

[auto build test ERROR on f2fs/dev]
[also build test ERROR on next-20160309]
[cannot apply to v4.5-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jaegeuk-Kim/f2fs-support-access-control-via-key-management/20160310-085442
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs dev
config: x86_64-randconfig-s0-03100948 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

fs/f2fs/file.c: In function 'validate_access_key':

>> fs/f2fs/file.c:462:16: error: implicit declaration of function 'request_key' [-Werror=implicit-function-declaration]
     keyring_key = request_key(&key_type_logon, full_key_descriptor, NULL);
                   ^
>> fs/f2fs/file.c:462:29: error: 'key_type_logon' undeclared (first use in this function)
     keyring_key = request_key(&key_type_logon, full_key_descriptor, NULL);
                                ^
   fs/f2fs/file.c:462:29: note: each undeclared identifier is reported only once for each function it appears in
>> fs/f2fs/file.c:466:17: error: dereferencing pointer to incomplete type 'struct key'
     if (keyring_key->type != &key_type_logon) {
                    ^
   In file included from include/uapi/linux/posix_types.h:4:0,
                    from include/uapi/linux/types.h:13,
                    from include/linux/compiler.h:199,
                    from include/linux/linkage.h:4,
                    from include/linux/fs.h:4,
                    from fs/f2fs/file.c:11:
   fs/f2fs/file.c: In function 'f2fs_ioc_keyctl':
   include/linux/stddef.h:7:14: warning: passing argument 6 of 'f2fs_setxattr' makes integer from pointer without a cast [-Wint-conversion]
    #define NULL ((void *)0)
                 ^
   fs/f2fs/file.c:1599:27: note: in expansion of macro 'NULL'
        value, F2FS_KEY_SIZE, NULL, type);
                              ^

In file included from fs/f2fs/file.c:29:0:
fs/f2fs/xattr.h:129:19: note: expected 'int' but argument is of type 'void *'
static inline int f2fs_setxattr(struct inode *inode, int index,
^
fs/f2fs/file.c:1597:9: error: too many arguments to function 'f2fs_setxattr'
return f2fs_setxattr(inode, F2FS_XATTR_INDEX_KEY,
^
In file included from fs/f2fs/file.c:29:0:
fs/f2fs/xattr.h:129:19: note: declared here
static inline int f2fs_setxattr(struct inode *inode, int index,
^
cc1: some warnings being treated as errors

vim +/request_key +462 fs/f2fs/file.c

456 memcpy(full_key_descriptor, F2FS_KEY_DESC_PREFIX,
457 F2FS_KEY_DESC_PREFIX_SIZE);
458 sprintf(full_key_descriptor + F2FS_KEY_DESC_PREFIX_SIZE,
459 "%*phN", F2FS_KEY_SIZE, key);
460 full_key_descriptor[F2FS_KEY_DESC_PREFIX_SIZE +
461 (2 * F2FS_KEY_SIZE)] = '\0';

 > 462		keyring_key = request_key(&key_type_logon, full_key_descriptor, NULL);
   463		if (IS_ERR(keyring_key))
   464			return PTR_ERR(keyring_key);
   465	
 > 466		if (keyring_key->type != &key_type_logon) {

467 printk_once(KERN_WARNING
468 "%s: key type must be logon\n", __func__);
469 key_put(keyring_key);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Nacked-by on v1 1/1 NAK
#3 lore raw
In reply to Jaegeuk Kim
Quoted 4 lines · Jaegeuk Kim

Through this patch, user can assign its key into a specific normal files. Then, other users who do not have that key cannot open the files. Later, owner can drop its key from the files for other users to access the files again.

No magic file system specific access control, please:

Nacked-by Christoph Hellwig <[email protected]>
author maintainer
#4 lore raw

Hello,

Quoted 7 lines · Christoph Hellwig
Quoted 4 lines · Jaegeuk Kim

Through this patch, user can assign its key into a specific normal files. Then, other users who do not have that key cannot open the files. Later, owner can drop its key from the files for other users to access the files again.

No magic file system specific access control, please:

I agree that I must follow FS convention here.
But, in order to make this clear out, could you please elaborate why this is not allowed?

I wrote this patch totally based on per-file encryption in which users cannot access their files if they have no right key.
The only difference is that this controls user access with a key only, neither encrypting file data nor dentries.

This was initiated by UX in android letting nobody be able to access the files that owner wants to protect by passcode or fingerprint.

Does it make no sense to support this by filesystems?

Thanks,

Quoted 2 lines
Nacked-by Christoph Hellwig <[email protected]>
#5 lore raw
Quoted 9 lines · Jaegeuk Kim

I agree that I must follow FS convention here.
But, in order to make this clear out, could you please elaborate why this is not allowed?

I wrote this patch totally based on per-file encryption in which users cannot access their files if they have no right key.
The only difference is that this controls user access with a key only, neither encrypting file data nor dentries.

Quoted 4 lines

This was initiated by UX in android letting nobody be able to access the files that owner wants to protect by passcode or fingerprint.

Does it make no sense to support this by filesystems?

I don't think it does. But if you want to argue for it you should

a) support it in the VFS
b) document the exact semantics
c) ensure linux-man and linux-api are on the Cc list.