<comments inline> On 6/20/12 11:53 AM, "James Lentini" <jlent...@netapp.com> wrote:
> >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 <rbar...@bbn.com> >To: ietf@ietf.org, The IESG <i...@ietf.org>, > draft-ietf-nfsv4-federated-fs-ad...@tools.ietf.org, gen-...@ietf.org >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. > 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. > >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" Thanks, we've been struggling with this issue, I'll take your suggested change here. > >_______________________________________________ >nfsv4 mailing list >nf...@ietf.org >https://www.ietf.org/mailman/listinfo/nfsv4
