Skip to content

Commit

Permalink
nfsd: new Kconfig option for legacy client tracking
Browse files Browse the repository at this point in the history 
We've had a number of attempts at different NFSv4 client tracking
methods over the years, but now nfsdcld has emerged as the clear winner
since the others (recoverydir and the usermodehelper upcall) are
problematic.

As a case in point, the recoverydir backend uses MD5 hashes to encode
long form clientid strings, which means that nfsd repeatedly gets dinged
on FIPS audits, since MD5 isn't considered secure. Its use of MD5 is not
cryptographically significant, so there is no danger there, but allowing
us to compile that out allows us to sidestep the issue entirely.

As a prelude to eventually removing support for these client tracking
methods, add a new Kconfig option that enables them. Mark it deprecated
and make it default to N.

Acked-by: NeilBrown <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
  • Loading branch information
@jtlayton @chucklever
jtlayton authored and chucklever committed Jan 7, 2024
1 parent 0dd3ee3 commit 74fd487
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 34 deletions.
16 changes: 16 additions & 0 deletions fs/nfsd/Kconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,19 @@ config NFSD_V4_SECURITY_LABEL

If you do not wish to enable fine-grained security labels SELinux or
Smack policies on NFSv4 files, say N.

config NFSD_LEGACY_CLIENT_TRACKING
bool "Support legacy NFSv4 client tracking methods (DEPRECATED)"
depends on NFSD_V4
default n
help
The NFSv4 server needs to store a small amount of information on
stable storage in order to handle state recovery after reboot. Most
modern deployments upcall to a userland daemon for this (nfsdcld),
but older NFS servers may store information directly in a
recoverydir, or spawn a process directly using a usermodehelper
upcall.

These legacy client tracking methods have proven to be probelmatic
and will be removed in the future. Say Y here if you need support
for them in the interim.
97 changes: 63 additions & 34 deletions fs/nfsd/nfs4recover.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ struct nfsd4_client_tracking_ops {
static const struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops;
static const struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops_v2;

#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
/* Globals */
static char user_recovery_dirname[PATH_MAX] = "/var/lib/nfs/v4recovery";

Expand Down Expand Up @@ -720,6 +721,7 @@ static const struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
.version = 1,
.msglen = 0,
};
#endif /* CONFIG_NFSD_LEGACY_CLIENT_TRACKING */

/* Globals */
#define NFSD_PIPE_DIR "nfsd"
Expand All @@ -731,8 +733,10 @@ struct cld_net {
spinlock_t cn_lock;
struct list_head cn_list;
unsigned int cn_xid;
bool cn_has_legacy;
struct crypto_shash *cn_tfm;
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
bool cn_has_legacy;
#endif
};

struct cld_upcall {
Expand Down Expand Up @@ -793,7 +797,6 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
uint8_t cmd, princhashlen;
struct xdr_netobj name, princhash = { .len = 0, .data = NULL };
uint16_t namelen;
struct cld_net *cn = nn->cld_net;

if (get_user(cmd, &cmsg->cm_cmd)) {
dprintk("%s: error when copying cmd from userspace", __func__);
Expand Down Expand Up @@ -833,11 +836,15 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
return PTR_ERR(name.data);
name.len = namelen;
}
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
if (name.len > 5 && memcmp(name.data, "hash:", 5) == 0) {
struct cld_net *cn = nn->cld_net;

name.len = name.len - 5;
memmove(name.data, name.data + 5, name.len);
cn->cn_has_legacy = true;
}
#endif
if (!nfs4_client_to_reclaim(name, princhash, nn)) {
kfree(name.data);
kfree(princhash.data);
Expand Down Expand Up @@ -1010,7 +1017,9 @@ __nfsd4_init_cld_pipe(struct net *net)
}

cn->cn_pipe->dentry = dentry;
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
cn->cn_has_legacy = false;
#endif
nn->cld_net = cn;
return 0;

Expand Down Expand Up @@ -1282,10 +1291,6 @@ nfsd4_cld_check(struct nfs4_client *clp)
{
struct nfs4_client_reclaim *crp;
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
struct cld_net *cn = nn->cld_net;
int status;
char dname[HEXDIR_LEN];
struct xdr_netobj name;

/* did we already find that this client is stable? */
if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
Expand All @@ -1296,7 +1301,12 @@ nfsd4_cld_check(struct nfs4_client *clp)
if (crp)
goto found;

if (cn->cn_has_legacy) {
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
if (nn->cld_net->cn_has_legacy) {
int status;
char dname[HEXDIR_LEN];
struct xdr_netobj name;

status = nfs4_make_rec_clidname(dname, &clp->cl_name);
if (status)
return -ENOENT;
Expand All @@ -1314,6 +1324,7 @@ nfsd4_cld_check(struct nfs4_client *clp)
goto found;

}
#endif
return -ENOENT;
found:
crp->cr_clp = clp;
Expand All @@ -1327,8 +1338,6 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
struct cld_net *cn = nn->cld_net;
int status;
char dname[HEXDIR_LEN];
struct xdr_netobj name;
struct crypto_shash *tfm = cn->cn_tfm;
struct xdr_netobj cksum;
char *principal = NULL;
Expand All @@ -1342,7 +1351,11 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
if (crp)
goto found;

#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
if (cn->cn_has_legacy) {
struct xdr_netobj name;
char dname[HEXDIR_LEN];

status = nfs4_make_rec_clidname(dname, &clp->cl_name);
if (status)
return -ENOENT;
Expand All @@ -1360,6 +1373,7 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
goto found;

}
#endif
return -ENOENT;
found:
if (crp->cr_princhash.len) {
Expand Down Expand Up @@ -1663,6 +1677,7 @@ static const struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops_v2 = {
.msglen = sizeof(struct cld_msg_v2),
};

#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
/* upcall via usermodehelper */
static char cltrack_prog[PATH_MAX] = "/sbin/nfsdcltrack";
module_param_string(cltrack_prog, cltrack_prog, sizeof(cltrack_prog),
Expand Down Expand Up @@ -2007,28 +2022,10 @@ static const struct nfsd4_client_tracking_ops nfsd4_umh_tracking_ops = {
.msglen = 0,
};

int
nfsd4_client_tracking_init(struct net *net)
static inline int check_for_legacy_methods(int status, struct net *net)
{
int status;
struct path path;
struct nfsd_net *nn = net_generic(net, nfsd_net_id);

/* just run the init if it the method is already decided */
if (nn->client_tracking_ops)
goto do_init;

/* First, try to use nfsdcld */
nn->client_tracking_ops = &nfsd4_cld_tracking_ops;
status = nn->client_tracking_ops->init(net);
if (!status)
return status;
if (status != -ETIMEDOUT) {
nn->client_tracking_ops = &nfsd4_cld_tracking_ops_v0;
status = nn->client_tracking_ops->init(net);
if (!status)
return status;
}
struct path path;

/*
* Next, try the UMH upcall.
Expand All @@ -2045,14 +2042,46 @@ nfsd4_client_tracking_init(struct net *net)
nn->client_tracking_ops = &nfsd4_legacy_tracking_ops;
status = kern_path(nfs4_recoverydir(), LOOKUP_FOLLOW, &path);
if (!status) {
status = d_is_dir(path.dentry);
status = !d_is_dir(path.dentry);
path_put(&path);
if (!status) {
status = -EINVAL;
goto out;
}
if (status)
return -ENOTDIR;
status = nn->client_tracking_ops->init(net);
}
return status;
}
#else
static inline int check_for_legacy_methods(int status, struct net *net)
{
return status;
}
#endif /* CONFIG_LEGACY_NFSD_CLIENT_TRACKING */

int
nfsd4_client_tracking_init(struct net *net)
{
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
int status;

/* just run the init if it the method is already decided */
if (nn->client_tracking_ops)
goto do_init;

/* First, try to use nfsdcld */
nn->client_tracking_ops = &nfsd4_cld_tracking_ops;
status = nn->client_tracking_ops->init(net);
if (!status)
return status;
if (status != -ETIMEDOUT) {
nn->client_tracking_ops = &nfsd4_cld_tracking_ops_v0;
status = nn->client_tracking_ops->init(net);
if (!status)
return status;
}

status = check_for_legacy_methods(status, net);
if (status)
goto out;
do_init:
status = nn->client_tracking_ops->init(net);
out:
Expand Down
6 changes: 6 additions & 0 deletions fs/nfsd/nfsctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ static ssize_t write_maxconn(struct file *file, char *buf, size_t size);
#ifdef CONFIG_NFSD_V4
static ssize_t write_leasetime(struct file *file, char *buf, size_t size);
static ssize_t write_gracetime(struct file *file, char *buf, size_t size);
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
static ssize_t write_recoverydir(struct file *file, char *buf, size_t size);
#endif
static ssize_t write_v4_end_grace(struct file *file, char *buf, size_t size);
#endif

Expand All @@ -93,7 +95,9 @@ static ssize_t (*const write_op[])(struct file *, char *, size_t) = {
#ifdef CONFIG_NFSD_V4
[NFSD_Leasetime] = write_leasetime,
[NFSD_Gracetime] = write_gracetime,
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
[NFSD_RecoveryDir] = write_recoverydir,
#endif
[NFSD_V4EndGrace] = write_v4_end_grace,
#endif
};
Expand Down Expand Up @@ -1021,6 +1025,7 @@ static ssize_t write_gracetime(struct file *file, char *buf, size_t size)
return nfsd4_write_time(file, buf, size, &nn->nfsd4_grace, nn);
}

#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
static ssize_t __write_recoverydir(struct file *file, char *buf, size_t size,
struct nfsd_net *nn)
{
Expand Down Expand Up @@ -1081,6 +1086,7 @@ static ssize_t write_recoverydir(struct file *file, char *buf, size_t size)
mutex_unlock(&nfsd_mutex);
return rv;
}
#endif

/*
* write_v4_end_grace - release grace period for nfsd's v4.x lock manager
Expand Down

0 comments on commit 74fd487

Please sign in to comment.