Skip to content

Commit

Permalink
smb3: fix broken reconnect when password changing on the server by al…
Browse files Browse the repository at this point in the history
…lowing password rotation

There are various use cases that are becoming more common in which password
changes are scheduled on a server(s) periodically but the clients connected
to this server need to stay connected (even in the face of brief network
reconnects) due to mounts which can not be easily unmounted and mounted at
will, and servers that do password rotation do not always have the ability
to tell the clients exactly when to the new password will be effective,
so add support for an alt password ("password2=") on mount (and also
remount) so that we can anticipate the upcoming change to the server
without risking breaking existing mounts.

An alternative would have been to use the kernel keyring for this but the
processes doing the reconnect do not have access to the keyring but do
have access to the ses structure.

Reviewed-by: Bharath SM <bharathsm@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
  • Loading branch information
Steve French committed Apr 11, 2024
1 parent c6ff459 commit 35f8342
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 0 deletions.
1 change: 1 addition & 0 deletions fs/smb/client/cifsglob.h
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,7 @@ struct cifs_ses {
and after mount option parsing we fill it */
char *domainName;
char *password;
char *password2; /* When key rotation used, new password may be set before it expires */
char workstation_name[CIFS_MAX_WORKSTATION_LEN];
struct session_key auth_key;
struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
Expand Down
8 changes: 8 additions & 0 deletions fs/smb/client/connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -2183,6 +2183,7 @@ cifs_set_cifscreds(struct smb3_fs_context *ctx, struct cifs_ses *ses)
}

++delim;
/* BB consider adding support for password2 (Key Rotation) for multiuser in future */
ctx->password = kstrndup(delim, len, GFP_KERNEL);
if (!ctx->password) {
cifs_dbg(FYI, "Unable to allocate %zd bytes for password\n",
Expand All @@ -2206,6 +2207,7 @@ cifs_set_cifscreds(struct smb3_fs_context *ctx, struct cifs_ses *ses)
kfree(ctx->username);
ctx->username = NULL;
kfree_sensitive(ctx->password);
/* no need to free ctx->password2 since not allocated in this path */
ctx->password = NULL;
goto out_key_put;
}
Expand Down Expand Up @@ -2317,6 +2319,12 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
if (!ses->password)
goto get_ses_fail;
}
/* ctx->password freed at unmount */
if (ctx->password2) {
ses->password2 = kstrdup(ctx->password2, GFP_KERNEL);
if (!ses->password2)
goto get_ses_fail;
}
if (ctx->domainname) {
ses->domainName = kstrdup(ctx->domainname, GFP_KERNEL);
if (!ses->domainName)
Expand Down
21 changes: 21 additions & 0 deletions fs/smb/client/fs_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ const struct fs_parameter_spec smb3_fs_parameters[] = {
fsparam_string("username", Opt_user),
fsparam_string("pass", Opt_pass),
fsparam_string("password", Opt_pass),
fsparam_string("password2", Opt_pass2),
fsparam_string("ip", Opt_ip),
fsparam_string("addr", Opt_ip),
fsparam_string("domain", Opt_domain),
Expand Down Expand Up @@ -345,6 +346,7 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx
new_ctx->nodename = NULL;
new_ctx->username = NULL;
new_ctx->password = NULL;
new_ctx->password2 = NULL;
new_ctx->server_hostname = NULL;
new_ctx->domainname = NULL;
new_ctx->UNC = NULL;
Expand All @@ -357,6 +359,7 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx
DUP_CTX_STR(prepath);
DUP_CTX_STR(username);
DUP_CTX_STR(password);
DUP_CTX_STR(password2);
DUP_CTX_STR(server_hostname);
DUP_CTX_STR(UNC);
DUP_CTX_STR(source);
Expand Down Expand Up @@ -905,6 +908,8 @@ static int smb3_reconfigure(struct fs_context *fc)
else {
kfree_sensitive(ses->password);
ses->password = kstrdup(ctx->password, GFP_KERNEL);
kfree_sensitive(ses->password2);
ses->password2 = kstrdup(ctx->password2, GFP_KERNEL);
}
STEAL_STRING(cifs_sb, ctx, domainname);
STEAL_STRING(cifs_sb, ctx, nodename);
Expand Down Expand Up @@ -1305,6 +1310,18 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
goto cifs_parse_mount_err;
}
break;
case Opt_pass2:
kfree_sensitive(ctx->password2);
ctx->password2 = NULL;
if (strlen(param->string) == 0)
break;

ctx->password2 = kstrdup(param->string, GFP_KERNEL);
if (ctx->password2 == NULL) {
cifs_errorf(fc, "OOM when copying password2 string\n");
goto cifs_parse_mount_err;
}
break;
case Opt_ip:
if (strlen(param->string) == 0) {
ctx->got_ip = false;
Expand Down Expand Up @@ -1608,6 +1625,8 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
cifs_parse_mount_err:
kfree_sensitive(ctx->password);
ctx->password = NULL;
kfree_sensitive(ctx->password2);
ctx->password2 = NULL;
return -EINVAL;
}

Expand Down Expand Up @@ -1713,6 +1732,8 @@ smb3_cleanup_fs_context_contents(struct smb3_fs_context *ctx)
ctx->username = NULL;
kfree_sensitive(ctx->password);
ctx->password = NULL;
kfree_sensitive(ctx->password2);
ctx->password2 = NULL;
kfree(ctx->server_hostname);
ctx->server_hostname = NULL;
kfree(ctx->UNC);
Expand Down
2 changes: 2 additions & 0 deletions fs/smb/client/fs_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ enum cifs_param {
Opt_source,
Opt_user,
Opt_pass,
Opt_pass2,
Opt_ip,
Opt_domain,
Opt_srcaddr,
Expand Down Expand Up @@ -177,6 +178,7 @@ struct smb3_fs_context {

char *username;
char *password;
char *password2;
char *domainname;
char *source;
char *server_hostname;
Expand Down
1 change: 1 addition & 0 deletions fs/smb/client/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ sesInfoFree(struct cifs_ses *buf_to_free)
kfree(buf_to_free->serverDomain);
kfree(buf_to_free->serverNOS);
kfree_sensitive(buf_to_free->password);
kfree_sensitive(buf_to_free->password2);
kfree(buf_to_free->user_name);
kfree(buf_to_free->domainName);
kfree_sensitive(buf_to_free->auth_key.response);
Expand Down
11 changes: 11 additions & 0 deletions fs/smb/client/smb2pdu.c
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,17 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
}

rc = cifs_setup_session(0, ses, server, nls_codepage);
if ((rc == -EACCES) || (rc == -EKEYEXPIRED) || (rc == -EKEYREVOKED)) {
/*
* Try alternate password for next reconnect (key rotation
* could be enabled on the server e.g.) if an alternate
* password is available and the current password is expired,
* but do not swap on non pwd related errors like host down
*/
if (ses->password2)
swap(ses->password2, ses->password);
}

if ((rc == -EACCES) && !tcon->retry) {
mutex_unlock(&ses->session_mutex);
rc = -EHOSTDOWN;
Expand Down

0 comments on commit 35f8342

Please sign in to comment.