chucklever / i-d-integrity-measurement Goto Github PK

Michael Halcrow pointed out to me that a checksum is used to detect (and possibly correct) a few flipped bits, but a hash is used to detect malicious behavior. The draft needs to use these terms appropriately.

Current Linux NFS server behavior is to deny access to NFS clients that try to access a file that does not pass an integrity check on the server. The server responds with EACCES. I have argued that because the server and its clients can have different IMA policies, a participating client (ie one that has an operating IMA security module and policy) would want to make its own determination. Mimi's opinion is that its always the responsibility of storage not to hand out bits it knows to be corrupt.

During IETF 105 it was suggested that a description of current (ie, unextended) behavior of Linux NFS clients and servers would be helpful to document. In particular, make it more clear how such implementations would interact with an implementation that supports the extensions described in the document.

After discussion at the North American Linux Security Summit 2019, Mimi and I concluded that exactly how an NFS server would authorize an IMA update from an NFS client (ie, SETATTR(IMA)) would be discussed in the document as implementation guidance. This is because the details of authorization are implemented as a policy on Linux, so flexibility is required here. Interoperation does not rely on the particulars of how IMA updates are authorized: only the error code if the authorization fails is relevant.

Feedback and comments on the draft “Integrity Measurement for Network File System version 4” - draft-ietf-nfsv4-integrity-measurement-07.

Note that these comments are personal contribution and do not reflect my position as working group co-chair.

In short, I am opposed to moving this draft to last call in its current state. Generally, the draft describes the Linux implementation of a feature that is to be extended via NFS. However, the description provided is insufficient for interoperable implementations to be achieved.

There are two options, in my opinion, to moving this document forward:

1. Limit the description to the addition of the new, opaque attribute and corresponding errors that can be returned as a result of the interpretation of that attribute.

2. Fully define the contents of the new attribute such that independent implementations can be achieved. This would include, but is not limited to, the open definition of the content of the new attribute and the procedures associated with defining new content and interpretation thereof.

If option 1) were chosen, I would still be of the opinion that the draft should not move forward since it would present another barrier to open, interoperable implementations.

Note that I am also doubtful of the use case being presented. Using NFS to directly store and supply application executables seems to be in rapid decline or has already fallen out of use. Given the rise of virtualization and the hosting of virtual disks on NFS along with the rise of containers and distribution thereof, application distribution seems to be a thing of the past with respect to their store/retrieval from NFS mounts.

If the effort was more focused on more traditional data integrity from source to consumption, that would be a more interesting use case. With the rise of large scale data center usage of NFS (e.g. cloud computing) where customers expect data integrity or the identification of failure, the scale of cloud computing presents many opportunities for the loss of data integrity and NFS (and the client and server implementations thereof) do nothing to ensure data integrity. The draft does mention spot fixes of data-at-rest methods, and in-transit methods, but there are many points that present areas of potential failure, and these are ignored in today’s implementations (at least to this commenter's knowledge).

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.

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

8. IANA Considerations

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

An NFS version 4.2 server needs to ensure that modifications to IMA
metadata are done only by appropriately authorized agents.

That's true but it is very difficult to do when there is no definition of "appropriately authorized agents"

Although
access to file content is typically controlled by ACLs and permission
bits, these mechanisms do not apply to IMA metadata.

First of all no reason is given as to why this is so. Note that:

• These are the mechanisms used throughout the NFSv4 protocols so it would seem that the variance would call for some explanation.
• Allowing independent decisions about whether to allow a write to a file and when to allow setting its IMa metadata allows the possibility that someone will mliciously set the IMA Metadata. While the cryptographic nature of the signature prevents signing of an inappropriately modified file, the possibility exists that a malicious actor could cause a denial of service by storing an inappropriate/invalid signature.
• It is unclear whether the phrase 'these mechanisms do not apply" is intemded normatively ot not. If it isn't, then there is no nornative guidance on this point whatever. On the other hand, if it is normative, then it is bizarre that the one factor that the server is not allowed to consider in deciding whether this is being done by an "appropriately authorized agent" is one that everyone else in NFSv4 uses to make these sorts of decisions.

The question of "who is authorized to modify IMA metadata" is often
left to the server's local IMA security policy.

Perhaps so, but without information about such security policies, this isn't all that helpful.

In addition, the
issue of whether to allow a particular IMA metadata update has no
bearing on protocol interoperability, as long as the server sticks to
returning NFS4ERR_ACCESS or NFS4ERR_INTEGRITY, as appropriate.

I don't agree. If the client tries to do an operation and is not allowed to do it, then you have an instance of inter-non-operability :-( Also, if the client has no indication of why this failed (wrong user, wrong client, wrong something else) there is no way to decide how to work around the failure.

Thus,
to enable server implementation flexibility,

It has to be recognized that server flexibility is not an unalloyed good, and that in granting such a large degree of fleixibility (almost unbounded) to the server, you basicallly undercut the value of defining a protocol, to provide interoperability.

the current document
treats the following recommendations as implementation guidance
rather than as normative protocol requirements.

If these are not normative requirements, then there are no normatve requirements and I don't see how you can get by saying essentially that the server is free to accept or reject requests as it chooses.
Also, although the word "recommendations" is used, ther are no actual recommendations to make any particular choice as better than another. As to "implementation guidance" what is the implemenation being guided to do?

Possible NFS server implementations include limiting IMA metadata
update authority in the following ways:

The use of the word "include" suggests that other ways are allowed/recommended or part of what you are guiding the server to do. These include some silly ones (e.g. do not allow the change of ima metadata in a month that does not include an "r") and some that are not silly at all (e.g. do not allow the change of IMA metdata when done by an unauthenticated client using AUTH_SYS).

Particular users
A server might allow IMA metadata updates only by UID 0 or by a
client's machine principal.

If applied to an AUTH_SYS client, this constitutes a glaring security hole. I think this would be Ok, for non-auth-sys as long as NFS4ERR_WRONGSEC is returned if AUTH_SYS is used.

Particular clients
A server might allow IMA metadata updates only from specific
client IP addresses.

Since the source IP addresses can be spoofed, the value of this is limited.

File owners
A server might allow IMA metadata updates only by the file's owner
or group owner.

I think this would be OK, as long NFS4ERR_PERM is returned when the owner is wrong.

No remote updates
A server might always return NFS4ERR_ACCESS when an NFS client
sends a SETATTR request that updates IMA metadata.

If a server is not allowing SETATTR of this attribute, it is really treating this as a get-only attribute, in which case NFS4ERR_INVAL would be the appropriate error to return.
Since each of the above clearly has ann existing constituency, making intra-community comporomise difficult to achieve, let me try to articulate my bottom line:

• If we have to live with these four approaches, then OK :-( but I don't think we can live with an infinite number of possible approaches.
• These restrctions have to be in addition to the ability to write the file in question, rather than as a potential replacement.
• It needs to be clear why a request was rejected, either due to a distinct error code or some sort of fs-wide policy attribute.
• If there are existing servers that do something bogus (e.g. accept an AUTH_SYS uid zero as determinative), then that fact can be noted but we should not recommened or in any way guide implementations to adopt such dubious practices.

I constructed a use case for my presentations (at IETF 105 and the Linux Security Symposium) that seemed to help audience members grasp the work. Add a cleaned-up English description of this use case to the document's Introduction.