Thanks for the review, Paulo. I have attached the updated patch. I will send the password rotation changes for DFS, SMB1 (and multiuser) later, separately. Best Meetakshi On Wed, Nov 13, 2024 at 3:30 PM 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.
Attachment:
0001-cifs-during-remount-make-sure-passwords-are-in-sync.patch
Description: Binary data
