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"