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

From: Tom Haynes <[email-masked]>
Date: Mon, 29 Sep 2014 11:48:15 -0400

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?

—

   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…

—

   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?

—

   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?

Also, I find the two sentences to be contradictory. Under what circumstances could the SETXATTR be successful and not change the xattrs?

——

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.

Also, wasn’t an earlier concern that these payloads could be executable code? I thought we agreed to disallow this?

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?

---

From: Tom Haynes <[email-masked]>
Date: Mon, 29 Sep 2014 11:54:07 -0400

> 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 so…

Also, I’d like to see some discussion of what client A does if the maxxattrsize it supports locally is less than the attrsize present in the xattr?

I.e., does it only use those before the limit?

What happens if one spans the limit, etc?

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 29 Sep 2014 12:08:37 -0400

> If there are no longer any atomicity requirements, does that open the door back for using named attributes?

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.

--b.

---

From: Manoj Naik <[email-masked]>
Date: Mon, 29 Sep 2014 18:15:42 -0700

"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.

> Is that a must or a MUST?

It's a MUST. I'll fix it in the next revision.

> Use a hanging list for these…

Okay.

>
> 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.

>
> 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.

>
> 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?

>
> 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?

>
> 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.

Thanks for the comments.
Manoj.

> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Thomas Haynes <[email-masked]>
Date: Mon, 29 Sep 2014 21:40:05 -0400

On Sep 29, 2014, at 9:15 PM, Manoj Naik <[email-masked]> wrote:

> 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.
> 
> > 
> > —
> > 
> Good point. An unsuccessful SETXATTR that changes some of the keys SHOULD also update time_modify and change. Will fix.
> 
> > 
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.

> 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. 

> I think this relates to the above, but perhaps you could elaborate.

Do existing client implementations allow security scanners to sanitize xattrs?