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.
--
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
