On Wed, Feb 16, 2011 at 9:27 AM, 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.
Right - we do not want to enable very weak authentication by default.
--
Thanks,
Steve
--
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
