[nfsv4] Review of draft-ief-nfsv4-integrity-measurement-04

David Noveck <davenoveck@gmail.com> Sun, 19 May 2019 09:41 UTC

Return-Path: <davenoveck@gmail.com>
X-Original-To: nfsv4@ietfa.amsl.com
Delivered-To: nfsv4@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E9E3D12007A for <nfsv4@ietfa.amsl.com>; Sun, 19 May 2019 02:41:14 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.999
X-Spam-Level:
X-Spam-Status: No, score=-1.999 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ollbPkg4jY1p for <nfsv4@ietfa.amsl.com>; Sun, 19 May 2019 02:41:11 -0700 (PDT)
Received: from mail-ot1-x32b.google.com (mail-ot1-x32b.google.com [IPv6:2607:f8b0:4864:20::32b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 9BF4212002F for <nfsv4@ietf.org>; Sun, 19 May 2019 02:41:11 -0700 (PDT)
Received: by mail-ot1-x32b.google.com with SMTP id l17so10489688otq.1 for <nfsv4@ietf.org>; Sun, 19 May 2019 02:41:11 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to:cc; bh=e2kyp9GEyM2t/qNOw882PgRGrIqyrDTBTKjLPd031uY=; b=cEWRVNvde1XUGcMD7gIPyXUDbVfeSDvS+ljyYXeRCt8kLabHOzrOyLGLW7VuefNDa9 i68CrxB2eng71A4KRffGND0Dzn1AvD94DO4N4DrEKqkOmH4T7t9PvZ6D8eghMus7osZF njmNsUsupJiBUW7pSJTrzzsSGlVDl6chE9VZnjXvHYHIWHQaLL5s0X81I4UIngWSX8k5 qRMfRhs+xGW+ojBImVJZ5P0TIt3kR6vj1/3PaqQiyPZbOcH13OzWc3PL4Zcly7gn3Ti1 ULsdhcFThc8yF9vvIp1UeD32htQl7AqzmHFGW5eJ+XUS54jsXlVIiSK/AQaEREXL+mCL oqwg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=e2kyp9GEyM2t/qNOw882PgRGrIqyrDTBTKjLPd031uY=; b=Dw6+uyQA6c7oiSqF6eBQLmlFMhni5bokO1DJ588Xom+YUAmXtRMqUcJuMyLXyCnvFp A+4kxMLfdUbpkZYEks2tGUnSTIOe/2NyvDT2PlPuavkuFJfyGEifIFmmW3m7mMs0mpD+ W1cgAoFjiWhWNE7TJex1drRcxwJ6OAYR/gKsKatv2YvHOo/JzlgKSi4nxubhsxASuROj vV5/CMrzEdXi9fT7lDqZUMMlD8GhgZsSR0y1cnCG5s8l1aVp+TSR0gEbqxWO+4xva8S1 Yu/s04kfuI4pa4DYRNILfMHadfbJhmRnOFD/0i9ODI5TJorW3ktIdGh6TPNK8slFKK7v 5/pQ==
X-Gm-Message-State: APjAAAU/0Fm6fHh7vb1Vox7r1agY0hkB2/wTyMLiIhzVuy57YzFpXwWw sInSTMS9zpV7ArBzU1nf9FVWvGhNrGDNG9krVWU=
X-Google-Smtp-Source: APXvYqwS4IeUY3fbRXR1e7RogpTA2NullztMbqWwX+uidbml0kb45spDok9b3dJmL9quJ8KdWSpyg9cY4hhbqNGluZo=
X-Received: by 2002:a9d:4809:: with SMTP id c9mr2977otf.208.1558258870751; Sun, 19 May 2019 02:41:10 -0700 (PDT)
MIME-Version: 1.0
From: David Noveck <davenoveck@gmail.com>
Date: Sun, 19 May 2019 05:41:01 -0400
Message-ID: <CADaq8jc2FoNEYHp282hxjY3EnWH7qQhF=WHomp+W9O5qf85USw@mail.gmail.com>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: NFSv4 <nfsv4@ietf.org>
Content-Type: multipart/alternative; boundary="00000000000075991a05893a6a55"
Archived-At: <https://mailarchive.ietf.org/arch/msg/nfsv4/mOVinx_41zxIi6LDprJwuqzroxA>
Subject: [nfsv4] Review of draft-ief-nfsv4-integrity-measurement-04
X-BeenThere: nfsv4@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NFSv4 Working Group <nfsv4.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/nfsv4/>
List-Post: <mailto:nfsv4@ietf.org>
List-Help: <mailto:nfsv4-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 19 May 2019 09:41:15 -0000

*General Comments*

*Context of Review*

Development of this document has gone on for a considerable time and it
appears to have reached something close to its eventual final form.
 Therefore it is appropriate consider the path forward and specifically
what would be necessary for the document to be ready for WGLC.  That issue
will be the focus of this review.

*Overall Evaluation*

This document is in pretty good shape but I feel that there are still a few
matters, beyond the minor editoral issues that consitutute the bulk of the
material in *Comments by Section, *that need tro be dealt with before WGLC:

   - I feel that some editorial work needs to be done in being more
   explicit about the basic  approach taken in this document.  There have, in
   the recent past, been concerns expressed about this approach and feel it is
   quite possible that such concerns will be raised as part of WGLC.   These
   issues are discussed in *Potential Concerns about the Basic Approach* but
   I don't feel that there is anything that the author can reasonably do to
   address them, partly because no specific issues have been raised, beyond
   the dissatisfaction/unease already mentioned.  Instead, as discussed below
   in *Clarity about Goals and non-Goals,* I feel it would be best if the
   author was more explicit about the limited goals of this document,
   continuing an existing trajectory from -00 to -04.
   - Although, for the most part, the current document follows the model of
   treating the IMA metadata as uninterpreted data whose format (beyond a
   necessary length limit) is not its concern, there are some areas where that
   division of labor is not strictly maintained, which poses issues for server
   implementers.   For further discussion of these areas, see *Issues for
   Server Implementations.*

*Potential Concerns about the Basic Approach*

The major area of potential concern centers around the fact that there is
likely to be only a single client implementation in that the document does
not  provide sufficient information for a client to be written and there is
no point for anyone to write another client implementation.   This can be
accurately described as a Linux-only feature and there is no point in
having multiple competing implementations unless one has gone horribly
wrong.

On the other hand, the document generally provides sufficient information
to allow multiple server implementations to be written and tested against
the Linux krnel client implmentation being written.

This is an atypical pattern for the working group and it is probably an
atypical one for the IETF as a whole.   During working group last call, it
is likely that concerns traceable to this issue will be raised as they have
in the past.   It is important that we have a consensus on the fact that we
want to do this although it is not necessary to have a consensus on why we
are doing it, as people may well differ on why we are doing this and thus
we may might not  have a policy about such one-client-only features going
foward.

*Clarity about Goal and non-Goals*

As this docment has been developed, there has been a shift from an earlier
state in which there were somewhat half-hearted attempts to provide for the
posssibility of other clients to the current state in which it is qiuite
clear that this is, as descibed in *Potential Concerns about the Basic
Approach* a one-client-only feature.   I believe it would be helpful to be
more explicit about that fact in a number of places, so that the document
can address the concerns of those who have trouble with that approach
head-on.

As I understand things, the Linux IMA metdata is essentally, from the
server's point of view, a piece of uninterpreted data, just as user data
and extended attributes are, although there are  a few remaining issues
discussed in *Issues for Server Implementations* that need to be
addressed.  However, unlike other sorts of user data, this does have an
internal structure known to the client implentations and not documented in
this document.   That information would not be needed in a one-client-only
implementation but it is typically provided in feature descriptions, so it
would be best if this point is mde explicitly.

*Issues for Server Implementations*

The following issues are discussed in more detail in* 5.2.1.  Authorizing
Updates to IMA Metadata:*

   - As described above, the basic architecture is that the server is to
   treat IMA metadata as uninterpreted data, yet there are some indications
   that this is not always so.
   - The issue of the proper privilege to update IMA metadata needs to be
   clarified.

*Comments by Section*

*Title*

Consideration should be given to including the word "Linux" in the title,
since this is being defined as a Linux-only feature.

*1.  The Linux Integrity Measurement Architecture*

In the second sentence of the first paragraph, I'm wondering about the
adjective "primary".  Since I'm having trouble imagining what a secondary
goal might be, I wonder if "primary" should be deleted.

With regard to the penultimate paragraph:

   - I don't understand the "therefore" in the second sentence.  I'm clear
   that the format is not described in this document, and it does not need to
   be since servers don't need it and there is only one client
   implementation.  I think we need more clarity about whether the format
   cannot be described in this document or whether it could but is not present
   because it is unnecessary.
   - In the third sentence suggest replacing "NFS clients" by "Linux NFS
   clients."
   - Suggest adding a final sentence such as the following :

Information about the format of IMA metadata is not needed by server
implementation as this data can be treated just as is other data whose
format is defined by others (e.g. user  data and extended attributes).


*4.   Managing IMA Metadata on NFS Files*

While I haven't found many issues with the material in the various
subsections of this section, the material is incomplete in that it does not
deal with all possible use of the IMA metdata attribute in
attribute-related operation but only in those that the author blieves
(probably correctly) are likely to be used.   In particular,

   - There is no mentiion of VERIFY/NVERIFY.   It is not clear if they are
   considered invalid, but I'm assuming they aren't.
   - There is no mention of the specfication of attributes in CREATE.   It
   needs to be specfied if this is valid or not.
   - In sections 4.2 and 4.3 the text seems to assume that the IMA metadata
   attribute will not be set/interrogated together with other attributes.

One other issue that applies to multiple subsections concerns the phrase "but
the object itself does not support the FATTR4_LINUX_IMA attribute". It
needs to be defined what types of objects are to have this support.
Otherwise, you have a Total Interoperability Nightmare (tm :-).

*4.3. Retrieving IMA Metadata*

In the third paragraph, there is a stray reference to the
FATTR4_FILE_PROVENANCE
bit which needs to fixed.

*5.2.1.  Authorizing Updates to IMA Metadata*

There are a number of issues in this section where sufficient information
is not given for  a server implementation to be written or there is a
reference to the possibility that the server will implement behavior based
on knowledge not provided in this spec, as mentioned above in  *Issues for
Server Implementations*.   Given the current scope of the problems, I'll
reproduce the existing section in italics and respond to each element that
I believe raises problems:

*A participating server should ensure that modifications to IMA
metadata are done only by appropriately authorized agents.*

That's unexceptionable, but the problem is that the server is given no way
to determine exactly when a request is made by an "appropriately authorized
agent". As ACLs are the primary means by which authorization decision about
actiions on files are determined in NFSv4, it would be best if the
specfication told how the file's ACL could be used to make this
determination.

*Such agents usually include only agents with super-user privileges. *

Since this imples that there are also unusual situations, the "usually" is
purely informational. Again, what is needed is a specfication of how a
server is to make the authorization. The obvious choice is to allow this to
be set iff the file data can be written but if it were that simple, the
spec would simply say that. If it is something else, what is needed is:

   - A specification of that other approach.
   - An explanation of why the obvious choice is not OK.

*The NFS server MAY confirm that the new IMA metadata actually
verifies the file content correctly before storing it.*

There are a number problems with this:

   - It contradicts the basic idea that the server is to treat IMA metadata
   as uninterpreted, as it does user data, I for one, would not want an NFSv4
   server to reject a WRITE of a .c file because it didn't like my comment
   indenting :-)
   - Since the format is not defined in this document, it is basically
   saying that the client that the server "MAY" refuse a request to store IMA
   metadata pretty much whenever it feels like it and that clients have to
   deal wit that.
   - There is no specification, either here or in Section 4.2, of what
   error SETATTR is to return in this event.

If this is necessary to accommodate Linux file systems that are aware of
the format of this data, enforce it, and cannot be changed to treat this
data according to the basic architecture treating this as uninterpreted
data, consideration should be given to making this a "SHOULD NOT".

*7.  Security Considerations*

The last paragraph has the following issues:

   - It ignores the fact the denial-of-service triggered by mismatch of
   data and IMA-metadata can result from changing the data as well as changing
   the IMA metadata.
   - It doesn't define "suitable level of pivilege".
   - It refers the reader to section 5.2.1 for details while that section
   has no useful details.