RFC: support keyrings for NFS TLS mounts

RFC: support keyrings for NFS TLS mounts

[Christoph Hellwig]

Hi all,

this series allows storing the key and certificate for NFS over
TLS mounts in the keyring and be specified using a mount option.
This way they don't need to be hardcoded in the global tlshd.conf
configuration file and can even be different per-mount.

Note that for now the .nfs keyring still needs to be added to
tlshd.conf, but I'm going to look into a way out of that.

This is in many ways based on how NVMe handles the keyring for
TLS, and I might not fully understand what I'm doing.

[PATCH 1/2] NFS: support the kernel keyring for TLS

[Christoph Hellwig]

Allow tlshd to use a per-mount key from the kernel keyring similar
to NVMe over TCP.

Note that tlshd expects keys and certificates stored in the kernel
keyring to be in DER format, not the PEM format used for file based keys
and certificates, so they need to be converted before they are added
to the keyring, which is a bit unexpected.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/fs_context.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 13f71ca8c974..58845c414893 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -96,6 +96,8 @@ enum nfs_param {
 	Opt_wsize,
 	Opt_write,
 	Opt_xprtsec,
+	Opt_cert_serial,
+	Opt_privkey_serial,
 };
 
 enum {
@@ -221,6 +223,8 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
 	fsparam_enum  ("write",		Opt_write, nfs_param_enums_write),
 	fsparam_u32   ("wsize",		Opt_wsize),
 	fsparam_string("xprtsec",	Opt_xprtsec),
+	fsparam_s32("cert_serial",	Opt_cert_serial),
+	fsparam_s32("privkey_serial",	Opt_privkey_serial),
 	{}
 };
 
@@ -551,6 +555,25 @@ static int nfs_parse_version_string(struct fs_context *fc,
 	return 0;
 }
 
+static int nfs_tls_key_verify(key_serial_t key_id)
+{
+	struct key *key = key_lookup(key_id);
+	int error = 0;
+
+	if (IS_ERR(key)) {
+		pr_err("key id %08x not found\n", key_id);
+		return PTR_ERR(key);
+	}
+	if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
+	    test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
+		pr_err("key id %08x revoked\n", key_id);
+		error = -EKEYREVOKED;
+	}
+
+	key_put(key);
+	return error;
+}
+
 /*
  * Parse a single mount parameter.
  */
@@ -807,6 +830,18 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		if (ret < 0)
 			return ret;
 		break;
+	case Opt_cert_serial:
+		ret = nfs_tls_key_verify(result.int_32);
+		if (ret < 0)
+			return ret;
+		ctx->xprtsec.cert_serial = result.int_32;
+		break;
+	case Opt_privkey_serial:
+		ret = nfs_tls_key_verify(result.int_32);
+		if (ret < 0)
+			return ret;
+		ctx->xprtsec.privkey_serial = result.int_32;
+		break;
 
 	case Opt_proto:
 		if (!param->string)
-- 
2.47.2

[PATCH 2/2] nfs: create a kernel keyring

[Christoph Hellwig]

Create a kernel .nfs keyring similar to the nvme .nvme one.  Unlike for
a userspace-created keyrind, tlshd is a possesor of the keys with this
and thus the keys don't need user read permissions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/inode.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 119e447758b9..fb1fe1bdfe92 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -2571,6 +2571,8 @@ static struct pernet_operations nfs_net_ops = {
 	.size = sizeof(struct nfs_net),
 };
 
+static struct key *nfs_keyring;
+
 /*
  * Initialize NFS
  */
@@ -2578,6 +2580,17 @@ static int __init init_nfs_fs(void)
 {
 	int err;
 
+	if (IS_ENABLED(CONFIG_NFS_V4)) {
+		nfs_keyring = keyring_alloc(".nfs",
+				     GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+				     current_cred(),
+				     (KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				     (KEY_USR_ALL & ~KEY_USR_SETATTR),
+				     KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
+		if (IS_ERR(nfs_keyring))
+			return PTR_ERR(nfs_keyring);
+	}
+
 	err = nfs_sysfs_init();
 	if (err < 0)
 		goto out10;
@@ -2653,6 +2666,8 @@ static void __exit exit_nfs_fs(void)
 	nfs_fs_proc_exit();
 	nfsiod_stop();
 	nfs_sysfs_exit();
+	if (IS_ENABLED(CONFIG_NFS_V4))
+		key_put(nfs_keyring);
 }
 
 /* Not quite true; I just maintain it */
-- 
2.47.2

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Sagi Grimberg]

Pretty straight-forward I think.

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 2/2] nfs: create a kernel keyring

[Sagi Grimberg]

On 07/05/2025 11:09, Christoph Hellwig wrote:
> Create a kernel .nfs keyring similar to the nvme .nvme one.  Unlike for
> a userspace-created keyrind, tlshd is a possesor of the keys with this
> and thus the keys don't need user read permissions.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/nfs/inode.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 119e447758b9..fb1fe1bdfe92 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -2571,6 +2571,8 @@ static struct pernet_operations nfs_net_ops = {
>   	.size = sizeof(struct nfs_net),
>   };
>   
> +static struct key *nfs_keyring;
> +
>   /*
>    * Initialize NFS
>    */
> @@ -2578,6 +2580,17 @@ static int __init init_nfs_fs(void)
>   {
>   	int err;
>   
> +	if (IS_ENABLED(CONFIG_NFS_V4)) {

xprtsec is sunrpc, meaning it is also supported with nfsv3.

> +		nfs_keyring = keyring_alloc(".nfs",
> +				     GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
> +				     current_cred(),
> +				     (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +				     (KEY_USR_ALL & ~KEY_USR_SETATTR),
> +				     KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
> +		if (IS_ERR(nfs_keyring))
> +			return PTR_ERR(nfs_keyring);
> +	}
> +
>   	err = nfs_sysfs_init();
>   	if (err < 0)
>   		goto out10;
> @@ -2653,6 +2666,8 @@ static void __exit exit_nfs_fs(void)
>   	nfs_fs_proc_exit();
>   	nfsiod_stop();
>   	nfs_sysfs_exit();
> +	if (IS_ENABLED(CONFIG_NFS_V4))
> +		key_put(nfs_keyring);

Same comment

>   }
>   
>   /* Not quite true; I just maintain it */

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Chuck Lever]

On 5/7/25 4:09 AM, Christoph Hellwig wrote:
> Allow tlshd to use a per-mount key from the kernel keyring similar
> to NVMe over TCP.
> 
> Note that tlshd expects keys and certificates stored in the kernel
> keyring to be in DER format, not the PEM format used for file based keys
> and certificates, so they need to be converted before they are added
> to the keyring, which is a bit unexpected.

I proposed adding support to the kernel's x.509 implementation for PEM
format keys, and it was rejected. So DER it is.


> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/nfs/fs_context.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 13f71ca8c974..58845c414893 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -96,6 +96,8 @@ enum nfs_param {
>  	Opt_wsize,
>  	Opt_write,
>  	Opt_xprtsec,
> +	Opt_cert_serial,
> +	Opt_privkey_serial,
>  };
>  
>  enum {
> @@ -221,6 +223,8 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
>  	fsparam_enum  ("write",		Opt_write, nfs_param_enums_write),
>  	fsparam_u32   ("wsize",		Opt_wsize),
>  	fsparam_string("xprtsec",	Opt_xprtsec),
> +	fsparam_s32("cert_serial",	Opt_cert_serial),
> +	fsparam_s32("privkey_serial",	Opt_privkey_serial),
>  	{}
>  };
>  
> @@ -551,6 +555,25 @@ static int nfs_parse_version_string(struct fs_context *fc,
>  	return 0;
>  }
>  
> +static int nfs_tls_key_verify(key_serial_t key_id)
> +{
> +	struct key *key = key_lookup(key_id);
> +	int error = 0;
> +
> +	if (IS_ERR(key)) {
> +		pr_err("key id %08x not found\n", key_id);
> +		return PTR_ERR(key);
> +	}
> +	if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
> +	    test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
> +		pr_err("key id %08x revoked\n", key_id);
> +		error = -EKEYREVOKED;
> +	}
> +
> +	key_put(key);
> +	return error;
> +}
> +
>  /*
>   * Parse a single mount parameter.
>   */
> @@ -807,6 +830,18 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
>  		if (ret < 0)
>  			return ret;
>  		break;
> +	case Opt_cert_serial:
> +		ret = nfs_tls_key_verify(result.int_32);
> +		if (ret < 0)
> +			return ret;
> +		ctx->xprtsec.cert_serial = result.int_32;
> +		break;
> +	case Opt_privkey_serial:
> +		ret = nfs_tls_key_verify(result.int_32);
> +		if (ret < 0)
> +			return ret;
> +		ctx->xprtsec.privkey_serial = result.int_32;
> +		break;
>  
>  	case Opt_proto:
>  		if (!param->string)


-- 
Chuck Lever

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[kernel test robot]

Hi Christoph,

kernel test robot noticed the following build errors:

[auto build test ERROR on trondmy-nfs/linux-next]
[also build test ERROR on linus/master v6.15-rc5 next-20250507]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-Hellwig/nfs-create-a-kernel-keyring/20250507-171041
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
patch link:    https://lore.kernel.org/r/20250507080944.3947782-2-hch%40lst.de
patch subject: [PATCH 1/2] NFS: support the kernel keyring for TLS
config: hexagon-defconfig (https://download.01.org/0day-ci/archive/20250508/202505081535.3PS62D63-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505081535.3PS62D63-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505081535.3PS62D63-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/nfs/fs_context.c:567:37: error: incomplete definition of type 'struct key'
     567 |         if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
         |                                         ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:45:37: note: expanded from macro 'bitop'
      45 |           __builtin_constant_p((uintptr_t)(addr) != (uintptr_t)NULL) && \
         |                                            ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
>> fs/nfs/fs_context.c:567:37: error: incomplete definition of type 'struct key'
     567 |         if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
         |                                         ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:46:16: note: expanded from macro 'bitop'
      46 |           (uintptr_t)(addr) != (uintptr_t)NULL &&                       \
         |                       ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
>> fs/nfs/fs_context.c:567:37: error: incomplete definition of type 'struct key'
     567 |         if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
         |                                         ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:47:50: note: expanded from macro 'bitop'
      47 |           __builtin_constant_p(*(const unsigned long *)(addr))) ?       \
         |                                                         ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
>> fs/nfs/fs_context.c:567:15: error: use of undeclared identifier 'KEY_FLAG_REVOKED'
     567 |         if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
         |                      ^
>> fs/nfs/fs_context.c:567:37: error: incomplete definition of type 'struct key'
     567 |         if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
         |                                         ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:48:17: note: expanded from macro 'bitop'
      48 |          const##op(nr, addr) : op(nr, addr))
         |                        ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
>> fs/nfs/fs_context.c:567:37: error: incomplete definition of type 'struct key'
     567 |         if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
         |                                         ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:48:32: note: expanded from macro 'bitop'
      48 |          const##op(nr, addr) : op(nr, addr))
         |                                       ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
>> fs/nfs/fs_context.c:567:15: error: use of undeclared identifier 'KEY_FLAG_REVOKED'
     567 |         if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
         |                      ^
>> fs/nfs/fs_context.c:567:15: error: use of undeclared identifier 'KEY_FLAG_REVOKED'
   fs/nfs/fs_context.c:568:41: error: incomplete definition of type 'struct key'
     568 |             test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
         |                                             ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:45:37: note: expanded from macro 'bitop'
      45 |           __builtin_constant_p((uintptr_t)(addr) != (uintptr_t)NULL) && \
         |                                            ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
   fs/nfs/fs_context.c:568:41: error: incomplete definition of type 'struct key'
     568 |             test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
         |                                             ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:46:16: note: expanded from macro 'bitop'
      46 |           (uintptr_t)(addr) != (uintptr_t)NULL &&                       \
         |                       ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
   fs/nfs/fs_context.c:568:41: error: incomplete definition of type 'struct key'
     568 |             test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
         |                                             ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:47:50: note: expanded from macro 'bitop'
      47 |           __builtin_constant_p(*(const unsigned long *)(addr))) ?       \
         |                                                         ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
>> fs/nfs/fs_context.c:568:15: error: use of undeclared identifier 'KEY_FLAG_INVALIDATED'
     568 |             test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
         |                      ^
   fs/nfs/fs_context.c:568:41: error: incomplete definition of type 'struct key'
     568 |             test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
         |                                             ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:48:17: note: expanded from macro 'bitop'
      48 |          const##op(nr, addr) : op(nr, addr))
         |                        ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
   fs/nfs/fs_context.c:568:41: error: incomplete definition of type 'struct key'
     568 |             test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
         |                                             ~~~^
   include/linux/bitops.h:61:50: note: expanded from macro 'test_bit'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                                              ^~~~
   include/linux/bitops.h:48:32: note: expanded from macro 'bitop'
      48 |          const##op(nr, addr) : op(nr, addr))
         |                                       ^~~~
   include/linux/key.h:33:8: note: forward declaration of 'struct key'
      33 | struct key;
         |        ^
>> fs/nfs/fs_context.c:568:15: error: use of undeclared identifier 'KEY_FLAG_INVALIDATED'
     568 |             test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
         |                      ^
>> fs/nfs/fs_context.c:568:15: error: use of undeclared identifier 'KEY_FLAG_INVALIDATED'
   16 errors generated.


vim +567 fs/nfs/fs_context.c

   557	
   558	static int nfs_tls_key_verify(key_serial_t key_id)
   559	{
   560		struct key *key = key_lookup(key_id);
   561		int error = 0;
   562	
   563		if (IS_ERR(key)) {
   564			pr_err("key id %08x not found\n", key_id);
   565			return PTR_ERR(key);
   566		}
 > 567		if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
 > 568		    test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
   569			pr_err("key id %08x revoked\n", key_id);
   570			error = -EKEYREVOKED;
   571		}
   572	
   573		key_put(key);
   574		return error;
   575	}
   576	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] nfs: create a kernel keyring

[kernel test robot]

Hi Christoph,

kernel test robot noticed the following build errors:

[auto build test ERROR on trondmy-nfs/linux-next]
[also build test ERROR on linus/master v6.15-rc5 next-20250507]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-Hellwig/nfs-create-a-kernel-keyring/20250507-171041
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
patch link:    https://lore.kernel.org/r/20250507080944.3947782-3-hch%40lst.de
patch subject: [PATCH 2/2] nfs: create a kernel keyring
config: hexagon-defconfig (https://download.01.org/0day-ci/archive/20250508/202505081720.vtCPwAc0-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505081720.vtCPwAc0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505081720.vtCPwAc0-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/nfs/inode.c:2584:17: error: call to undeclared function 'keyring_alloc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2584 |                 nfs_keyring = keyring_alloc(".nfs",
         |                               ^
>> fs/nfs/inode.c:2587:11: error: use of undeclared identifier 'KEY_POS_ALL'
    2587 |                                      (KEY_POS_ALL & ~KEY_POS_SETATTR) |
         |                                       ^
>> fs/nfs/inode.c:2587:26: error: use of undeclared identifier 'KEY_POS_SETATTR'
    2587 |                                      (KEY_POS_ALL & ~KEY_POS_SETATTR) |
         |                                                      ^
>> fs/nfs/inode.c:2588:11: error: use of undeclared identifier 'KEY_USR_ALL'
    2588 |                                      (KEY_USR_ALL & ~KEY_USR_SETATTR),
         |                                       ^
>> fs/nfs/inode.c:2588:26: error: use of undeclared identifier 'KEY_USR_SETATTR'
    2588 |                                      (KEY_USR_ALL & ~KEY_USR_SETATTR),
         |                                                      ^
>> fs/nfs/inode.c:2589:10: error: use of undeclared identifier 'KEY_ALLOC_NOT_IN_QUOTA'
    2589 |                                      KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
         |                                      ^
   6 errors generated.


vim +/keyring_alloc +2584 fs/nfs/inode.c

  2575	
  2576	/*
  2577	 * Initialize NFS
  2578	 */
  2579	static int __init init_nfs_fs(void)
  2580	{
  2581		int err;
  2582	
  2583		if (IS_ENABLED(CONFIG_NFS_V4)) {
> 2584			nfs_keyring = keyring_alloc(".nfs",
  2585					     GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
  2586					     current_cred(),
> 2587					     (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> 2588					     (KEY_USR_ALL & ~KEY_USR_SETATTR),
> 2589					     KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
  2590			if (IS_ERR(nfs_keyring))
  2591				return PTR_ERR(nfs_keyring);
  2592		}
  2593	
  2594		err = nfs_sysfs_init();
  2595		if (err < 0)
  2596			goto out10;
  2597	
  2598		err = register_pernet_subsys(&nfs_net_ops);
  2599		if (err < 0)
  2600			goto out9;
  2601	
  2602		err = nfsiod_start();
  2603		if (err)
  2604			goto out7;
  2605	
  2606		err = nfs_fs_proc_init();
  2607		if (err)
  2608			goto out6;
  2609	
  2610		err = nfs_init_nfspagecache();
  2611		if (err)
  2612			goto out5;
  2613	
  2614		err = nfs_init_inodecache();
  2615		if (err)
  2616			goto out4;
  2617	
  2618		err = nfs_init_readpagecache();
  2619		if (err)
  2620			goto out3;
  2621	
  2622		err = nfs_init_writepagecache();
  2623		if (err)
  2624			goto out2;
  2625	
  2626		err = nfs_init_directcache();
  2627		if (err)
  2628			goto out1;
  2629	
  2630		err = register_nfs_fs();
  2631		if (err)
  2632			goto out0;
  2633	
  2634		return 0;
  2635	out0:
  2636		nfs_destroy_directcache();
  2637	out1:
  2638		nfs_destroy_writepagecache();
  2639	out2:
  2640		nfs_destroy_readpagecache();
  2641	out3:
  2642		nfs_destroy_inodecache();
  2643	out4:
  2644		nfs_destroy_nfspagecache();
  2645	out5:
  2646		nfs_fs_proc_exit();
  2647	out6:
  2648		nfsiod_stop();
  2649	out7:
  2650		unregister_pernet_subsys(&nfs_net_ops);
  2651	out9:
  2652		nfs_sysfs_exit();
  2653	out10:
  2654		return err;
  2655	}
  2656	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 1/2] NFS: support the kernel keyring for TLS

[Christoph Hellwig]

Allow tlshd to use a per-mount key from the kernel keyring similar
to NVMe over TCP.

Note that tlshd expects keys and certificates stored in the kernel
keyring to be in DER format, not the PEM format used for file based keys
and certificates, so they need to be converted before they are added
to the keyring, which is a bit unexpected.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/fs_context.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 13f71ca8c974..9e94d18448ff 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -96,6 +96,8 @@ enum nfs_param {
 	Opt_wsize,
 	Opt_write,
 	Opt_xprtsec,
+	Opt_cert_serial,
+	Opt_privkey_serial,
 };
 
 enum {
@@ -221,6 +223,8 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
 	fsparam_enum  ("write",		Opt_write, nfs_param_enums_write),
 	fsparam_u32   ("wsize",		Opt_wsize),
 	fsparam_string("xprtsec",	Opt_xprtsec),
+	fsparam_s32("cert_serial",	Opt_cert_serial),
+	fsparam_s32("privkey_serial",	Opt_privkey_serial),
 	{}
 };
 
@@ -551,6 +555,32 @@ static int nfs_parse_version_string(struct fs_context *fc,
 	return 0;
 }
 
+#ifdef CONFIG_KEYS
+static int nfs_tls_key_verify(key_serial_t key_id)
+{
+	struct key *key = key_lookup(key_id);
+	int error = 0;
+
+	if (IS_ERR(key)) {
+		pr_err("key id %08x not found\n", key_id);
+		return PTR_ERR(key);
+	}
+	if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
+	    test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
+		pr_err("key id %08x revoked\n", key_id);
+		error = -EKEYREVOKED;
+	}
+
+	key_put(key);
+	return error;
+}
+#else
+static inline int nfs_tls_key_verify(key_serial_t key_id)
+{
+	return -ENOENT;
+}
+#endif /* CONFIG_KEYS */
+
 /*
  * Parse a single mount parameter.
  */
@@ -807,6 +837,18 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		if (ret < 0)
 			return ret;
 		break;
+	case Opt_cert_serial:
+		ret = nfs_tls_key_verify(result.int_32);
+		if (ret < 0)
+			return ret;
+		ctx->xprtsec.cert_serial = result.int_32;
+		break;
+	case Opt_privkey_serial:
+		ret = nfs_tls_key_verify(result.int_32);
+		if (ret < 0)
+			return ret;
+		ctx->xprtsec.privkey_serial = result.int_32;
+		break;
 
 	case Opt_proto:
 		if (!param->string)
-- 
2.47.2

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Jarkko Sakkinen]

On Thu, May 15, 2025 at 01:50:55PM +0200, Christoph Hellwig wrote:
> Allow tlshd to use a per-mount key from the kernel keyring similar
> to NVMe over TCP.
> 
> Note that tlshd expects keys and certificates stored in the kernel
> keyring to be in DER format, not the PEM format used for file based keys
> and certificates, so they need to be converted before they are added
> to the keyring, which is a bit unexpected.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/nfs/fs_context.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 13f71ca8c974..9e94d18448ff 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -96,6 +96,8 @@ enum nfs_param {
>  	Opt_wsize,
>  	Opt_write,
>  	Opt_xprtsec,
> +	Opt_cert_serial,
> +	Opt_privkey_serial,
>  };
>  
>  enum {
> @@ -221,6 +223,8 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
>  	fsparam_enum  ("write",		Opt_write, nfs_param_enums_write),
>  	fsparam_u32   ("wsize",		Opt_wsize),
>  	fsparam_string("xprtsec",	Opt_xprtsec),
> +	fsparam_s32("cert_serial",	Opt_cert_serial),
> +	fsparam_s32("privkey_serial",	Opt_privkey_serial),
>  	{}
>  };
>  
> @@ -551,6 +555,32 @@ static int nfs_parse_version_string(struct fs_context *fc,
>  	return 0;
>  }
>  
> +#ifdef CONFIG_KEYS
> +static int nfs_tls_key_verify(key_serial_t key_id)
> +{
> +	struct key *key = key_lookup(key_id);
> +	int error = 0;
> +
> +	if (IS_ERR(key)) {
> +		pr_err("key id %08x not found\n", key_id);
> +		return PTR_ERR(key);
> +	}
> +	if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
> +	    test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
> +		pr_err("key id %08x revoked\n", key_id);
> +		error = -EKEYREVOKED;
> +	}
> +
> +	key_put(key);
> +	return error;
> +}

This is equivalent nvme_tls_key_lookup() so would it be more senseful
to call it nfs_tls_key_lookup()? I'm also a bit puzzled how the code
will associate nfs_keyring to all this (e.g., with keyring_search as
done in nvme_tls_psk_lookup())?

> +#else
> +static inline int nfs_tls_key_verify(key_serial_t key_id)
> +{
> +	return -ENOENT;
> +}
> +#endif /* CONFIG_KEYS */
> +
>  /*
>   * Parse a single mount parameter.
>   */
> @@ -807,6 +837,18 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
>  		if (ret < 0)
>  			return ret;
>  		break;
> +	case Opt_cert_serial:
> +		ret = nfs_tls_key_verify(result.int_32);
> +		if (ret < 0)
> +			return ret;
> +		ctx->xprtsec.cert_serial = result.int_32;
> +		break;
> +	case Opt_privkey_serial:
> +		ret = nfs_tls_key_verify(result.int_32);
> +		if (ret < 0)
> +			return ret;
> +		ctx->xprtsec.privkey_serial = result.int_32;
> +		break;
>  
>  	case Opt_proto:
>  		if (!param->string)
> -- 
> 2.47.2
> 

I get the change i.e., keep keys opaque, and it is a reasonable goal.
However, the keyring-key association is where I get lost, so if you
could help me out with that a bit, we could make progress :-)

BR, Jarkko

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Hannes Reinecke]

On 5/15/25 14:51, Jarkko Sakkinen wrote:
> On Thu, May 15, 2025 at 01:50:55PM +0200, Christoph Hellwig wrote:
>> Allow tlshd to use a per-mount key from the kernel keyring similar
>> to NVMe over TCP.
>>
>> Note that tlshd expects keys and certificates stored in the kernel
>> keyring to be in DER format, not the PEM format used for file based keys
>> and certificates, so they need to be converted before they are added
>> to the keyring, which is a bit unexpected.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>   fs/nfs/fs_context.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 42 insertions(+)
>>
>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
>> index 13f71ca8c974..9e94d18448ff 100644
>> --- a/fs/nfs/fs_context.c
>> +++ b/fs/nfs/fs_context.c
>> @@ -96,6 +96,8 @@ enum nfs_param {
>>   	Opt_wsize,
>>   	Opt_write,
>>   	Opt_xprtsec,
>> +	Opt_cert_serial,
>> +	Opt_privkey_serial,
>>   };
>>   
>>   enum {
>> @@ -221,6 +223,8 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
>>   	fsparam_enum  ("write",		Opt_write, nfs_param_enums_write),
>>   	fsparam_u32   ("wsize",		Opt_wsize),
>>   	fsparam_string("xprtsec",	Opt_xprtsec),
>> +	fsparam_s32("cert_serial",	Opt_cert_serial),
>> +	fsparam_s32("privkey_serial",	Opt_privkey_serial),
>>   	{}
>>   };
>>   
>> @@ -551,6 +555,32 @@ static int nfs_parse_version_string(struct fs_context *fc,
>>   	return 0;
>>   }
>>   
>> +#ifdef CONFIG_KEYS
>> +static int nfs_tls_key_verify(key_serial_t key_id)
>> +{
>> +	struct key *key = key_lookup(key_id);
>> +	int error = 0;
>> +
>> +	if (IS_ERR(key)) {
>> +		pr_err("key id %08x not found\n", key_id);
>> +		return PTR_ERR(key);
>> +	}
>> +	if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
>> +	    test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
>> +		pr_err("key id %08x revoked\n", key_id);
>> +		error = -EKEYREVOKED;
>> +	}
>> +
>> +	key_put(key);
>> +	return error;
>> +}
> 
> This is equivalent nvme_tls_key_lookup() so would it be more senseful
> to call it nfs_tls_key_lookup()? I'm also a bit puzzled how the code
> will associate nfs_keyring to all this (e.g., with keyring_search as
> done in nvme_tls_psk_lookup())?
> 
With this patch the keyring is pretty much immaterial; the interface
is passing in a serial number which is unique across all keyrings.
Where the keyring comes in when looking up keys on the TLS server,
as there the TLS client hello only transports the key description
(which are not required to be unique across all keyrings).
So there we'll need the keyring to be specified.
But for the client we really don't.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Christoph Hellwig]

On Thu, May 15, 2025 at 04:46:43PM +0200, Hannes Reinecke wrote:
> With this patch the keyring is pretty much immaterial; the interface
> is passing in a serial number which is unique across all keyrings.
> Where the keyring comes in when looking up keys on the TLS server,
> as there the TLS client hello only transports the key description
> (which are not required to be unique across all keyrings).
> So there we'll need the keyring to be specified.
> But for the client we really don't.

Yes.  Patch 1 on it's own actually works fine-ish.  The big difference
is that the keys would have to be made user-readable as without the
keyring, tlshd would not be the possesor of the key.

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Jarkko Sakkinen]

On Thu, May 15, 2025 at 04:46:43PM +0200, Hannes Reinecke wrote:
> > This is equivalent nvme_tls_key_lookup() so would it be more senseful
> > to call it nfs_tls_key_lookup()? I'm also a bit puzzled how the code
> > will associate nfs_keyring to all this (e.g., with keyring_search as
> > done in nvme_tls_psk_lookup())?
> > 
> With this patch the keyring is pretty much immaterial; the interface
> is passing in a serial number which is unique across all keyrings.
> Where the keyring comes in when looking up keys on the TLS server,
> as there the TLS client hello only transports the key description
> (which are not required to be unique across all keyrings).
> So there we'll need the keyring to be specified.
> But for the client we really don't.

I did not see anything that would depend on having 2/2 at all, or
it getting populated.

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke                  Kernel Storage Architect
> hare@suse.de                                +49 911 74053 688
> SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

BR, Jarkko