diff mbox

f2fs: support access control via key management

Message ID 1457571168-56724-1-git-send-email-jaegeuk@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jaegeuk Kim March 10, 2016, 12:52 a.m. UTC
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 <jaegeuk@kernel.org>
---
 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(+)

Comments

kernel test robot March 10, 2016, 2:05 a.m. UTC | #1
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
kernel test robot March 10, 2016, 2:18 a.m. UTC | #2
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
Christoph Hellwig March 15, 2016, 7:24 a.m. UTC | #3
On Wed, Mar 09, 2016 at 04:52:48PM -0800, Jaegeuk Kim wrote:
> 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 <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jaegeuk Kim March 15, 2016, 4:37 p.m. UTC | #4
Hello,

On Tue, Mar 15, 2016 at 12:24:22AM -0700, Christoph Hellwig wrote:
> On Wed, Mar 09, 2016 at 04:52:48PM -0800, Jaegeuk Kim wrote:
> > 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,

> 
> Nacked-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig March 21, 2016, 3:39 p.m. UTC | #5
On Tue, Mar 15, 2016 at 09:37:25AM -0700, Jaegeuk Kim wrote:
> 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?

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bbe2cd1..cbc115b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -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
 
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b41c357..80f6b29 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -21,6 +21,7 @@ 
 #include <linux/mount.h>
 #include <linux/pagevec.h>
 #include <linux/random.h>
+#include <keys/user-type.h>
 
 #include "f2fs.h"
 #include "node.h"
@@ -438,6 +439,40 @@  static int f2fs_file_mmap(struct file *file, struct vm_area_struct *vma)
 	return 0;
 }
 
+static int validate_access_key(struct inode *inode)
+{
+	u8 full_key_descriptor[F2FS_KEY_DESC_PREFIX_SIZE +
+					(F2FS_KEY_SIZE * 2) + 1];
+	struct key *keyring_key = NULL;
+	u8 key[F2FS_KEY_SIZE];
+	int ret;
+
+	ret = f2fs_getxattr(inode, F2FS_XATTR_INDEX_KEY,
+				F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
+				key, F2FS_KEY_SIZE, NULL);
+	if (ret != F2FS_KEY_SIZE)
+		return -EINVAL;
+
+	memcpy(full_key_descriptor, F2FS_KEY_DESC_PREFIX,
+					F2FS_KEY_DESC_PREFIX_SIZE);
+	sprintf(full_key_descriptor + F2FS_KEY_DESC_PREFIX_SIZE,
+					"%*phN", F2FS_KEY_SIZE, key);
+	full_key_descriptor[F2FS_KEY_DESC_PREFIX_SIZE +
+					(2 * F2FS_KEY_SIZE)] = '\0';
+	keyring_key = request_key(&key_type_logon, full_key_descriptor, NULL);
+	if (IS_ERR(keyring_key))
+		return PTR_ERR(keyring_key);
+
+	if (keyring_key->type != &key_type_logon) {
+		printk_once(KERN_WARNING
+				"%s: key type must be logon\n", __func__);
+		key_put(keyring_key);
+		return -ENOKEY;
+	}
+	key_put(keyring_key);
+	return 0;
+}
+
 static int f2fs_file_open(struct inode *inode, struct file *filp)
 {
 	int ret = generic_file_open(inode, filp);
@@ -453,6 +488,9 @@  static int f2fs_file_open(struct inode *inode, struct file *filp)
 	if (f2fs_encrypted_inode(dir) &&
 			!fscrypt_has_permitted_context(dir, inode))
 		return -EPERM;
+
+	if (file_has_key(inode))
+		return validate_access_key(inode);
 	return ret;
 }
 
@@ -1532,6 +1570,35 @@  static bool uuid_is_nonzero(__u8 u[16])
 	return false;
 }
 
+static int f2fs_ioc_keyctl(struct file *filp, unsigned long arg)
+{
+	struct inode *inode = file_inode(filp);
+	struct f2fs_key key;
+	void *value = key.key;
+	int type = XATTR_CREATE;
+
+	if (copy_from_user(&key, (u8 __user *)arg, sizeof(key)))
+		return -EFAULT;
+
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
+	if (key.mode == F2FS_DROP_KEY) {
+		int ret = validate_access_key(inode);
+
+		if (ret)
+			return ret;
+
+		value = NULL;
+		type = XATTR_REPLACE;
+	}
+
+	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+	return f2fs_setxattr(inode, F2FS_XATTR_INDEX_KEY,
+				F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
+				value, F2FS_KEY_SIZE, NULL, type);
+}
+
 static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
 {
 	struct fscrypt_policy policy;
@@ -1845,6 +1912,8 @@  long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return f2fs_ioc_shutdown(filp, arg);
 	case FITRIM:
 		return f2fs_ioc_fitrim(filp, arg);
+	case F2FS_IOC_KEYCTL:
+		return f2fs_ioc_keyctl(filp, arg);
 	case F2FS_IOC_SET_ENCRYPTION_POLICY:
 		return f2fs_ioc_set_encryption_policy(filp, arg);
 	case F2FS_IOC_GET_ENCRYPTION_POLICY:
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 06a72dc..5b02b31 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -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);
diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
index 79dccc8..cb5282c 100644
--- a/fs/f2fs/xattr.h
+++ b/fs/f2fs/xattr.h
@@ -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"