Subject: [nfsv4] Gen-ART LC review of draft-ietf-nfsv4-federated-fs-admin-11 (fwd)

From: James Lentini <[email-masked]>
Date: Wed, 20 Jun 2012 12:53:36 -0400 (EDT)

Below is the Gen-ART review comments for the FedFS admin protocol.

-james

---------- Forwarded message ----------
Date: Tue, 19 Jun 2012 11:31:42 -0400
From: Richard L. Barnes <[email-masked]>
To: [email-masked], The IESG <[email-masked]>,
    [email-masked], [email-masked]
Subject: Gen-ART LC review of draft-ietf-nfsv4-federated-fs-admin-11

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-nfsv4-federated-fs-admin-11
Reviewer: Richard Barnes
Review Date: Jun-19-2012
IETF LC End Date: Not known
IESG Telechat date: Jun-28-2012

Summary: Almost ready, couple of issues

MAJOR: 

4. / 7.
This protocol allows for the provisioning of security information as part of the FedFsNsdbParams, in the form of a certificate to be used in verifying a TLS server certificate.  There are two notable issues with how the document does this now:
1. Section 4 does not adequately specify how the certificate in the FedFsNsdbParams should be used in the verification process.  Is it to be used as a trust anchor, so that any subordinate certificate is considered valid?  Or perhaps it is to be matched against the end-entity certificate.  In either case, it seems like it would be sufficient to provision a certificate fingerprint (or even a public key fingerprint) instead of the whole certificate.
2. In the security considerations, the document should discuss the implications of provisioning trust information in this way (depending on how it is used), and any requirements for security mechanisms to be used in these cases.

MINOR:

2. 
It's not clear what the difference is between utf8string_cs and utf8string_cis.  Should you mention at some point that these MUST be UTF-8, even though the XDR won't enforce that?  (Say, at the beginning of Section 4?)

3.
It's not clear to me how FEDFS_ERR_PERM differs from FEDFS_ERR_ACCESS.  I don't see it called out for use anywhere in the procedure calls.  Perhaps this is a legacy of prior versions that should be deleted?

EDITORIAL: 

3. 
The definition of FEDFS_ERR_LOOP isn't actually related to looping.  It seems like this should either detect a loop (e.g., via a repeated name), or be renamed something like FEDFS_ERR_TOOMANYREFERS.

4. "The port is the NSDB transport port for the LDAP interface"
Suggest rewording to clarify that this is an LDAP/TCP port (I initially wondered if other transports could be used), e.g.:
"The port value in the FedFsNsdbName indicates the LDAP port on the NSDB"

---

From: "Haynes, Tom" <[email-masked]>
Date: Fri, 22 Jun 2012 15:06:14 +0000

<comments inline>

On 6/20/12 11:53 AM, "James Lentini" <[email-masked]> wrote:

>This protocol allows for the provisioning of security information as part
>of the FedFsNsdbParams, in the form of a certificate to be used in
>verifying a TLS server certificate.  There are two notable issues with
>how the document does this now:
>1. Section 4 does not adequately specify how the certificate in the
>FedFsNsdbParams should be used in the verification process.  Is it to be
>used as a trust anchor, so that any subordinate certificate is considered
>valid?  Or perhaps it is to be matched against the end-entity
>certificate.  In either case, it seems like it would be sufficient to
>provision a certificate fingerprint (or even a public key fingerprint)
>instead of the whole certificate.
>2. In the security considerations, the document should discuss the
>implications of provisioning trust information in this way (depending on
>how it is used), and any requirements for security mechanisms to be used
>in these cases.
>

I'm going to have to work with the rest of the group to answer this one.

>
>MINOR:
>
>2. 
>It's not clear what the difference is between utf8string_cs and
>utf8string_cis.  Should you mention at some point that these MUST be
>UTF-8, even though the XDR won't enforce that?  (Say, at the beginning of
>Section 4?)

Argh, we did away with utf8string_cs and utf8string_cis in 3530bis.

utf8string_cis should be utf8val_REQUIRED4
utf8string_cs should be ascii_REQUIRED4

In 3530bis, these are defined as:

typedef :opaque  :utf8string<>:UTF-8 encoding for strings.
typedef :utf8string:utf8_expected:String expected to be UTF-8 but no
validation
typedef :utf8string:utf8val_RECOMMENDED4:String SHOULD be sent UTF-8 and
SHOULD be validated
typedef :utf8string:utf8val_REQUIRED4:String MUST be sent UTF-8 and MUST
be validated
typedef :utf8string:ascii_REQUIRED4:String MUST be sent as ASCII and thus
is automatically UTF-8

I'll change the types and there is already some text pointing the reader
to Chapter 12 of 3530bis.

>
>3.
>It's not clear to me how FEDFS_ERR_PERM differs from FEDFS_ERR_ACCESS.  I
>don't see it called out for use anywhere in the procedure calls.  Perhaps
>this is a legacy of prior versions that should be deleted?

This is inherited from the base NFSv4.0 documentation and also NFSv3. It
is also the difference between EPERM and EACCES. (see
http://www.wlug.org.nz/EACCES)

13.1.6.1.  NFS4ERR_ACCESS (Error Code 13)

   Indicates permission denied.  The caller does not have the correct
   permission to perform the requested operation.  Contrast this with
   NFS4ERR_PERM (Section 13.1.6.2), which restricts itself to owner or
   privileged user permission failures.

13.1.6.2.  NFS4ERR_PERM (Error Code 1)

   Indicates requester is not the owner.  The operation was not allowed
   because the caller is neither a privileged user (root) nor the owner
   of the target of the operation.

>
>
>EDITORIAL: 
>
>3. 
>The definition of FEDFS_ERR_LOOP isn't actually related to looping.  It
>seems like this should either detect a loop (e.g., via a repeated name),
>or be renamed something like FEDFS_ERR_TOOMANYREFERS.

This has its roots in a Linux error code:

>> As I recall this error is supposed to be returned when fedfsd is passed
>>a pathname that contains too many symlinks.  The equivalent errno on
>>Linux is called ELOOP.

We are striving to maintain parity.

>Suggest rewording to clarify that this is an LDAP/TCP port (I initially
>wondered if other transports could be used), e.g.:

Thanks, we've been struggling with this issue, I'll take your suggested
change here.

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