On Wed, 16 Feb 2011 09:27:28 -0600
Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
> On Wed, Feb 16, 2011 at 9:21 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Wed, 16 Feb 2011 09:06:08 -0600
> > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
> >
> >> On Wed, Feb 16, 2011 at 9:01 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> >> > On Wed, 16 Feb 2011 08:46:03 -0600
> >> > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
> >> >
> >> >> On Wed, Feb 16, 2011 at 6:53 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> >> >> > On Tue, 15 Feb 2011 17:10:43 -0600
> >> >> > shirishpargaonkar@xxxxxxxxx wrote:
> >> >> >
> >> >> >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >> >> >>
> >> >> >>
> >> >> >> Fix lanman (lm) authentication code.
> >> >> >>
> >> >> >> Change lm response length back to 24 from 16.
> >> >> >> Parse lanmani mount option.
> >> >> >> Add code to add odd parity bit to each of the eight bytes of a DES key.
> >> >> >>
> >> >> >>
> >> >> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >> >> >> ---
> >> >> >> fs/cifs/cifsglob.h | 3 ++-
> >> >> >> fs/cifs/connect.c | 3 +++
> >> >> >> fs/cifs/sess.c | 8 ++++----
> >> >> >> fs/cifs/smbdes.c | 19 ++++++++++++++++++-
> >> >> >> 4 files changed, 27 insertions(+), 6 deletions(-)
> >> >> >>
> >> >> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> >> >> index 17afb0f..0b5c950 100644
> >> >> >> --- a/fs/cifs/cifsglob.h
> >> >> >> +++ b/fs/cifs/cifsglob.h
> >> >> >> @@ -710,7 +710,8 @@ require use of the stronger protocol */
> >> >> >> #define CIFSSEC_MUST_SEAL 0x40040 /* not supported yet */
> >> >> >> #define CIFSSEC_MUST_NTLMSSP 0x80080 /* raw ntlmssp with ntlmv2 */
> >> >> >>
> >> >> >> -#define CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLM | CIFSSEC_MAY_NTLMV2)
> >> >> >> +#define CIFSSEC_DEF (CIFSSEC_MAY_LANMAN | CIFSSEC_MAY_SIGN | \
> >> >> >> + CIFSSEC_MAY_NTLM | CIFSSEC_MAY_NTLMV2)
> >> >> > ^^^^^^^^^^^
> >> >> > Won't this change enable lanman auth by default? Is that
> >> >> > intended?
> >> >>
> >> >> I think it depends on what server sends in negprot response, the
> >> >> minimun dialect it supports.
> >> >> If the dialect is greater than Lanman2.1, the default auth mech for
> >> >> cifs client is NTLM (ntlmv1)
> >> >> if the dialect is less than or upto Lanman2.1, default auth mech for
> >> >> cifs client is LANMAN (lm).
> >> >>
> >> >
> >> > Historically we've required that the admin
> >> > set /proc/fs/cifs/SecurityFlags to allow LANMAN auth before the client
> >> > will allow it to be used. I'm not opposed to changing that, but the
> >> > description doesn't even mention anything about that.
> >> >
> >> > I think this ought to be a separate patch with a clearly described
> >> > reason, and probably doesn't belong in 2.6.38. It doesn't seem like
> >> > it's necessary to fix this bug.
> >> >
> >> > --
> >> > Jeff Layton <jlayton@xxxxxxxxxx>
> >> >
> >>
> >> I think that is still the case. If we do not have the code, we will get error
> >> CIFS VFS: mount failed weak security disabled in /proc/fs/cifs/SecurityFlags
> >> and auth will fail even if admin did necessary to allow LANMAN.
> >
> > For the record, I'm of the opinion that this secFlags stuff is far too
> > convoluted for anyone's good. It's horribly confusing. I don't think
> > what you're saying is the case however:
> >
> > ...global_secflags is initialized to CIFSSEC_DEF:
> >
> > unsigned int global_secflags = CIFSSEC_DEF;
> >
> > ...then, if ses->overrideSecFlg is not set (which is the case if no
> > sec= option is set):
> >
> > /* if any of auth flags (ie not sign or seal) are overriden use them */
> > if (ses->overrideSecFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
> > secFlags = ses->overrideSecFlg; /* BB FIXME fix sign flags? */
> > else /* if override flags set only sign/seal OR them with global auth */
> > secFlags = global_secflags | ses->overrideSecFlg;
> >
> >
> > ...now, secFlags ends up being global_secflags plus some optional
> > sign/seal flags. Then later:
> >
> > if ((secFlags & CIFSSEC_MAY_LANMAN) ||
> > (secFlags & CIFSSEC_MAY_PLNTXT))
> > server->secType = LANMAN;
> > else {
> > cERROR(1, "mount failed weak security disabled"
> > " in /proc/fs/cifs/SecurityFlags");
> > rc = -EOPNOTSUPP;
> > goto neg_err_exit;
> > }
> >
> > ...so, because CIFSSEC_MAY_LANMAN is now set by default, we'll not fall
> > into the else condition above and the mount will work by default when
> > it would have failed before, right? So the default behavior has
> > changed...or am I missing something?
> >
> > --
> > Jeff Layton <jlayton@xxxxxxxxxx>
> >
>
> Jeff, I am saying I will undo the change in cifsglob.h that is
> submitted in this patch.
> With definition of CIFSSEC_DEF unchagned, all I will be fixing is
> allowing lanmani
> mount option and proper lm authentication (if admin allow lm auth in
> /proc/fs/cifs/SecurityFlags.
> So I would not be changing default behaviour as it exists right now.
Sorry about that -- our earlier replies crossed paths. I think that
sounds like the best thing to do.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
