Here is an updated patch which handles the point about preventing remount
from changing it for the sec=krb5 case.
This does look like something important for stable (users have cases where
they can't unmount due to a running app but need to update an expired
password) - are you preferring that we only allow this when user also specifies
a new mount option in addition to "remount" ie "forcenewcreds" (or probably
better would be to call it "newpassword=" mount option)?
On Fri, Feb 23, 2024 at 8:08 AM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>
> Shyam Prasad N <nspmangalore@xxxxxxxxx> writes:
>
> > No major objections for this patch. While it may not cover all cases
> > like DFS, multiuser etc., it's still a starting point to allowing
> > users to change password on existing mounts without unmounting.
>
> As long as it doesn't go through -stable and is accompanied with at
> least a new option like 'forcenewcreds', should be fine. Then you have
> the next merge window to handle the missing cases and fix any problems.
--
Thanks,
Steve
From 78d5c7683f45e55e555516de80e5a17da6ede6c4 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
Date: Tue, 13 Feb 2024 00:40:01 -0600
Subject: [PATCH] cifs: allow changing password during remount
There are cases where a session is disconnected and password has changed
on the server (or expired) for this user and this currently can not
be fixed without unmount and mounting again. This patch allows
remount to change the password (for the non Kerberos case, Kerberos
ticket refresh is handled differently) when the session is disconnected
and the user can not reconnect due to still using old password.
Future patches should also allow us to setup the keyring (cifscreds)
to have an "alternate password" so we would be able to change
the password before the session drops (without the risk of races
between when the password changes and the disconnect occurs -
ie cases where the old password is still needed because the new
password has not fully rolled out to all servers yet).
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/smb/client/cifs_debug.c | 2 ++
fs/smb/client/cifsglob.h | 1 +
fs/smb/client/fs_context.c | 25 ++++++++++++++++++++-----
fs/smb/client/smb2pdu.c | 5 +++++
4 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 3e4209f41c18..23d2622b969f 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -488,6 +488,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
ses->ses_count, ses->serverOS, ses->serverNOS,
ses->capabilities, ses->ses_status);
}
+ if (ses->expired_pwd)
+ seq_puts(m, "password no longer valid ");
spin_unlock(&ses->ses_lock);
seq_printf(m, "\n\tSecurity type: %s ",
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 53c75cfb33ab..ec9a26bd05a1 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1066,6 +1066,7 @@ struct cifs_ses {
enum securityEnum sectype; /* what security flavor was specified? */
bool sign; /* is signing required? */
bool domainAuto:1;
+ bool expired_pwd; /* track if access denied or expired pwd so can know if need to update */
unsigned int flags;
__u16 session_flags;
__u8 smb3signingkey[SMB3_SIGN_KEY_SIZE];
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 4b2f5aa2ea0e..128270e7694f 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -772,7 +772,7 @@ static void smb3_fs_context_free(struct fs_context *fc)
*/
static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
struct smb3_fs_context *new_ctx,
- struct smb3_fs_context *old_ctx)
+ struct smb3_fs_context *old_ctx, bool need_recon)
{
if (new_ctx->posix_paths != old_ctx->posix_paths) {
cifs_errorf(fc, "can not change posixpaths during remount\n");
@@ -798,8 +798,13 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
}
if (new_ctx->password &&
(!old_ctx->password || strcmp(new_ctx->password, old_ctx->password))) {
- cifs_errorf(fc, "can not change password during remount\n");
- return -EINVAL;
+ if (need_recon == false) {
+ cifs_errorf(fc,
+ "can not change password of active session during remount\n");
+ return -EINVAL;
+ } else if (old_ctx->sectype == Kerberos)
+ cifs_errorf(fc,
+ "can not change password for Kerberos via remount\n");
}
if (new_ctx->domainname &&
(!old_ctx->domainname || strcmp(new_ctx->domainname, old_ctx->domainname))) {
@@ -843,9 +848,14 @@ static int smb3_reconfigure(struct fs_context *fc)
struct smb3_fs_context *ctx = smb3_fc2context(fc);
struct dentry *root = fc->root;
struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb);
+ struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses;
+ bool need_recon = false;
int rc;
- rc = smb3_verify_reconfigure_ctx(fc, ctx, cifs_sb->ctx);
+ if (ses->expired_pwd)
+ need_recon = true;
+
+ rc = smb3_verify_reconfigure_ctx(fc, ctx, cifs_sb->ctx, need_recon);
if (rc)
return rc;
@@ -858,7 +868,12 @@ static int smb3_reconfigure(struct fs_context *fc)
STEAL_STRING(cifs_sb, ctx, UNC);
STEAL_STRING(cifs_sb, ctx, source);
STEAL_STRING(cifs_sb, ctx, username);
- STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
+ if (need_recon == false)
+ STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
+ else {
+ kfree_sensitive(ses->password);
+ ses->password = kstrdup(ctx->password, GFP_KERNEL);
+ }
STEAL_STRING(cifs_sb, ctx, domainname);
STEAL_STRING(cifs_sb, ctx, nodename);
STEAL_STRING(cifs_sb, ctx, iocharset);
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 608ee05491e2..a500380d1b2e 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -1536,6 +1536,11 @@ SMB2_sess_sendreceive(struct SMB2_sess_data *sess_data)
&sess_data->buf0_type,
CIFS_LOG_ERROR | CIFS_SESS_OP, &rsp_iov);
cifs_small_buf_release(sess_data->iov[0].iov_base);
+ if (rc == 0)
+ sess_data->ses->expired_pwd = false;
+ else if ((rc == -EACCES) || (rc == -EKEYEXPIRED) || (rc == -EKEYREVOKED))
+ sess_data->ses->expired_pwd = true;
+
memcpy(&sess_data->iov[0], &rsp_iov, sizeof(struct kvec));
return rc;
--
2.40.1
