Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 1/3] [security] Add new hook to compare new mount to an
 existing mount
To:     Olga Kornievskaia <olga.kornievskaia@gmail.com>,
        Paul Moore <paul@paul-moore.com>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        linux-nfs <linux-nfs@vger.kernel.org>,
        Linux Security Module list 
        <linux-security-module@vger.kernel.org>,
        SElinux list <selinux@vger.kernel.org>,
        Casey Schaufler <casey@schaufler-ca.com>
References: <20210219222233.20748-1-olga.kornievskaia@gmail.com>
 <CAHC9VhRKLBNNfUE0FMgGJBR5eBQ+Et=oK1rcErUU_i62AGhfsQ@mail.gmail.com>
 <CAN-5tyGuV-gs0KzVbKSj42ZMx553zy9wOfVb1SoHoE-WCoN1_w@mail.gmail.com>
From:   Casey Schaufler <casey@schaufler-ca.com>
Message-ID: <e6d315d6-ee07-bde3-8d87-acff38d43b53@schaufler-ca.com>
Date:   Thu, 25 Feb 2021 10:22:58 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.0
MIME-Version: 1.0
In-Reply-To: <CAN-5tyGuV-gs0KzVbKSj42ZMx553zy9wOfVb1SoHoE-WCoN1_w@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
Content-Length: 3324
Precedence: bulk

On 2/25/2021 10:03 AM, Olga Kornievskaia wrote:
> On Thu, Feb 25, 2021 at 12:53 PM Paul Moore <paul@paul-moore.com> wrote=
:
>> On Fri, Feb 19, 2021 at 5:25 PM Olga Kornievskaia
>> <olga.kornievskaia@gmail.com> wrote:
>>> From: Olga Kornievskaia <kolga@netapp.com>
>>>
>>> Add a new hook that takes an existing super block and a new mount
>>> with new options and determines if new options confict with an
>>> existing mount or not.
>>>
>>> A filesystem can use this new hook to determine if it can share
>>> the an existing superblock with a new superblock for the new mount.
>>>
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>>  include/linux/lsm_hook_defs.h |  1 +
>>>  include/linux/lsm_hooks.h     |  6 ++++
>>>  include/linux/security.h      |  8 +++++
>>>  security/security.c           |  7 +++++
>>>  security/selinux/hooks.c      | 56 +++++++++++++++++++++++++++++++++=
++
>>>  5 files changed, 78 insertions(+)
>> ...
>>
>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>> index a19adef1f088..d76aaecfdf0f 100644
>>> --- a/include/linux/lsm_hooks.h
>>> +++ b/include/linux/lsm_hooks.h
>>> @@ -142,6 +142,12 @@
>>>   *     @orig the original mount data copied from userspace.
>>>   *     @copy copied data which will be passed to the security module=
=2E
>>>   *     Returns 0 if the copy was successful.
>>> + * @sb_mnt_opts_compat:
>>> + *     Determine if the existing mount options are compatible with t=
he new
>>> + *     mount options being used.
>> Full disclosure: I'm a big fan of good documentation, regardless of if=

>> it lives in comments or a separate dedicated resource.  Looking at the=

>> comment above, and the SELinux implementation of this hook below, it
>> appears that the comment is a bit vague; specifically the use of
>> "compatible".  Based on the SELinux implementation, "compatible" would=

>> seem to equal, do you envision that to be the case for every
>> LSM/security-model?

The original implementation did use sb_mnt_opts_equal(). The
change to "compatible" was my suggestion. Smack has multiple
mount options, and while I haven't actually delved into how
you would have compatible but different mount options, I
think it's possible. That's why I think that "equal" isn't
a good name for the function.

>>   If the answer is yes, then let's say that (and
>> possibly rename the hook to "sb_mnt_opts_equal").  If the answer is
>> no, then I think we need to do a better job explaining what
>> compatibility really means; put yourself in the shoes of someone
>> writing a LSM, what would they need to know to write an implementation=

>> for this hook?
> That's is tough to do as it is vague. All I was doing was fixing a
> bug. Selinux didn't allow a new mount because it had a different
> security context. What that translates to for the new hook, is up to
> the LSM module whether it would need the options to be exactly the
> same or if they can be slightly different but yet compatible this is
> really up to the LSM.
>
> Do you care to suggest wording to use? It is hard to find words that
> somebody else is looking for but one is unable to provide them.
>
>>> + *     @sb superblock being compared
>>> + *     @mnt_opts new mount options
>>> + *     Return 0 if options are compatible.
>> --
>> paul moore
>> www.paul-moore.com