Subject: [nfsv4] comments on draft-williams-rpcsecgssv3-00

From: "Mike Eisler" <[email-masked]>
Date: Tue, 7 Apr 2009 13:01:21 -0700 (PDT)

the server issues a "child" RPCSEC_GSSv3 handle, but the
      underlying GSS-API security context for the parent handle is used
      in all subsequent exchanges using the child handle (this works
      because the RPCSEC_GSS handle is included in the integrity
      protected RPCSEC_GSS auth/verifier header for all versions of
      RPCSEC_GSS)

Actually the verifier is not integrity protected; the verifier
contains the MIC. But more importantly, the handle is not included
as the input to the MIC that is stored in the verifier of the response.
This a serious problem (and not just for RPCSEC_GSSv3; the SSV
mechanism in NFSv4.1 has a similar issue). The verifier of RPCSEC_GSSv[12]
replies computes a MIC on just the sequence number Since a child and
parent RPCSEC_GSSv3 handle could have the same sequence numbers, this
will provide opportunities for man in the middle attacks.

This is easily addressed: RPCSEC_GSSv3 MUST change the verifier of
the reply to compute the verifier use the exact same input as
that used for verifier of the request, except artificially
change mtype from CALL to REPLY:

   unsigned int xid;
   msg_type mtype; /* set to REPLY */
   unsigned int rpcvers;
   unsigned int prog;
   unsigned int vers;
   unsigned int proc;
   opaque_auth  cred; /* captures the RPCSEC_GSS handle */

----------------

     This means that RPCSEC_GSSv3 depends on RPCSEC_GSS versions 1 and/or
   2 for actual GSS-API security context establishment.  This keeps the
   specification of RPCSEC_GSSv3 simple by avoiding the need to
   duplicate the core functionality of RPCSEC_GSS version 1.

Given that we need to change how the verifiers of replies are calculated,
this comment is somewhat moot. Anyway speaking as one who has implemented
RPCSEC_GSS, using one version for some RPCSEC_GSS procedures, and
another for another set of procedures is going to be pain. In RPCSEC_GSSv2,
we started the convention (which follows NFSv4.x convention), that
each version is a proper superset.

           enum rpc_gss3_proc_t {
                   RPCSEC_GSS3_DATA = 0,
                   RPCSEC_GSS3_LIST = 1,
                   RPCSEC_GSS3_CREATE = 2,
                   RPCSEC_GSS3_DESTROY = 3
           };

A couple points. First All the I-Ds on the NFSv4 WG's RFC editor queue
use self extracting XDR, so something similar should be done here
for RPCSEC_GSSv3. See http://tools.ietf.org/html/rfc5403. Eventually,
the license used in RFC5403 will need to be added. (And yes I know
the copy I-D doesn't do any of this yet, but I figure turnabout
nit picking is only fair :-).

Second. RPCSEC_GSSv2 has:

   ///   enum rpc_gss_proc_t {
   ///     RPCSEC_GSS_DATA          = 0,
   ///     RPCSEC_GSS_INIT          = 1,
   ///     RPCSEC_GSS_CONTINUE_INIT = 2,
   ///     RPCSEC_GSS_DESTROY       = 3,
   ///     RPCSEC_GSS_BIND_CHANNEL  = 4 /* new */
   ///  };

_LIST and _CREATE conflict. They should be numbered at 5 and 6.
Otherwise this create unnecessary challenges for packet decoders,
both the software and wetware varieties.

           struct {
                   rpc_gss3_proc_t         gss_proc;
                   unsigned int            seq_num;
                   rpc_gss_service_t       service;
                   unsigned int            handle_version;
                   opaque                  handle<>;
           } rpc_gss_cred_vers_3_t;

RPCSEC_GSSv[12] use:
 ///  struct rpc_gss_cred_vers_1_t {
   ///    rpc_gss_proc_t    gss_proc; /* control procedure */
   ///    unsigned int      seq_num;  /* sequence number */
   ///    rpc_gss_service_t service;  /* service used */
   ///    opaque            handle<>; /* context handle */
   ///  };

I don't see the point of handle_version. There is this remark in
in the prose:

   It differs from rpc_gss_cred_vers_1_t in that:
   a) the values for gss_proc corresponding to control messages are
   different, b) the presence of a field indicating the version of
   RPCSEC_GSS used to established the context handle used, if any.

What problem does this solve? I suggest deleting the verson. That
will simplify the implementation.

BTW the above prose was rather distant from the new credential.
It would be easier to read if the type defs were immediately
followed by prose.

   o  another RPCSEC_GSS (version 1, 2, or 3) context handle

Keep it simple: another RPCSEC_GSSv3 context handle.

           struct {
                   int     type;
                   bool    critical;
                   opaque  data<>;
           } rpc_gss3_extension;

The convention of current NFSv4 I-Ds is to prefix fields with
an abbreviation of the type definition. I'm pretty sure if I
use cscope to search for "data", "type", or even "critical" in
a type operating system's kernel, I will find thousands of hits.
Whereas, rge_type, etc. is likely to have fewer conflicting hits.

Anhyway, type, critical, and data are not defined else where in the spec.
There are similar issues with other fields of other type defs, and
so understanding of the spec is a challenge.

More examples of how to use the label, doi, privilege, assertion stuff
are needed. REQUIRED instances of each are needed, or a statement
that a particular ONC RPC protocol MUST state the REQUIRED instances
is needed (similar to how in RFC2203, we skirted the REQUIRED mechanism
issue by requiring that protocols using RPCSEC_GSS MUST
state REQUIRED GSS mechanisms). I would expect an I-D that states
the REQUIRED labels, dois, privileges, assertions for NFSv4.x. The
COPY I-D has a start on the privileges.

If we are going to change channel bindings, I'd rather not have
to refer back to RFC5403 to and this I-D to get complete picture
of how to implement channel bindings. But regardless, the way
channel bindings are being proposed in RPCSEC_GSSv3, it appears I
have to create a child RPCSEC_GSS handle. I like RPCSEC_GSS handles
for lots of things, but in RPCSEC_GSSv2 we proved that we don't
need child handles to accomplish it. It seems like the biggest issue
RPCSEC_GSSv3 is trying to address with RPCSEC_GSSv2 channel bindings
is that v2 uses a hash of the bindings, and v3 asserts the bindings
are already hashed. Fine, then assert in v3 that rbcmia_bind_chan_hash
is no longer hash (or define an identify function x = hash(x)).

   All uses of a child context handle MUST use the GSS-API security
   context associated with the parent context handle of the
   RPCSEC_GSS3_CREATE request that produced the child context handle.
   The child context, however, has its own sequence number space and
   window, distinct from that of the parent.

The response to the window does not have the sequence window, so
the prose is false; the seq_window is the same as the parent, and
that's a good thing.

           struct {
                   unsigned int    version;
                   opaque          handle<>;
                   opaque          nonce<>;
                   opaque          mic<>;
           } rpc_gss3_gss_binding;

I suggest instead:

           struct {
                   opaque          rggbi_handle<>;
                   opaque          rggbi_nonce<>;
           } rpc_gss3_gss_binding_input;

           typedef opaque rpc_gss3_gss_binding; /* GSS_Wrap of
rpc_gss3_gss_binding_input; */

That way the input can be XDR encoded, then passed to
GSS_Wrap(). The change:

                   rpc_gss3_gss_binding    *compound_binding;

to

                   rpc_gss3_gss_binding    rgcb_compound_binding;

           struct {
                   rpc_gss3_gss_binding    *compound_binding;
                   rpc_gss3_chan_binding   *chan_binding_mic;
                   rpc_gss3_assertion      assertions<>;
                   rpc_gss3_extension      extensions<>;
           } rpc_gss3_create_args;

           struct {
                   opaque                  handle<>;
                   rpc_gss3_chan_binding   *chan_binding_mic;
                   rpc_gss3_assertion      granted_assertions<>;
                   rpc_gss3_assertion      server_assertions<>;
                   rpc_gss3_extension      extensions<>;
           } rpc_gss3_create_res;

Why no rpc_gss3_gss_binding in the result? The handle that
would be placed in rggbi_handle would be that of
rpc_gss3_create_res.handle, and the GSS_Wrap would be called with
the GSS context defined by rggbi_handle so that we can sure
that the target principal agrees to compounding.

3. Privileges and identity representation for NFSv4

This section should be in an NFSv4.x spec, not RPCSEC_GSSv3.

That is as far as I can go for now.

-- 
Mike Eisler, Senior Technical Director, NetApp, 719 599 9026,
http://blogs.netapp.com/eislers_nfs_blog/

---

From: Nicolas Williams <[email-masked]>
Date: Tue, 7 Apr 2009 16:05:24 -0500

Mike,

These are very good comemnts.  Thanks for taking the time to write them
up.  My replies are below.

> contains the MIC. But more importantly, the handle is not included

That was sloppy of me, though one could think of the auth+verifier as a
self-protecting unit.

> as the input to the MIC that is stored in the verifier of the response.

Oh.  That is bad.  Not so bad for RPCSEC_GSSv1, but real bad for v3,

> change mtype from CALL to REPLY:

Right.

Well, I don't think that would be true in our case.  I've thought
enough about how to implement this and even have some header changes
lying around rotting.

But more importantly, reducing the number of cryptographic context
needed is one my goals here, and the current v3 proposal does that.
Adding the context handle to the RPC response verifier input does not
seem like a steep price to pay for getting that reduction.

> nit picking is only fair :-).

Yes, but first things first.  There's little point in making the XDR
extractable if no one wanted to implement this!  So I saved myself some
time and effort because I wanted feedback first.

> both the software and wetware varieties.

Makes sense.

> will simplify the implementation.

It allows you to have different handle namespaces for each RPCSEC_GSS
version.  This is probably not important.

> BTW the above prose was rather distant from the new credential.

Yes, I just noticed that, and also I should have mentioned the field by
name in the text, to make it easier to find the relevant text.

I'll fix that.

> Anhyway, type, critical, and data are not defined else where in the spec.

'critical' is defined:

"
   If a client wants to ensure that the server understands the asserted
   label then it MUST set the 'critical' field of the label assertion to
   TRUE, otherwise it MUST set it to FALSE.
"

'type' and 'data' part of what we call a "typed hole," a {type,
opaque-data} tuple where the <type> indicates how to interpret the
<opaque-data> field.  Typed holes are part of how we handle
extensibility in PKIX, Kerberos V, ...  

'type' and 'data' are fairly self-evident and this is a -00...

> so understanding of the spec is a challenge.

I intend to add text explaining type, date, ...

> COPY I-D has a start on the privileges.

I agree.  Though with respect to labels there's not much we can do in
_this_ document (see the whole thread about labeled NFS on this list,
and also the sub-thread on the SAAG list).

> is no longer hash (or define an identify function x = hash(x)).

Child handles save cryptographic contexts.

Lacking an implementation of v2 I don't much care to complicate our
implementation of v3.  I do realize that v4.1 mandates v2 (don't
construe this to mean that we won't implement v2 -- I don't work for the
relevant team at Sun).  Of course, if the consensus is that we should
aim for commonality I'll pursue that.

> the prose is false; the seq_window is the same as the parent, and

It's not important that either peer select an initial sequence number,
actually.  It could always start at 0 and thus we'd avoid having to
negotiate it.

> Why no rpc_gss3_gss_binding in the result? The handle that

Because it suffices that the server return an error or success according
to channel binding failure/success.

> This section should be in an NFSv4.x spec, not RPCSEC_GSSv3.

Indeed, and my intention is to move these into such a spec, but having
everything in one place for a -00/-01 seemed like a good idea.

> That is as far as I can go for now.

Thanks!

Nico
--

---

From: "Mike Eisler" <[email-masked]>
Date: Tue, 7 Apr 2009 15:50:55 -0700 (PDT)

On Tue, April 7, 2009 1:01 pm, Mike Eisler wrote (regarding comments on

>    opaque_auth  cred; /* captures the RPCSEC_GSS handle */

Credit to Nico for leading me to finding the above hole and the
one below (he reminded me why RPCSEC_GSS cannot tolerate a
wrap-around of the 32 bit sequence number).

It turns out the SSV mechanism in NFSv4.1 has a similar issue because
there is just one SSV GSS-API context between a client ID and
major server owner, but multiple "child RPCSEC_GSS contexts" (to
borrow terminology from draft-williams-rpcsecgssv3-00). An
example attack is as follows:

Let's say we have two SSV handles, 0x1 and 0x2, each derived from the
same SSV context.

Client sends an RPC request protected with the SSV
mechanism for handle 0x1 to do a GETATTR for
fs_locations_info on filehandle 0x3 using RPCSEC_GSS
sequence number 2. The reply directs the client to server
COMPROMISED.example.com. Full integrity is used on the arguments and reply.

The verifier in the reply simply consists of the output of
GSS_GetMIC() on the 32 bit sequence number value (2).

The attacker records the reply.

Client sends an RPC request protected using the SSV for handle
0x2 to do a GETATTR for fs_locations_info on filehandle 0x3
using RPCSEC_GSS sequence number 2.
Again full integrity is used on the arguments and reply.

The reply directs the client server to GOOD.example.com.

The attacker becomes a man in the middle, and blocks the response. It
replaces the response with the first reply. The client verifies the
MIC of the verifier and of the reply, and they check out.

The client is now directed to COMPROMISED.example.com and gets
bad, fake data.

Knowing what we know now, we'd have likely changed RPCSEC_GSSv2
to correct the issue as I've proposed above. But RPCSEC_GSSv2 is
now an RFC.

We don't need to make a .x change to fix this.

There are a couple ways to fix this and both are implementation
advice:

(1) Safest but perhaps harder:

In the current spec, the SSV key is changed on every SET_SSV operation,
and the server indicates how many simultaneous versions of the SSV it will
support (see spi_window in the NFSv4.1 spec). The fix is to
provide advice to the requester (not just the client, because
SV handles can be used on the backchannel) to assign each
RPCSEC_GSS SSV handle to a unique version of the SSV. The attack
is defeated. However, there are would now we
effectively N (N == spi_window) different GSS contexts, which
might complicate implementation, especially those implementations
that have proceeded down the "one-SSV-GSS-context" road.

(2) Tricky but perhaps easier:

Another way is for the client to partition the RPCSEC_GSS sequence
number space into N pools, for each of N RPCSEC_GSS handles. E.g.
if there are 16 RPCSEC_GSS SSV handles on the fore channel,then
each handle gets 2^32 / 2^4 = 2^28 or about 250 million sequence
numbers. Handle zero gets to use sequence numbers 0 through 268 435 455
handle 1 gets to use 268 435 456 through 536 870 911, and so on.
One a handle reaches its maximum (e.g. once the client using handle 0
hits 268 435 455) the requester  has to stop using the handle,
and the client has to use EXCHANGE_ID to get new handles. If
the requester is on the backchannel, the client has to also
use BACKCHANNEL_CTL to get a handle for the backchannel.

----------

I am inclined to offer both ideas in the specification. There
are likely other approaches.

Regardless of the approach, the NFSv4.1 spec needs to provide
advice to the server implementor to make sure that the
value of the RPCSEC_GSS handles themselves does not get reused until
SET_SSV has been called N times (N == spi_window). Given that
most RPCSEC_GSS targets use 32 bit generation numbers, and given
that an spi_window of 2^32 is impractical, this is not onerous.

The safest place to make this observation and implementation
suggestions is probably in the Security Considerations.

I don't anticipate most clients and servers will need more than one
SSV RPCSEC_GSS handle on the fore channel or backchannel, so this issue
might be theoretical for most.

---

From: "Mike Eisler" <[email-masked]>
Date: Sat, 11 Apr 2009 13:19:38 -0700 (PDT)

> (1) Safest but perhaps harder:
[...]
> (2) Tricky but perhaps easier:
[...]

After several drafts of both, and one not the other, I opted for (2).

http://eisler.com/nfsv41/ has PAv3 which describes (2), fixes
an issue with one of the IANA registry (which I have reported to IANA),
and corrects the data types used for owner, owner_group, and mimetype.

On Tue, April 7, 2009 3:50 pm, Mike Eisler wrote:
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4
>