Updated the patch to allow remount to only change the password if
disconnected and the session fails to reconnect due to continued
access denied or password expired errors.
Any thoughts on whether I should add another patch to throttle
repeated session setups after access denied or key expired (currently
looks like repeated every few seconds) maybe double the time
repeatedly (e.g. until a max of 10 or 20 or 30 seconds? between
reconnect attempts) instead of every two seconds.
On Fri, Feb 16, 2024 at 8:41 AM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>
> Shyam Prasad N <nspmangalore@xxxxxxxxx> writes:
>
> > need_recon would also be true in other cases, for example when the
> > network is temporarily disconnected. This patch will allow changing of
> > password even then.
> > We could setup a special flag when the server returns a
> > STATUS_LOGON_FAILURE for SessionSetup. We can make the check for that
> > flag and then allow password change on remount.
>
> Yes. Allowing password change over remount simply because network is
> disconnected is not a good idea. The user could mistype the password
> when performing a remount and then everything would stop working.
>
> Not to mention that this patch is only handling a specfic case where a
> mount would have a single SMB session, which isn't true for a DFS mount.
>
> > Another option is to extend the multiuser keyring mechanism for single
> > user use case as well, and use that for password update.
> > Ideally, we should be able to setup multiple passwords in that keyring
> > and iterate through them once to see if SessionSetup goes through.
>
> Yes, sounds like the best approach so far. It would allow users to
> update their passwords in keyring and sysadmins could drop existing SMB
> sessions from server side and then the client would reconnect by using
> new password from keyring. This wouldn't even require a remount.
>
> Besides, marking this for -stable makes no sense.
--
Thanks,
Steve
From 0ab0a5fed47634cef2a49b80a35197880aa96658 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 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 | 23 ++++++++++++++++++-----
fs/smb/client/smb2pdu.c | 5 +++++
4 files changed, 26 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..99702ab05f8d 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,11 @@ 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;
+ }
}
if (new_ctx->domainname &&
(!old_ctx->domainname || strcmp(new_ctx->domainname, old_ctx->domainname))) {
@@ -843,9 +846,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 +866,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
