Re: [nfsv4] New Version Notification for draft-cel-nfsv4-linux-seclabel-xtensions-00.txt

[Chuck Lever <[e-mail]>]

> On May 2, 2018, at 7:10 PM, Quigley, David <[e-mail]> wrote:
> 
> Hi Chuck,
> 
> I'll address each thing in turn since outlook doesn't seem to like inline commenting
> 
> I would say there is a prohibition on it as it was the only reason we went down the sec_label route in the first place. The initial proposal was to have xattrs on nfsv4 and put the security label in there. We designed the sec_label attr for MAC labels and the LFS specification is also explicitly for MAC Labels. We explicitly state that FATTR4_SEC_LABEL is a MAC Security Attribute in the 4.2 specification. This field is called sec_label which is kind of unfortunate name as it was intended to represent security labels in the traditional orange book sense.  It is not intended for arbitrary security related data. It is an access control field not an integrity field. Using it as a blob store for integrity data is similar to the pushback I received from Trond when I wanted to put this data in xattrs to begin with.
> 
> I also said I could see why you'd want to use it for capability data but to be honest capabilities should have their own mechanism just like how they do in Linux today. Its unfortunate that all of this is stores in xattrs on Linux because it really shouldn't be. The initial version of SELinux had new system calls for getting and setting security related information. It was only because the way of handling things at the time was to extend interfaces through the file system and psudo filesystems that SELinux and IMA look the way they do today. If you look at other systems that did it properly (FreeBSD) they introduced proper syscalls for all of this.
> 
> With respect to the three LFS numbers they are only supposed to cover data in the sec_label attribute. Do you only intend on storing one of the three in the attribute? How is this supposed to work? If you have a setfattr with an IMA LFS encoded attribute and then an EVM encoded attribute it would just overwrite it. You wouldn't be able to store them independently with the one attribute as it is unless I am misunderstanding something.

Let's dive into this one for a moment.

Are you saying that a file is allowed only one NFSv4 security label at a time,
similar to the way a file can have only one NFSv4 ACL?

If that's the case, then NFSv4 security labels are not going to work at all,
and we'll have to create a separate purpose-built mechanism for each of these
objects. Obviously we do need to store an ACL and an SELinux label and IMA
metadata and file capabilities separately.


> I will have to look at what was done with the final implementation that made its way into the Linux kernel but the sec_label attribute is supposed to be limited to one value per file. It is not supposed to be a multiplexed attribute based on the LFS. Security labels in a MAC sense are explicitly one value per file.
> 
> I will give it a closer read tonight and look at the implementation in the Linux Kernel. However it was not intended that if you do serfattr with two different lfs that you would store two different values.
> 
> Dave
> 
> -----Original Message-----
> From: Chuck Lever [mailto:[e-mail]] 
> Sent: Wednesday, May 2, 2018 4:51 PM
> To: Quigley, David <[e-mail]>
> Cc: Tom Haynes <[e-mail]>; Spencer Shepler <[e-mail]>; NFSv4 <[e-mail]>
> Subject: Re: [nfsv4] New Version Notification for draft-cel-nfsv4-linux-seclabel-xtensions-00.txt
> 
> Hi David-
> 
>> On May 2, 2018, at 6:36 PM, Quigley, David <[e-mail]> wrote:
>> 
>> I can understand wanting to use the sec_label field for capabilities but using it for the IMA and EVM data isn’t really in the spirit of what we designed this for. Those are integrity mechanisms and on Linux systems they are completely separate from the LSMs (access control is not integrity).
> 
> Yep, I get that, but I don't see a functional problem here. It seems like an artificial constraint to say that we can't store IMA metadata in a generic security label, and I don't see any explicit prohibition to doing so laid out in the existing RFCs (if there is one, please point it out).
> 
> You can make a similar argument about file capabilities, which are not MAC security. Should they be excluded from using a security label for this reason?
> 
> 
>> Also having them as 3 different LFS ids make no sense. The LFS is supposed to describe the format of what you are sending and receiving for the sec_label field. Unless something changed in the last rounds of revisions to the sec_label support there should only be one value per file?
> 
> A file can have capabilities associated with it, a security.ima xattr, and a security.evm xattr. All three are separate objects and can change independently of each other. Capabilities and IMA are not at all related to each other, so at least these two need separate LSF numbers.
> 
> 
>> Also something to consider is that Nico explicitly separates capabilities from MAC labeling in in RPCSECGSSv3. This is because traditionally capabilities and MAC systems are orthogonal to each other. I know file capabilities are different from the regular capabilities on processes in Linux but it is something to consider that on a normal Linux system they can co-exist and be enforced at the same time. With this mechanism you couldn’t support both SELinux and file-capabilities at the same time like you could on a local system.
> 
> Implementations can store the content of each LSF in separate xattrs, or so it appears from my brief audit of the Linux implementation. I don't see anything that prevents storing an SELinux label in one xattr, file capabilities in a second xattr, and IMA metadata in a third xattr.
> 
> 
>> I will be able to provide better feedback once I’ve read through the document another couple of times but those are my initial impressions just from a sec_label and LFS perspective. 
> 
> Thanks for looking!
> 
> 
>> Dave
>> 
>> From: nfsv4 [mailto:[e-mail]] On Behalf Of Tom Haynes
>> Sent: Wednesday, May 2, 2018 3:30 PM
>> To: Chuck Lever <[e-mail]>; Spencer Shepler 
>> <[e-mail]>
>> Cc: NFSv4 <[e-mail]>
>> Subject: Re: [nfsv4] New Version Notification for 
>> draft-cel-nfsv4-linux-seclabel-xtensions-00.txt
>> 
>> Hey Chuck,
>> 
>> For the most part, my issues are the ones common to first drafts.
>> 
>> And Spencer S,
>> 
>> Can we nominate this as official WG document? It is mature enough and on point with the earlier LFS work.
>> 
>> Thanks,
>> Tom
>> 
>> Abstract
>> 
>> NFS version 4.2
>> Need to define NFS first.
>> 
>> Introduction
>> 
>> Before specifying new protocol,
>> ->
>> 
>> Before specifying a new protocol,
>> —
>> 
>> (hereafter, IMA)
>> (hereafter, EVM)
>> drop the hereafters
>> 
>> —
>> 
>> This is done by cryptographically signing HMAC please introduce HMAC
>> --
>> RSA public key signature
>> Please define RSA
>> 
>> ——
>> 
>> The goals and use cases of the Linux Integrity Measurement
>>  Architecture (IMA) are presented in further detail in [IMA-WP].
>> Already have defined IMA
>> 
>> —
>> 
>> with the
>>  superuser
>> What is a superuser?
>> 
>> ——
>> 
>> You then have four bullet points, which as they are complete sentences should end with ‘.’
>> 
>> —
>> 
>> execve(2)
>> 
>> need a citation (probably to POSIX)
>> 
>> ===
>> 
>> Section 3
>> 
>> Linux file capabilities
>> becomes
>> a capability set
>> without motivating the connection.
>> 
>> =====
>> 
>> LFS
>> 
>> not introduced.
>> 
>> ====
>> 
>> MAC
>> 
>> not introduced
>> 
>> —
>> 
>>  In order to enable file capabilities to be retrieved or updated in a
>>  single RPC, the text format representation of a capability set MUST
>>  NOT exceed 8192 bytes in length.
>> 
>>  In order to enable IMA metadata to be retrieved or updated in a
>>  single RPC, a signed hash MUST NOT exceed 4096 bytes in length.
>> Why the restrictions? Is it a matter of the length of the compound?
>> 
>> ——
>> 
>> a Merkle
>> citation?
>> 
>> ——
>> 
>> An NFSv4 server is required to enforce a suitable level of privilege
>>  before allowing a local or remote agent to alter NFSv4 Security
>>  Labels.  Consult Section 9.6 of [RFC7862] for further details.
>> 
>> Is the last sentence a sentance?
>> 
>> ——
>> 
>> Therefore additional protection using GSS
>>  [RFC7861] or other security mechanisms is not mandatory.
>> 
>> Either define GSS or perhaps say RPCSEC_GSS?
> 
> --
> Chuck Lever
> 
> 
> 

--
Chuck Lever