Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dave Noveck's review of draft-ietf-nfsv4-integrity-measurement-07 #8

Open
chucklever opened this issue Nov 4, 2019 · 0 comments
Open
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@chucklever
Copy link
Owner

General Comments

Overall Evaluation

This document is in good shape and I feel it is ready for Working Group Last Call. While I note some potential improvements below in Per-section Comments, I feel these changes could reasonably be made as part of last call rather then needing to be be done before that time. However, if there are plans for a -08, I feel it would simplify things if some of the suggestions made here were considered for inclusion in -08

Possible Overuse of the Word "MUST"

I feel that the use of "MUST" within this document is not in accord with the advice given in RFC2119 that these terms be used "sparingly". Although I haven't looked at each such use, where I suggest rewriting for other reasons, I would only use "MUST" where there is a special reason to do so.

Issues Related to Documentation of IMA Metadata Format

There has been a lot of discussion/comment on the mailing list regarding the fact this format is not documented in this draft. Some of the suggestions made in Per-section Comments are intended to make it clearer why this has not been done. While I feel this would be helpful, I have no expectation it will resolve the objections that have been expressed, which I don't fully understand.

While I do not think that this issue should pose any obstacle to publication, it might make it hard for the working group to arrive at a consensus for publication. Nevertheless, I feel we need to move to WGLC and try to resolve the issue in that context, as I feel that discussion on the list so far has not been all that helpful.

Per-section Comments

Abstract

Suggest addition of the following sentence: "The format of the IMA Metadata is not described in this document."

1.1. The Linux Integrity Measurement Architecture

In the first paragraph, suggest replacement of the phrase "local tinkering" by "other local modifications"

1.1.1. IMA Metadata

The last paragraph of this section, while a correct description of the current situation may give readers pause since it suggests the possibility of an interoprerability nightmare. Even though such a scenario is, strictly speaking, out of scope, I believe it would be helpful to assure people that steps are being taken to avoid it, even though those steps are being taken by others. For that reason, I am proposing replacing that parapraph by the following two paragraphs.

The precise format of this metadata is currently determined by policies set by the local security administrator. The metadata and its format are opaque to the mechanisms that store or transport it (i.e. filesystems). The particulars of the PKI and the hash algorithm are set by local policy. In order to provide interoperability, there is a need to provide agreement on these matters by an out-of-band mechanism so that the IMA data can be recorgnized by all participating IMA subsystems.

The difficulties which arise when multiple formats are supported makes it likely that a sinlgle format will be arrived at, avoiding the need for an out-of-band agreement mechanism. However the details of that format and the means by which it will standardized remain uncertain at this tiime.

  1. Protocol Extension Considerations

In the last clause of the last sentence of the last paragraph suggest replacing "does not update [RFC7862] or [RFC7863]" by "updates neither [RFC7862] nor [RFC7863]"

4.1.1. NFS4ERR_INTEGRITY (Error Code YYYYY)

Suggest that the following additional paragraph be added to this section:

This error provides a means by which servers which implement an internal apprasal mechanism may communicate to the client the fact that an appraisal failure occurred, causing the requested operarion not be be performed.

4.2.1. Reporting Server-Side IMA Appraisal Failures

Suggest rewriting the first paragraph to read as follows:

An NFS server that implements an internal appraisal mechanism needs to be able to report integrity-related failures to clients. In the absense of the NFS4ERR_INTEGRITY error, described in Section 1.1,1, a server implementer would be forced choose choose one of the status codes that were available in the base NFS version 4.2 protocol, typically NFS4ERR_IO or NFS4ERR_ACCESS, even though these code points have generic meanings that do iindicate an integrity-related failure.

4.3.2. Authorizing Updates to IMA Metadata

As written, the second pragraph, does not make any provision for the possibility that update of IMA Metadata would be supported for some but not all of the file system on a particular server. If it is desirable to allow this possibility suggest rewriting this paragraph as follows:

If an NFS server implementation does not support modification of IMA metadata via NFS within a particular file system, the server returns NFS4ERR_INVAL to a SETATTR request within that file systen with the FATTR4_IMA attribute, as specified by Section 5.5 of [RFC5661].

4.5. Using NFS Attribute Fencing (VERIFY/NVERIFY)

Suggest rewriting the second paragraph as follows:

When implementing these operations, the server is to use a simple byte-by-byte comparison to evaluate whether the client-provided FATTR4_IMA matches the FATTR4_IMA attribute associated with the target object. If the server has a local IMA appraisal implementation, its failure MAY prevent the use of the local FATTR4_IMA attribute value for the purpose of this comparison. In the event that this happens, the appraisal failure is indicated by returing NFS4ERR_INTEGRITY if the client has indicated support for IMA metadata, with NFS4ERR_ACCESS used otherwise.

  1. IANA Considerations

Suggest replacing "has no" by "does not require any"

@chucklever chucklever added bug Something isn't working enhancement New feature or request labels Nov 4, 2019
@chucklever chucklever self-assigned this Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant