Re: [PATCH] cifs: eliminate CONFIG_CIFS_WEAK_PW_HASH

[Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Fri, 20 Jan 2012 14:45:48 -0600
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> My general thinking on this is as follows:
> 
> If the kernel is distributed to all the workstations in an organization
> with this Kconfig option disabled, it makes it harder for individual users
> to make the mistake of enabling lanman (sec=lanman, or the Kconfig
> option) on a public network and thus send weak password hashes
> which could be discovered simply.   Most distros make the choice
> of enabling broader compatibility with old pre-1997 servers but
> it is a very small set of servers who would require lanman support,
> and a large number of potential attackers who could benefit if
> users enable lanman on a public network.  I suspect that there
> are environments where removing code (via Kconfig) is preferred
> to trusting all owners of all workstations running that organizations
> standard linux to never enable lanman at runtime.
> 
> But ... the opinion of security specialists on this would be welcome.
> 

There are myriads of ways for someone to screw themselves if you give
them root access. This one is pretty low on the list.

Code quality is important for security too, and the writhing mass of
ifdefs below is impossible to parse by eye and therefore debug. At some
point we have to weigh what requires a Kconfig option, and what doesn't.

Given that this can be administratively prohibited at runtime, I see
absolutely no need to keep this Kconfig option around. It just adds a
maintenance burden for no benefit.

While it's well and good to say you want to hear from "security
specialists", I'm not sure what that means and it sounds awfully open
ended. Is a lack of objection sufficient for you to take this patch or
do you need something else?

[...]

> >> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> > index 76e7d8b..0e56b1e 100644
> >> > --- a/fs/cifs/cifsglob.h
> >> > +++ b/fs/cifs/cifsglob.h
> >> > @@ -853,13 +853,8 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
> >> >  #define   CIFSSEC_MAY_NTLM     0x00002
> >> >  #define   CIFSSEC_MAY_NTLMV2   0x00004
> >> >  #define   CIFSSEC_MAY_KRB5     0x00008
> >> > -#ifdef CONFIG_CIFS_WEAK_PW_HASH
> >> >  #define   CIFSSEC_MAY_LANMAN   0x00010
> >> >  #define   CIFSSEC_MAY_PLNTXT   0x00020
> >> > -#else
> >> > -#define   CIFSSEC_MAY_LANMAN    0
> >> > -#define   CIFSSEC_MAY_PLNTXT    0
> >> > -#endif /* weak passwords */
> >> >  #define   CIFSSEC_MAY_SEAL     0x00040 /* not supported yet */
> >> >  #define   CIFSSEC_MAY_NTLMSSP  0x00080 /* raw ntlmssp with ntlmv2 */
> >> >
> >> > @@ -870,23 +865,13 @@ require use of the stronger protocol */
> >> >  #define   CIFSSEC_MUST_NTLM    0x02002
> >> >  #define   CIFSSEC_MUST_NTLMV2  0x04004
> >> >  #define   CIFSSEC_MUST_KRB5    0x08008
> >> > -#ifdef CONFIG_CIFS_WEAK_PW_HASH
> >> >  #define   CIFSSEC_MUST_LANMAN  0x10010
> >> >  #define   CIFSSEC_MUST_PLNTXT  0x20020
> >> >  #ifdef CONFIG_CIFS_UPCALL
> >> >  #define   CIFSSEC_MASK          0xBF0BF /* allows weak security but also krb5 */
> >> > -#else
> >> > +#else /* UPCALL */
> >> >  #define   CIFSSEC_MASK          0xB70B7 /* current flags supported if weak */
> >> >  #endif /* UPCALL */
> >> > -#else /* do not allow weak pw hash */
> >> > -#define   CIFSSEC_MUST_LANMAN  0
> >> > -#define   CIFSSEC_MUST_PLNTXT  0
> >> > -#ifdef CONFIG_CIFS_UPCALL
> >> > -#define   CIFSSEC_MASK          0x8F08F /* flags supported if no weak allowed */
> >> > -#else
> >> > -#define          CIFSSEC_MASK          0x87087 /* flags supported if no weak allowed */
> >> > -#endif /* UPCALL */
> >> > -#endif /* WEAK_PW_HASH */
> >> >  #define   CIFSSEC_MUST_SEAL    0x40040 /* not supported yet */
> >> >  #define   CIFSSEC_MUST_NTLMSSP 0x80080 /* raw ntlmssp with ntlmv2 */
> >> >
-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>