From patchwork Sat Nov 24 04:11:00 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 1798011 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 4D3103FC23 for ; Sat, 24 Nov 2012 04:11:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932632Ab2KXELC (ORCPT ); Fri, 23 Nov 2012 23:11:02 -0500 Received: from mail-qa0-f53.google.com ([209.85.216.53]:64133 "EHLO mail-qa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932617Ab2KXELB (ORCPT ); Fri, 23 Nov 2012 23:11:01 -0500 Received: by mail-qa0-f53.google.com with SMTP id k31so1852943qat.19 for ; Fri, 23 Nov 2012 20:11:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=2DKnidInH4NoGu7NM9GyDiwqNyDSpkaeZSBwlMTGzAE=; b=CyRGdrKShq+g0553Tgmsg4gSDFitT1eq5PW3cVL02LQp2TU3LEa1vGj71UZso1/uYS 27IHS9qz59JjsPMLfdZIFzQP0o0rHoxVVymSfAkWUAxFTgxr0gyqnoP3SQ1TKcvgdKoe nSCfWXPKCr9b+wRn4U3Dq2aVlWf7XOw6THkCQToGWxes02yLKYzySyQs01Dmh479/EXa akc4jIA+sYjCWEljZy3dGS75vg5SI+lHtKKu0u1qcCuHUlFPzvfNKty/e/ZJKwPNuWPJ bKwz+lZZkgsKFcPzqW33R5XVsQk1AGLIgtKRFPMw4Ei3UxtxCLnMpfO1mV6IDGnHJ5KC waOw== MIME-Version: 1.0 Received: by 10.224.182.203 with SMTP id cd11mr6332919qab.22.1353730260438; Fri, 23 Nov 2012 20:11:00 -0800 (PST) Received: by 10.49.73.170 with HTTP; Fri, 23 Nov 2012 20:11:00 -0800 (PST) In-Reply-To: References: <20121123204136.5466f0e9@corrin.poochiereds.net> Date: Fri, 23 Nov 2012 22:11:00 -0600 Message-ID: Subject: Re: Upgrading security default From: Steve French To: Jeff Layton Cc: linux-cifs@vger.kernel.org Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org To be more specific: do you prefer this CIFSSEC_MAY_LANMAN | CIFSSEC_MAY_PLNTXT | CIFSSEC_MAY_ /* On Fri, Nov 23, 2012 at 10:09 PM, Steve French wrote: > On Fri, Nov 23, 2012 at 8:52 PM, Steve French wrote: >> changing >> #define CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLM | >> CIFSSEC_MAY_NTLMV2 | CIFSSEC_MAY_NTLMSSP) >> >> to >> >> #define CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLMSSP) >> >> affects more code > > It does seem to work - global_secflag is touched in more places, but > looks safe enough as an alternative. Do you prefer the other change? > >> On Fri, Nov 23, 2012 at 8:48 PM, Steve French wrote: >>> it doesn't change security flags - but it seemed the smallest and >>> safest since it basically says: >>> 1) if you pass in "sec=" then use that >>> 2) otherwise use ntlmssp (with ntlmv2) >>> >>> so shouldn't have any unintended consequences (and the sign mount >>> option should work as expected as well) >>> >>> On Fri, Nov 23, 2012 at 7:41 PM, Jeff Layton wrote: >>>> On Fri, 23 Nov 2012 17:36:45 -0600 >>>> Steve French wrote: >>>> >>>>> This patch to upgrade the default security mechanism to ntlmv2/ntlmssp >>>>> (which is broadly supported for years now, and a reasonable minimum, >>>>> far better than ntlm) is overdue, but I had to rework it to simplify >>>>> it. >>>>> >>>>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >>>>> index 5c670b9..3bca289 100644 >>>>> --- a/fs/cifs/connect.c >>>>> +++ b/fs/cifs/connect.c >>>>> @@ -1103,6 +1103,7 @@ cifs_parse_mount_options(const char *mountdata, >>>>> const char *devname, >>>>> bool uid_specified = false; >>>>> bool gid_specified = false; >>>>> bool sloppy = false; >>>>> + bool sec_explicitly_set = false; >>>>> char *invalid = NULL; >>>>> char *nodename = utsname()->nodename; >>>>> char *string = NULL; >>>>> @@ -1763,6 +1764,7 @@ cifs_parse_mount_options(const char *mountdata, >>>>> const char *devname, >>>>> >>>>> if (cifs_parse_security_flavors(string, vol) != 0) >>>>> goto cifs_parse_mount_err; >>>>> + sec_explicitly_set = true; >>>>> break; >>>>> case Opt_cache: >>>>> string = match_strdup(args); >>>>> @@ -1799,6 +1801,8 @@ cifs_parse_mount_options(const char *mountdata, >>>>> const char *devname, >>>>> goto cifs_parse_mount_err; >>>>> } >>>>> #endif >>>>> + if (sec_explicitly_set == false) >>>>> + vol->secFlg |= CIFSSEC_MAY_NTLMSSP; >>>>> >>>>> if (vol->UNCip == NULL) >>>>> vol->UNCip = &vol->UNC[2]; >>>>> @@ -2397,8 +2401,6 @@ cifs_set_cifscreds(struct smb_vol *vol >>>>> __attribute__((unused)), >>>>> } >>>>> #endif /* CONFIG_KEYS */ >>>>> >>>>> -static bool warned_on_ntlm; /* globals init to false automatically */ >>>>> - >>>>> static struct cifs_ses * >>>>> cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info) >>>>> { >>>>> @@ -2475,14 +2477,6 @@ cifs_get_smb_ses(struct TCP_Server_Info >>>>> *server, struct smb_vol *volume_info) >>>>> ses->cred_uid = volume_info->cred_uid; >>>>> ses->linux_uid = volume_info->linux_uid; >>>>> >>>>> - /* ntlmv2 is much stronger than ntlm security, and has been broadly >>>>> - supported for many years, time to update default security mechanism */ >>>>> - if ((volume_info->secFlg == 0) && warned_on_ntlm == false) { >>>>> - warned_on_ntlm = true; >>>>> - cERROR(1, "default security mechanism requested. The default " >>>>> - "security mechanism will be upgraded from ntlm to " >>>>> - "ntlmv2 in kernel release 3.3"); >>>>> - } >>>>> ses->overrideSecFlg = volume_info->secFlg; >>>>> >>>>> mutex_lock(&ses->session_mutex); >>>>> >>>> >>>> How does this change the SecurityFlags interface? >>>> >>>> -- >>>> Jeff Layton >>> >>> >>> >>> -- >>> Thanks, >>> >>> Steve >> >> >> >> -- >> Thanks, >> >> Steve > > > > -- > Thanks, > > Steve diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index f5af252..2cd5ea2 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -1362,7 +1362,7 @@ 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 | CIFSSEC_MAY_NTLMSSP) +#define CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLMSSP) #define CIFSSEC_MAX (CIFSSEC_MUST_SIGN | CIFSSEC_MUST_NTLMV2) #define CIFSSEC_AUTH_MASK (CIFSSEC_MAY_NTLM | CIFSSEC_MAY_NTLMV2 |