Subject: Re: [nfsv4] Review of http://www.ietf.org/id/draft-naik-nfsv4-xattrs-01.txt

From: Manoj Naik <[email-masked]>
Date: Thu, 2 Oct 2014 15:22:10 -0700

Thomas Haynes <[email-masked]> wrote on 09/29/2014 06:40:05
PM:

> From: Thomas Haynes <[email-masked]>
> To: Manoj Naik/Almaden/IBM@IBMUS
> Cc: NFSv4 <[email-masked]>
> Date: 09/29/2014 06:41 PM
> Subject: Re: [nfsv4] Review of http://www.ietf.org/id/draft-naik-
> nfsv4-xattrs-01.txt
>
> On Sep 29, 2014, at 9:15 PM, Manoj Naik <[email-masked]> wrote:
>
> "nfsv4" <[email-masked]> wrote on 09/29/2014 08:48:15 AM:
>
> > From: Tom Haynes <[email-masked]>
> > To: NFSv4 <[email-masked]>
> > Date: 09/29/2014 08:48 AM
> > Subject: [nfsv4] Review of http://www.ietf.org/id/draft-naik-nfsv4-
> > xattrs-01.txt
> > Sent by: "nfsv4" <[email-masked]>
> >
> > Section 5.2.3.4.
> >    the delegation. See Section 18.7.4 of RFC 5661 for details [2].
> >    Consequently, if a client needs to verify the list of extended
> >    attributes with the server, it must also query the change attribute
> >    of the file with GETATTR. This handling is similar to how a client
>
> > Is that a must or a MUST?
>
> It's a MUST. I'll fix it in the next revision.
>
> >
> > —
> >
> >    The SETXATTR operation changes one or more of the extended
attributes
> >    of a file system object. The change desired is specified by sr_type.
> >    SETXATTR4_CREATE is used to associate the specified values with the
> >    extended attribute keys for the file system object specified by the
> >    current filehandle. The server MUST return an error if the attribute
> >    key already exists. SETXATTR4_REPLACE is also used to set an xattr,
> >    but the server MUST return an error if the attribute key does not
> >    exist. An application can delete all existing xattrs for a file and
> >    replace them with a new set by using SETXATTR4_REPLACE_ALL.
> >    SETXATTR4_DELETE can be used to remove the specified xattr keys, if
> >    they exist. SETXATTR4_DELETE_ALL removes all the xattr keys for the
> >    file.
>
> >
> > Use a hanging list for these…
>
> Okay.
>
> >
> > —
> >
> >    While the SETXATTR request MAY contain multiple attribute keys
and/or
> >    values to be changed for a file, this does not impose any atomicity
> >    considerations on the server implementation. If the server cannot
> >    update all the attributes for the file atomically, it MUST set them
> >    in the order specified. In such cases, it is possible that some keys
> >    are changed successfully while others encounter errors. To handle
> >    this, contained within the SETXATTR results is a "status" field.  If
> >    any of the change operations incur an error, then the "status" value
> >    MUST NOT be NFS4_OK. In this case, the status of the individual
> >    change operations is returned in sr_array. If the xattr keys and/or
> >    values contained in the client request exceeds the channel's
> >    negotiated maximum request size, then the server MUST return
> >    NFS4ERR_REQ_TOO_BIG in sr_status.
>
> >
> > I thought in earlier drafts that the atomicity requirements were
> > what kept us from using named attributes.
> > If there are no longer any atomicity requirements, does that open
> > the door back for using named attributes?
>
> Bruce touched on this too, while the setting of a single xattr is
> atomic, multiple keys for the same file MUST be updated on order,
> but MAY NOT be atomic. This is mentioned in Section 4 that
> differentiates with named attrs, but I'll reinforce it in this section.
>
> >
> > —
> >
> >    A successful SETXATTR SHOULD change the file time_modify and change
> >    attributes. However, these attributes SHOULD NOT be changed unless
> >    the xattrs are changed.
>
> >
> > And an unsuccessful SETXATTR can do what? I.e., what if some keys
> > are changed successfully and some are not?
>
> Good point. An unsuccessful SETXATTR that changes some of the keys
> SHOULD also update time_modify and change. Will fix.
>
> >
> > Also, I find the two sentences to be contradictory. Under what
> > circumstances could the SETXATTR be successful and not change the
xattrs?
>
> I think you are saying the SHOULD needs to be a MUST. Agreed.
>
> >
> > ——
> >
> > 6  Security Considerations
> >
> >    The additions to the NFS protocol for supporting extended attributes
> >    do not alter the security considerations of the NFSv4.1 protocol
[2].
>
> >
> > I’m not sure this is as simple as suggested here.  You should talk
> > about what could happen if the NFS client interprets the value.
>
> Are the considerations different for this compared to named attributes?
> So no security discussion for named attributes in 3530 or 5661.
>
> But named attributes are not supposed to be opaque blobs which could
> be executed.

Neither are xattrs. Although opaque, they are just additional metadata that
need to be associated with a file.

>
>
> >
> > Also, wasn’t an earlier concern that these payloads could be
> > executable code? I thought we agreed to disallow this?
>
> Perhaps I missed that - what specifically would you prefer we
> disallow in the spec? xattrs have been defined to be opaque to both
> the client and the server - is the concern about the underlying
> filesystem being compromised through the protocol?
>
> Yes.

OK, could you elaborate on how this is possible? The purpose of the
protocol is to pass these opaque blobs of (application-specific) data
between the client and server. These are then handed off by the server to
the underlying file system using a well-defined, perhaps platform-specific,
interface that exists today and is used by other applications (besides
NFS). What additional insecurity are we introducing?

Manoj.

>
>
> >
> > Finally, can these values be used as attack vectors?  I.e., the
> > client may already be exposed to them from local xattrs, but doesn’t
> > having the xattrs being stored away from the client introduce some
risk?
>
> I think this relates to the above, but perhaps you could elaborate.
>
> Do existing client implementations allow security scanners to sanitize
xattrs?
>
>
> Thanks for the comments.
> Manoj.
>
> > _______________________________________________
> > nfsv4 mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Nico Williams <[email-masked]>
Date: Thu, 2 Oct 2014 17:53:28 -0500

On Mon, Sep 29, 2014 at 11:48:15AM -0400, Tom Haynes wrote:
>    do not alter the security considerations of the NFSv4.1 protocol [2].
> 

The client itself wouldn't interpret any such values unless we define
NFS-specific xattrs (which I gather we wouldn't).

Applications running on the client might, and there's nothing so
different about xattrs relative to plain file data that should warrant
additional security considerations.  The moment you trust the server to
give you this data... (or to take some from you...)

> executable code? I thought we agreed to disallow this?

How?  Suppose I store "foo () { echo bar; }" in an xattr.  How
would either the client or server know that that is shell code?  It's
one thing to recommend against, another to disallow.

You might say that users SHOULD NOT execute code in xattrs, but... why
not?  Are we going to tell users not to put executable code in file
data?

NFS is just a filesystem protocol.  You'd have to tell them not to do it
regardless of the filesystem type.  This is not NFS's place.

> Finally, can these values be used as attack vectors?  I.e., the client
> may already be exposed to them from local xattrs, but doesn’t having
> the xattrs being stored away from the client introduce some risk?

I don't see anything new here.  The moment you are willing to hand your
data to a server and later get it (and other data) back from it, you're
in that server's hands -- and others trusting your files' contents are
in yours.  There's nothing new here since the 1970s at least, if not
easier.

Nico
--

---

From: Thomas Haynes <[email-masked]>
Date: Fri, 3 Oct 2014 13:53:30 -0400

Nico,

I can't find a definitive reference, but google gets me this:

http://www.tuxera.com/community/ntfs-3g-advanced/extended-attributes/

> On Linux, specifically, four categories of extended attributes have been defined :
> 
> trusted : to record properties which should only be accessed by the kernel,
> security : to record security properties of a file,
> system : to record other system related properties on which the file owner has some control,
> user : to record properties defined by applications.
> The names of the extended attributes must be prefixed by the name of the category and a dot, hence these categories are generally qualified as name spaces. Examples of extended attribute names aresecurity.selinux,  system.posix_acl_access or user.mime_type. However a high-level language may hide the system prefixing so that, for instance, the user.mime_type attribute name would appear as“mime_type” in a source code.
> 
> 

So, given the text in the current draft, how does the server enforce that the Linux client is going to do
the right thing with trusted and security properties?

Note, I am assuming by the text in system that the file owner has no control over security.

Also, given that there is no standard on xattrs, what prevents a non-Linux client from accessing them?

BTW - http://blog.cloudera.com/blog/2014/06/why-extended-attributes-are-coming-to-hdfs/ discusses how
HDFS tackles this issue:

> Every extended attribute name must include a namespace prefix, and just like in the Linux filesystem implementations, there are four extended attribute namespaces: user, trusted, system, and security. The system and security namespaces are for HDFS internal use only; only the HDFS super user can access trusted namespace. So, user extended attributes will generally reside in the user namespace (for example, “user.myXAttrkey”). Namespaces are case-insensitive and extended attribute names are case-sensitive.

So we see other protocol implementations tackling security issues.

Another interesting thread about security and xattrs: http://oss.sgi.com/archives/xfs/2013-12/msg00677.html

Two points in here:

1) XFS has trusted and user address spaces.

I.e., current xattr implementations are dependent on the underlying file system implementation for
normal semantics and security semantics. If the client layer is built on a common subset of features
across all of the filesystems, what happens if the NFS client provides none of those features?

I'm willing to concede that there may indeed not be any such common subset, but that is something
that the draft should prove, not assert.

The draft takes the stance that xattrs exist on many platforms, there is no standard for their use,
and that means there is no need to define any requirements for the use in NFS.

2) Subject/Object labels can control access to the xattr.

So some of these concerns go away with LNFS. But the fact that LNFS does take care of some of the
security in my mind means that the draft needs to call out what happens when there is no LNFS available.

Tom

---

From: Nico Williams <[email-masked]>
Date: Fri, 3 Oct 2014 14:20:10 -0500

> http://www.tuxera.com/community/ntfs-3g-advanced/extended-attributes/

The attr(5) manpage on Linux ought to be definitive, but some such
manpages are much less informative...  This one seems OK:

http://www.bestbits.at/acl/man/man5/attr.txt

> > user : to record properties defined by applications.

These relate to what authorization is required in order to get/set them.
The page you link to explains this; others do as well.

> So, given the text in the current draft, how does the server enforce
> that the Linux client is going to do the right thing with trusted and
> security properties?

Either the client is authorized (and trusted) or not (and not).

For some xattrs the server might be able to validate content, but as a
general rule it will not be able to: it's an open set of attributes.

In any case, I expect that the NFSv4 server _proper_ will do no such
validation.  Rather, I expect the underlying filesystem to do it, if at
all.  If we have to say much about this it's only because: a) there may
be no other place for us to say it or because the relevant external
references are not acceptable as normative, b) because we have to say
something about when a client is equivalent to "root" or CAP_SYS_ADMIN
(which is what those external references refer to).

(And of course CAP_SYS_ADMIN gets us back to talking about RPCSEC_GSSv3.)

I would argue that we can't fork the external references, therefore
we'll have to have normative references to external sources that we'd
not normally be happy to normatively refer to.

> non-Linux client from accessing them?

Clearly some component on the server must.  For a typical NFSv4 layered
implementation that component would have to be the filesystem, not the
NFSv4 server.

> BTW -
> http://blog.cloudera.com/blog/2014/06/why-extended-attributes-are-coming-to-hdfs/
> discusses how HDFS tackles this issue:
> 
> > Every extended attribute name must include a namespace prefix, and
> > just like in the Linux filesystem implementations, there are four
> > extended attribute namespaces: user, trusted, system, and security.
> > The system and security namespaces are for HDFS internal use only;
> > only the HDFS super user can access trusted namespace. So, user
> > extended attributes will generally reside in the user namespace (for
> > example, “user.myXAttrkey”). Namespaces are case-insensitive and
> > extended attribute names are case-sensitive.
> 

The right thing to do would be to have a reference to a suitable
document and explain how the server is to perform the relevant
authorization checks.

> 1) XFS has trusted and user address spaces.

Not just XFS.

> I.e., current xattr implementations are dependent on the underlying
> file system implementation for normal semantics and security
> semantics. If the client layer is built on a common subset of features

The *filesystem* has to implement these semantics, not the NFSv4 client,
and not the NFSv4 server.  The NFSv4 client and server provide the
necessary transport between the OS on the client and the filesystem on
the server.  And the server has to mediate authorization.  But that's it.

I've commented before that we ought to strongly distinguish the protocol
and the filesystems.

> I'm willing to concede that there may indeed not be any such common
> subset, but that is something that the draft should prove, not assert.

I don't agree.  First, "prove" is a bit strong.  Second, the set of
xattrs is open.  Third, semantics may vary by filesystem/OS.

> The draft takes the stance that xattrs exist on many platforms, there
> is no standard for their use, and that means there is no need to
> define any requirements for the use in NFS.

Here I agree with you that the I-D must explain how the server helps
mediate access control.  And it must explain that some xattrs have
security-relevant semantics.  But that's as far as an NFSv4
specification can go here.

> So some of these concerns go away with LNFS. But the fact that LNFS
> does take care of some of the security in my mind means that the draft
> needs to call out what happens when there is no LNFS available.

LNFS seems orthogonal here.  RPCSEC_GSSv3 is more relevant.

Nico
--

---

From: Christoph Hellwig <[email-masked]>
Date: Sat, 4 Oct 2014 09:21:28 +0200

On Mon, Sep 29, 2014 at 12:08:37PM -0400, J. Bruce Fields wrote:
> They're saying that setting multiple xattrs isn't atomic.
> 
> I assume that a set of an individual xattr still is.  (I haven't checked
> whether the draft states that.)
> 
> That would differ from named attributes, which are "set" by (possibly
> multiple, possibly short) WRITEs.

How about fixing this by not supporting multiple writes in one operation?

I've noticed that NFSv4 has a lot of cases of poinless multiple operation
support in the invididual operation XDR.  Not only can't most clients make
use of this and it places a burden of pointless complexity on the server,
but it's also not needed for batching purposes either in case a client
can handle it, as a compound operation can already be archived for that
effect.

---

From: Christoph Hellwig <[email-masked]>
Date: Sat, 4 Oct 2014 09:24:09 +0200

> give you this data... (or to take some from you...)

They actually are slightly simpler as far as the OS is concerned,
although very little of that matters of the NFSv4 spec.  Unlike files
and named attributes there is no executable bit for xattrs, and there are
no system calls to execute them or mmap them as executable code.

---

From: David Quigley <[email-masked]>
Date: Wed, 08 Oct 2014 09:21:22 -0400

> specification can go here.

I think that the xattrs exposed through this shouldn't be 
security-relevant unless you're talking about application layer 
security. I don't care if a web app or some user-space program uses an 
xattr for its internal access control but anything beyond that I don't 
think is ok. Trond in the past has expressed concerns about people 
establishing arbitrary sub protocols by using xattrs. I agree that this 
is a problem because it opens up the system to interoperability issues 
by encouraging people to bypass any sort of standardization effort and 
just using the excuse that its just opaque data.

> LNFS seems orthogonal here.  RPCSEC_GSSv3 is more relevant.

I agree. LNFS is just another way of conveying access control and 
because we punted on syntax, semantics and enforcement I think the only 
references to access control should be what is concretely defined in 
NFSv4. In that case it would be existing RPCSEC_GSS flavors for 
authentication and NFSv4 ACLs.

---

From: Nico Williams <[email-masked]>
Date: Fri, 10 Oct 2014 19:56:18 -0500

On Wed, Oct 08, 2014 at 09:21:22AM -0400, David Quigley wrote:
> security. [...]

I agree with you, but system security-relevant xattrs is a ship that has
sailed.  Perhaps NFSv4 should just not transport them, but I think
that's probably not useful enough.  The really important thing is to
decide when a client is sufficiently privileged, and for that you need
something like RPCSEC_GSSv3 (that allows you to authenticate a client
and a user together and allows the client to make assertions about local
privilege that the server can then decide how much to reduce -- or even
to reject completely, of course).

>                                                       [...]. I agree
> that this is a problem because it opens up the system to
> interoperability issues by encouraging people to bypass any sort of
> standardization effort and just using the excuse that its just
> opaque data.

I agree that non-standardized conventions make xattrs _dangerous_.
Fortunately there appear to be naming conventions that we could
encourage the standardization of, and maybe that will be enough.

> because we punted on syntax, semantics and enforcement I think the
> only references to access control should be what is concretely
> defined in NFSv4. In that case it would be existing RPCSEC_GSS
> flavors for authentication and NFSv4 ACLs.

Right, except that xattrs have no ACLs as such.  (We could codify the
naming conventions and express their associated access controls
synthetically with NFSv4 ACLs...  But note that still, when a filesystem
says that you need "root" privilege (or similar), the context is local
access, and extending such notions of privilege to the network requires
something like RPCSEC_GSSv3.

Nico
--