diff mbox series

Print sec=<type> in /proc/mounts if not set from cmdline.

Message ID 20200602143205.23854-1-kdsouza@redhat.com (mailing list archive)
State New, archived
Headers show
Series Print sec=<type> in /proc/mounts if not set from cmdline. | expand

Commit Message

Kenneth Dsouza June 2, 2020, 2:32 p.m. UTC 
Currently the end user is unaware with what sec type the
cifs share is mounted if no sec=<type> option is parsed.

Example:
$ echo 0x3 > /proc/fs/cifs/SecurityFlags

Mount a cifs share with vers=1.0, it should pick sec=ntlm.
If mount is successful we print sec=ntlm

//smb-server/data /cifs cifs rw,relatime,vers=1.0,sec=ntlm,cache=strict,username=testuser,uid=0,noforceuid,gid=0,noforcegid,addr=x.x.x.x,file_mode=0755,dir_mode=0755,soft,nounix,serverino,mapposix,rsize=61440,wsize=16580,bsize=1048576,echo_interval=60,actimeo=1

Signed-off-by: Kenneth D'souza <kdsouza@redhat.com>
Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
---
 fs/cifs/sess.c    | 1 +
 fs/cifs/smb2pdu.c | 1 +
 2 files changed, 2 insertions(+)

Comments

Steve French June 3, 2020, 5:09 a.m. UTC | #1 
This is a good point. Iit can be helpful to see if we defaulted to
krb5 or ntlmssp in a particular mount - but printing sectype in mount
options when it is "Default" (ie unspecified) may be confusing.

I prefer that you change this to simply print KRB5 or NTLMSSP in
/proc/fs/cifs/DebugData so we know what is negotiated.
We already include whether "signed" or "sealed" (encrypted) and
whether "guest" or whether "anonymous" etc. so could add whether
NTLMSSP or KRB5 around line 368 of cifs_debug.c
e.g. after
                                if (ses->session_flags &
SMB2_SESSION_FLAG_IS_GUEST)
                                        seq_printf(m, "Guest\t");
                                else if (ses->session_flags &
SMB2_SESSION_FLAG_IS_NULL)
                                        seq_printf(m, "Anonymous\t");

Probably easiest to check the values of server->sec_mskerberos vs.
server->sec_kerberosu2u vs. server->sec_ntlmssp and print it to
/proc/fs/cifs/DebugData

It is more helpful to show in /proc/mounts what we actually passed in
on mount (or the default if not passed in in some cases can be shown)
- while /proc/fs/cifs/DebugData shows more information about what was
negotiated (not just encryption and signing but number of credits
etc.) - seems better to put this in /proc/fs/cifs/DebugData

On Tue, Jun 2, 2020 at 9:32 AM Kenneth D'souza <kdsouza@redhat.com> wrote:
>
> Currently the end user is unaware with what sec type the
> cifs share is mounted if no sec=<type> option is parsed.
>
> Example:
> $ echo 0x3 > /proc/fs/cifs/SecurityFlags
>
> Mount a cifs share with vers=1.0, it should pick sec=ntlm.
> If mount is successful we print sec=ntlm
>
> //smb-server/data /cifs cifs rw,relatime,vers=1.0,sec=ntlm,cache=strict,username=testuser,uid=0,noforceuid,gid=0,noforcegid,addr=x.x.x.x,file_mode=0755,dir_mode=0755,soft,nounix,serverino,mapposix,rsize=61440,wsize=16580,bsize=1048576,echo_interval=60,actimeo=1
>
> Signed-off-by: Kenneth D'souza <kdsouza@redhat.com>
> Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> ---
>  fs/cifs/sess.c    | 1 +
>  fs/cifs/smb2pdu.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 43a88e26d26b..a5673fcab2f7 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -1698,6 +1698,7 @@ static int select_sec(struct cifs_ses *ses, struct sess_data *sess_data)
>                 return -ENOSYS;
>         }
>
> +       ses->sectype = type;
>         return 0;
>  }
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index b30aa3cdd845..bce8a0137fa4 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1601,6 +1601,7 @@ SMB2_select_sec(struct cifs_ses *ses, struct SMB2_sess_data *sess_data)
>                 return -EOPNOTSUPP;
>         }
>
> +       ses->sectype = type;
>         return 0;
>  }
>
> --
> 2.21.1
>
Kenneth Dsouza June 4, 2020, 3:43 p.m. UTC | #2 
Will send out a v2 patch to print Security type in /proc/fs/cifs/DebugData
This would help end users to know with what security type the share is mounted.

Example:
# cat /proc/fs/cifs/DebugData | grep -w "Security type"
1) Name: x.x.x.x Uses: 1 Capability: 0x8001f3fc    Session Status: 1
Security type: RawNTLMSSP

On Wed, Jun 3, 2020 at 10:40 AM Steve French <smfrench@gmail.com> wrote:
>
> This is a good point. Iit can be helpful to see if we defaulted to
> krb5 or ntlmssp in a particular mount - but printing sectype in mount
> options when it is "Default" (ie unspecified) may be confusing.
>
> I prefer that you change this to simply print KRB5 or NTLMSSP in
> /proc/fs/cifs/DebugData so we know what is negotiated.
> We already include whether "signed" or "sealed" (encrypted) and
> whether "guest" or whether "anonymous" etc. so could add whether
> NTLMSSP or KRB5 around line 368 of cifs_debug.c
> e.g. after
>                                 if (ses->session_flags &
> SMB2_SESSION_FLAG_IS_GUEST)
>                                         seq_printf(m, "Guest\t");
>                                 else if (ses->session_flags &
> SMB2_SESSION_FLAG_IS_NULL)
>                                         seq_printf(m, "Anonymous\t");
>
> Probably easiest to check the values of server->sec_mskerberos vs.
> server->sec_kerberosu2u vs. server->sec_ntlmssp and print it to
> /proc/fs/cifs/DebugData
>
> It is more helpful to show in /proc/mounts what we actually passed in
> on mount (or the default if not passed in in some cases can be shown)
> - while /proc/fs/cifs/DebugData shows more information about what was
> negotiated (not just encryption and signing but number of credits
> etc.) - seems better to put this in /proc/fs/cifs/DebugData
>
> On Tue, Jun 2, 2020 at 9:32 AM Kenneth D'souza <kdsouza@redhat.com> wrote:
> >
> > Currently the end user is unaware with what sec type the
> > cifs share is mounted if no sec=<type> option is parsed.
> >
> > Example:
> > $ echo 0x3 > /proc/fs/cifs/SecurityFlags
> >
> > Mount a cifs share with vers=1.0, it should pick sec=ntlm.
> > If mount is successful we print sec=ntlm
> >
> > //smb-server/data /cifs cifs rw,relatime,vers=1.0,sec=ntlm,cache=strict,username=testuser,uid=0,noforceuid,gid=0,noforcegid,addr=x.x.x.x,file_mode=0755,dir_mode=0755,soft,nounix,serverino,mapposix,rsize=61440,wsize=16580,bsize=1048576,echo_interval=60,actimeo=1
> >
> > Signed-off-by: Kenneth D'souza <kdsouza@redhat.com>
> > Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> > ---
> >  fs/cifs/sess.c    | 1 +
> >  fs/cifs/smb2pdu.c | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> > index 43a88e26d26b..a5673fcab2f7 100644
> > --- a/fs/cifs/sess.c
> > +++ b/fs/cifs/sess.c
> > @@ -1698,6 +1698,7 @@ static int select_sec(struct cifs_ses *ses, struct sess_data *sess_data)
> >                 return -ENOSYS;
> >         }
> >
> > +       ses->sectype = type;
> >         return 0;
> >  }
> >
> > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> > index b30aa3cdd845..bce8a0137fa4 100644
> > --- a/fs/cifs/smb2pdu.c
> > +++ b/fs/cifs/smb2pdu.c
> > @@ -1601,6 +1601,7 @@ SMB2_select_sec(struct cifs_ses *ses, struct SMB2_sess_data *sess_data)
> >                 return -EOPNOTSUPP;
> >         }
> >
> > +       ses->sectype = type;
> >         return 0;
> >  }
> >
> > --
> > 2.21.1
> >
>
>
> --
> Thanks,
>
> Steve
>
diff mbox series

Patch

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 43a88e26d26b..a5673fcab2f7 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -1698,6 +1698,7 @@  static int select_sec(struct cifs_ses *ses, struct sess_data *sess_data)
 		return -ENOSYS;
 	}
 
+	ses->sectype = type;
 	return 0;
 }
 
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index b30aa3cdd845..bce8a0137fa4 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1601,6 +1601,7 @@  SMB2_select_sec(struct cifs_ses *ses, struct SMB2_sess_data *sess_data)
 		return -EOPNOTSUPP;
 	}
 
+	ses->sectype = type;
 	return 0;
 }