My opinion is that since password rotation is important for various security scenarios, there is value in improving it for all dialects, especially as the missing piece in the SMB1 reconnect path shouldn't be too hard to fix. On Wed, Nov 13, 2024 at 4:00 AM Meetakshi Setiya <meetakshisetiyaoss@xxxxxxxxx> wrote: > > Typo: password rotation for SMB1.0 is NOT supported for reconnects. It works for fresh > mounts and remounts. > Should I add support for reconnect or remove it completely? > > On Wed, Nov 13, 2024 at 3:20 PM Meetakshi Setiya <meetakshisetiyaoss@xxxxxxxxx> wrote: >> >> Hi Paulo, >> >> Given your and Shyam's comments, I am thinking of making the >> following changes to the code to support password rotation for DFS: >> >> 1. For a fresh mount: >> - In cifs_do_automount, bring the passwords in fs_context in sync with >> the passwords in the session object before sending the context to the >> child/submount. >> >> 2. For a remount (of the root only): >> - In smb3_reconfigure, bring the passwords in the fs_context of the master >> tcon in sync with its session object passwords. After this is done, a new >> function will be called to iterate over the dfs_ses_list held in this tcon >> and sync their session passwords with the updated root session password >> and password2. >> >> Password rotation for multiuser mounts is out of scope for this patch and I will >> address it later. >> >> Please let me know if you have any comments or suggestions on this approach. >> >> Also, we share the mount code path (cifs_mount) for all SMB versions. So, password >> rotation for SMB1.0 is currently supported ONLY on mounts. Password rotation on >> remount, however, is not. Should I remove the support completely for SMB1.0 >> (print a warning message), leave it be, or add remount support? > > > reconnect, not remount. > >> >> Thanks >> Meetakshi >> >> On Mon, Nov 11, 2024 at 5:34 PM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote: >>> >>> Shyam Prasad N <nspmangalore@xxxxxxxxx> writes: >>> >>> > On Fri, Nov 8, 2024 at 5:47 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: >>> >> > What about SMB sessions from cifs_tcon::dfs_ses_list? I don't see their >>> >> > password getting updated over remount. >>> >> >>> >> This is in our to-do list as well. >>> > >>> > I did some code reading around how DFS automount works. >>> > @Paulo Alcantara Correct me if I'm wrong, but it sounds like we make >>> > an assumption that when a DFS namespace has a junction to another >>> > share, the same credentials are to be used to perform the mount of >>> > that share. Is that always the case? >>> >>> Yes, it inherits fs_context from the parent mount. For multiuser >>> mounts, when uid/gid/cruid are unspecified, we need to update its values >>> to match real uid/gid from the calling process. >>> >>> > If we go by that assumption, for password2 to work with DFS mounts, we >>> > only need to make sure that in cifs_do_automount, cur_ctx passwords >>> > are synced up to the current ses passwords. That should be quite easy. >>> >>> Correct. The fs_context for the automount is dup'ed from the parent >>> mount. smb3_fs_context_dup() already dups password2, so it should work. >>> >>> The 'remount' case isn't still handled, that's why I mentioned it above. >>> You'd need to set password2 for all sessios in @tcon->dfs_ses_list. >>> >>> I think we need to update password2 for the multiuser sessions as well >>> and not only for session from master tcon. -- Thanks, Steve
