Re: [kitten] [nfsv4] draft-ietf-nfsv4-rpcsec-gssv3: request for review

[Nico Williams <[e-mail]>]

On Thu, Jul 31, 2014 at 06:38:09PM -0400, Benjamin Kaduk wrote:
> The union construct in 2.6.1 with the default case being an opaque
> is quite similar to the extensible union construct defined in
> draft-keiser-afs3-xdr-union; the only difference is that the
> encoding of the currently listed 'LABEL' and 'PRIVS' cases do not
> include a length field.  It might be nice to have afs3 and nfsv4

They don't need to.  They are struct types, and may (do) have lengths
embedded.

> agree on a consistent type for the extensible union primitive,
> though I understand that this is unlikely to happen on the timescale
> you desire for the rpcsec-gssv3 draft.

We should agree as to data model for this, but not necessarily as to
encoding.  NFS uses XDR.

> ---
> It feels a little strange to be talking about adding a way to
> specify privileges/restrictions via "assertions" onto a GSS-based

But that's what they are: the client asserting that some process local
to it is running with some privilege.  There's no way to authenticate
this as we don't and could not possibly have a credential for every set
of {user, <privs>}.

> security scheme, since GSS is philosophically more of a "request and
> check" scheme for features.  This is not a realy problem, of course,

This isn't GSS though.  It's RPSEC_GSS.  We could use GSS with naming
attributes to convey everything.  And a stackable GSS mechanism to do
compound authentication.  However, stackable GSS mechs went nowhere.

> it just takes some getting used to.  Actually, I guess it is still
> kind of "request and check", since the server only replies with the
> assertions it has accepted. Maybe this is indicative that the name
> could be more descriptive.

They aren't assertions in the send of a programming language "assert".
They are the client's assertions about local conditions, which the
client and only the client can know about.  The server's job is to
decide whether to accept those assertions considering all the context at
hand, including the compound authenticated principals.

> ---
> This draft makes no mention of the use of the 'critical' bit for
> structured privilege assertions, but does imply that the critical
> bit will apply to any future branches that are added to the
> rgss3_assertion_u. This strikes me as odd; at the very least it
> could say that the critical bit is ignored.  (That's my reading of
> what the behavior is supposed to be, from section 2.6.1.3.)

Criticality can always be decided by the client based on what the server
accepts, I suppose, so we could probably remove either the critical bit
or the echoing of accepted assertions in the reply.  I would rather
remove the critical bit than the latter.

> ---
> It seems like it would not adversely affect the document to move the
> definition of rgss3_chan_binding up to between rgss3_gss_mp_auth and
> the branches of rgss3_assertion, to match the layout of the
> rgss3_create_args structure.  Kind of minor, but helps the reader
> find things.

Channel binding is a lesser thing now that we never got RFC5660 widely
implemented (or at all).

> ---
> I think it's valid to assume that a successful call to GSS_GetMIC
> will never return a zero-length output, so the encoding of

The GSS-API never produces zero-length tokens (RFC2743 says this).

> rgss3_create_{args,res} could be reduced in size by just using an
> opaque for their respective _chan_bind_mic fields, instead of
> including the extra pointer in the type.  I don't know if there's
> enough need for space to make this worth doing, though.  There is
> some potential value in being able to easily differentiate "not
> present" from zero-length.

Good (if minor) point!

> ---
> Why is RPCSEC_GSS_BIND_CHANNEL marked as "not used" in the sample
> code on page 7?  It is not otherwise mentioned in the draft, and the

Dunno.  It was never marked so in my draft.  Andy?

> ---
> And now, the minutiae:
> 
> The phrasing in section 2.3 that "RPCSEC_GSS version 3 MUST change
> the verifier" seems odd.  This is the specification of RPCSEC_GSSv3,
> [...]

Also not in my draft.

> In 2.6.1.1, the following text is a little confusing:
>    Thus a server may refuse to
>    grant requested authority to a user acting alone (e.g., via an
>    unprivileged user-space program), or to a client acting alone (e.g.
>    when a client is acting on behalf of a user) but may grant requested
>    authority to a client acting on behalf of a user if the server
>    identifies the user and trusts the client.
>
> It makes more sense when one reads "client" to mean "in-kernel NFS
> client", but would probably be more clear if the "client is acting

This is tricky.  What's a "client"?  The TCP client?  The physical
hardware running it and the NFS client stack?  The process running the
NFS client?  Does it matter if it's in-kernel or a FUSE process, or some
micro-kernel thing?  For me the only thing that matters is if it has
access to GSS credentials that the server will understand as being a
"client's" as opposed to a "user's".

Wording this is hard, yes.

> on behalf of a user" is reworded, possibly to "on behalf of a single
> user, using only that user's credentials).  It looks like the

That's more verbose!  Yes, we need to distinguish "user with
credentials" from "user without credentials" (since identity assertions
are partly about the latter), but I think that should be clear anyways.

> "client" vs. "kernel service" distinction is fuzzy throughout the
> rest of the section, too.  I would prefer if the word "client"

We should absolutely not refer to "kernel" -- that's just an
implementation detail.  Instead we should refer to a "multi-user client"
or something of the sort.

> throughout the document referred only to "the RPC client", without
> any implications about whether the RPC client is a trusted

Yes, we could probably qualify "client" at every step, but it's not
clear what other qualifiers to use when referring to "the OS", the
"client service" (ah, maybe that's a good one), and so on.

> (in-kernel) service or a program run by the user or anything else.
> This document defines RPCSEC_GSSv3, not NFS; "client" should mean
> "RPC client", not "NFS client".

True, NFS is not the only application that could use RPCSEC_GSSv3, but
it's very likely the only one for a long time that will.

> Still on multi-principal authentication, the text says "Other
> multi-principal parent and inner context handle uses might
> eventually make sense."  It seems like any such uses would require
> standards action to specify them; you might as well also say that
> they would be introduced in a new revision of the RPCSEC_GSS
> protocol (e.g., v4).

Sure, but that's implied.  It's a forward-looking statement.  (GSS did
the same w.r.t. channel binding.)

More later.

Nico
--