[nfsv4] Re: Review of draft-haynes-nfsv4-uncacheable

[Thomas Haynes <[e-mail]>]

Finally got around to rewriting the draft based on this input.

First, I’d like to point out that this review focuses rather heavily on the rubric "proprientary policies”. I’ll admit that Hanmerspace has a means to do this, but does not. I added this throw-away addition because I am aware of two other server implementations that had policy engines which could be used for this purpose.

I’m not going to address the points below, it is almost a complete rewrite and it addresses the intent.

> On Nov 12, 2024, at 10:10 PM, David Noveck <[e-mail]> wrote:
> 
> General Comments
> Brief Summary
> Although not specfically mentioned, the document under review is an attempt to provide support for the two features listed below.  Later, in Replacement Suggestions, we will discuss better ways of providing support for these features.
>  Access-based enumeration, although not explicitly mentioned, is clearly being referred to under the rubric "proprientary policies".  While the document under review does not explicitly make this feature valid and the reviewer has no objection to its incluson in NFSv4, the docment under review does so implicitly and then uses that presumed validity as a justification for the new uncacheable attribute.
> The addition of such a feature, while desirable, is better done explicitly so that clients are be made aware of its use, rather than opening the door for an unbounded set of "proprietary policies"
> 
> The denial of autnorirization to perform attribute interrogaion is a feature of the NFSv4 ACL model.  Servers that support it are quite limited and it is not totally sure that any exist.  In the document under review, this limited level of use is not noted and the potential existence of such servers is used as a justfication for an uncacheable attribute whose implementation is not necessary for most servers and could have further negative effects.
> The implications  of ACLs which deny permission interrogate a file's attribute have never been dealt with but te proper place to deal with these issues is as par of the rfc5661bis effort where work in this area has already begun.  That work need to be completed in that framework.
> 
> As will be seen later, by combinng these need in this single attribute, a short document can be arrived at but it needs to be noted how disruptive the addition of such an attribute would be.  For this reason, the author feels that the secific motivations mentioned above need to be dealt with specfically while other supposed justifications looked at carefully to see if they merit the disruption of the protocol architecture which treats the availbility of caching as a generally hepful tool important in providing acceptable performance.
> Overall Evaluation
> The proposed additions to NFSv4.2 suffer from the following basic problems:
> The proposed non-cacheability applies to three different categories of data and presents a justification for only one of these, i.e for authorization control attributes, while assuming, without any providing any justification, that a file/directory whose authorization control attributes are not to be cached will be one whose data cannot be cached.
> The justification makes some assumptions about control of authorzation for fetching attributes that are, for the vast majority of servers, incorrect.   This reflects the POSIX authorization model which most servers use.
> The presentation has the unfortunate effect of giving the impression that when this new OPTIONAL attribute is not set or is not supported by either the client or server, authorization control attributes can be cached when they should not be.
> The repeated use of of the phrase "MUST NOT" is troublesome, since it is defined by RFC2119 as indicating a "Fundamental requirement of the specification".   It is hard to see how a "fundamental requirement" of the v4.2 specification could be added via an extension, since, if it were a fundamental requirement, it would have already been present.  In addition, the optionality of the attribute suggests that the need to not cache the data in question is not so fundamental after all.  In addition, conceptually, it is hard to understand how one client, by setting this attribute, can impose a fundamental requirement on others or how another client can vitiate this requirement, by resetting this attribute.
> Access-based Enumeration:
> While this is a useful feature, it needs to be introduced into NFSv4 as a valid choice, rather than as a miscellaneous piece of "proprietary" semantics. If, as appears likely, it has noteworthry effects on caching, that needs to be made clear in a read-only  attribute specifying this behavior.  If an "uncachable" attribute is the vehicle for caching control, then:
> The server should be the one controlling the attribute cacheability, rather than assumng, as curently, that somehow, this client an find out whether attribute-based enueration is to be provided and whether caching is to be inibited.
> The connection with caching of data, if needed, should be separate attribute as there is no reason to suppose it is somehow connected to access-based enumeration.
> ACL-Based Denial of Getattr.
> While this feature was added, without much attention to the consequences, when NFSv4 ACLs were added to RFC3010, it is more appropriate to address the inadequate discussion of the consequences as part of the rfc5661bis effort.  This has already been done in connection with directory caching and could be extended to deal with other forms of positive and negative directory caching.
> In any case, it needs to be made clear when caching can and cannot be validly done, rather than assuming that, for all clients and servers on which this new OPTIONAL attribute is not implemented have license to cache where the ACL as defined in rfc8881/rfc5661bis clearly implies that this is wrong/
> Per-section Comments
> Abstract
> The sentence "But this defeats the server checking to see if the user has permission to view the dirent's attributes"  adds confusion and start the discussion in the wrong discussion in that it ignores the fact that the purported server checking referred to is extremely rare.   Vewing attributes for files is universally allowed when using the POSIX authorization model or authorization using draft POSIX ACLs.   While it is true that the NFSv4 ACLs model does have provisions allowing reading of attributes to be denied via ACLs, so far we don't know of any server that actually implements this feature.   If the server in question follows POSIX in making attribute interrogation universally available, then there is no need for this attribute, while, if it does, the attribute is also unnecessary since the protocol needs to be specified not to use caching when it would defeat the security provisions of ACLs.  This is done in the latest rfc5661bis draft and should be reviewed for adequacy.  rfc8881 does not address this issue but adding this new attribute is not the right way to address this issue, since it leaves the problem unaddressed in v4.0 and in v4.1 implementations not implementing this extension..
> 
> The sentence "Caching the data can cause performance impacts if the page cache hit rate is low", while true, ignores the reasons for caching.  There is no explanation how one client can or should be given the ability to prevent another client from caching, where the actual caching client finds it helpful.
> 
> Overall, it needs to be made clearer whether there is any need/justifcation  for this new attribute for servers that do POSIX-like authorization (i. e., extremely liberal for attribute interrogation). It also needs to be clarified whether the need/validity for caching attributes  is in any way connected to the need/validity to cache file or directory contents.   This is of particular concern since NFSv4.1 already has a directory caching performance problem which should not be exacerbated where there is no clear motivation for further restruction. 
> 
> In the penultimate sentence, the use of the term "dirent" is incorrect.   This is a per-file-object attribute and would not have multiple copies for multiply-linked files.
> Introduction
> The penultimate paragraph does not provide any justification for tying the caching of file or directory contents to the caching of file attributes.  In particular, it is not clear:
> Why this attribute is needed (or is somehow raised to the status of a "fundamental requirement") for servers that don't implement special attribute access control outside POSIX of via "proprietary policies".
> How can one client be expected to appropraely optimize the caching/non-cacing of data by other clients?
> Definitions
> The entry for "Access-Based Enumeration" is not really a definition.  Suggest as a replacement the following:
> The return, by the server, of a limited subset of directory contents to which the requesting user has a specific level of access, somehow based on the access of that user to the underlying file.  Although not specifically provided for within NFSv4 so far,
> it is available in other protocols and its use has neither been described as valid or prohibited.  Currently, its use is treated as an acceptable server-chosen proprietary policy.
> Uncacheable Dirents
> Need to be rewritten to reflect the fact that this is a per-file-object attribute and there are no per-dirent attributes.
> Security Considerations
> Needs to be revised in light of the fact that this new attribute is OPTIONAL.  If this is truly required to effect security, which I believe not to be the case, this would have needed to be added as a REQUIRED attrbute, when ACLs were added to the protcol making their absence in v4.0 and v4.2 a defect that needs to be fixed as part of the rfc566bis effort as described in RFC8178.  In fact, the issue of the denial of attribute read permission is dealt with in rfc56661bis and dealing with it did not require a new attribute like uncacheable although there may be a need for an additional flag in aclchoices.
> Replacement Suggestions
> We deal here with the case in which there is a valid motivation for caching restructios.   In cases where no real justification is provided in the document under review (other than vague worries about caching being used where it might not be appropriate), we suggest that it needs to bedetermined whther valid jutifications exist.
> Handling of  Authorization Debial for GETATTR
> It needs to be made clear that, when a server supports the denial of GETATTR authorization for a particular fille object as provided by some forms of NFSv4 ACLs, this denial cannot be circumvented via inappropriate use of attribute caching.  By deriving this diallowance of caching based on a read-write per-file-object attribute as is done by the document under review, this is done in a coarse-grained way that has the following problems:
> It completely vitiates negative name caching even though, for directory entries not present, there are no attributes to fetch so that the motivation referred to, GETATTR autorzation denial doesnot apply.   As a result, it is undesirale to implement an attribute that could have a negative performance effect for many workloads.
> Although the document under review is not clear on this point, the naming f this attribute could beinterprested as invalidating all posiitive name caching.  Insead, it needs to be made clear that LOOKUP and READDIR always need to be properly authorized.  This is probably done in the latest security document but , if it isn't, that document needs to be corrected.  Making this dependent on the existence of  a new extension is the wong way to approach the matter since it doesn't address the issue for server with the extension and overaddresses te isssue for servers who do implement it.
> Handling of Access-Based Enumeration
> If one wants to spport Access-based enumeration, then it shold be made as as an extension including any necessary caching constraints,  In addition, there need to be an attribute allowing the client to find out whether it will be provided.   In addution, it might be desirable to allow the client to indicate its need for the feature whereit is not wanted, unless security considerations make that impossible.   Although not an issue for this feature, wide allowance of proprietary semantics is a recipe for pervasive interoprability isssues.
> 
> Control of Caching of Dirents and File Attributes
> It needs to be determined whether therare any justifucations for limitation on this sort of caching  other than those implied by acccess-based enumeration or the denial of GETATTR authorization.  The author has seen no evidence so far that tese exist while noting that thissort od caching has been used extebsively in earlier versions of NFS and was extended in NFSv4.1 with some necessary cleanuo in rfc5661bis.
> Control of Other Sorts of Data Caching.
> Besides the lack of a clear justificaton for additional restrictions on the caching of file data, it is hardto undrstand exatly what is intended by making certain file "uncacheable" since there is no discussiion of what is meant by "caching".  Every read of a file creates a copy of the contents of the file and the document under review appears to stigmatize some subset of them as constituting caching while it is inherently impossible to explain what this subset considts of.  For example, it could somehow forclose mmap but, even aparty from the confusion generated, it is a major change to the architecture to give one client the ability to define how other are to use the data that they have validly read.