Subject: [kitten] Review of draft-ietf-kitten-channel-bound-flag-01

From: Greg Hudson <[email-masked]>
Date: Sat, 24 Jun 2017 10:38:53 -0400

Here are my semantic concerns:

Section 1:

* Paragraph 2 discusses taking advantage of implementation behavior to
  deploy channel bindings on initiators first, then acceptors.  A
  naive reader might conclude that this is sufficient, because an
  acceptor can be configured to not provide channel bindings if it
  does not wish to require all initiators to provide them as well.  It
  would be helpful to explain that the value obtained by providing
  channel bindings on the acceptor without enforcing them, which is
  that participating initiators gain protection against MITM attacks.

* Paragraph 4 should either be omitted or made more specific.  It's
  interesting to me that SSPI implements these semantics (all of them
  or just the existence of a channel-bound ret flag?), but that should
  be stated directly or just skipped.

Section 1.1:

* "[RFC2743] seems to indicate that mechanisms must ignore channel
  bindings when one party provided none."  What part of RFC 2743?  How
  does this statement square with "This facility is meant to be
  all-or-nothing" in the introduction?

* The remainder of this paragraph seems redundant with the top material
  in section 1.

Sections 1.2-1.4: these sections are kind of informal and may not all
be of interest to implementors.  I think it's worth saying that
gss_create_sec_context() is expected to lead to simpler stepper
functions in the future, but the rest can probably go.

Section 2.2:

* The term "understood" is a little unclear, and aside from the new
  channel-bound flag, it's unclear how a mechanism should process the
  absence of an understood flag.  Out of band, Nico has said that he
  doesn't expect the mechanism to pare down its set of ret_flags based
  on ret_flags_understood (e.g. it should not omit the mutual-auth
  flag if the application doesn't include it in ret_flags_understood).
  So the mechanism should only consult ret_flags_understood when it
  really needs to know, such as with the new channel-bound flag.  The
  draft should make that clear.

* The new gss_set_context_flags() call allows us to specify req_flags
  to gss_accept_sec_context(), which is an intentional addition.
  However, we have no immediate use for it, and the draft doesn't make
  it clear whether an existing mechanism should do anything with
  existing req_flags on the acceptor.

Section 2.2.1:

* Naming the parameter "ret_flags" rather than "ret_flags_understood"
  in the C bindings appears to create some confusion as to what should
  be done with the flags.

Section 2.4 and 2.5:

* For background, the overall purpose of this draft is to make channel
  bindings fit the GSS-API model where the caller asks for things and
  then checks whether it actually got them.  On the initiator it is
  awkward to achieve this goal, since the krb5 mech does not have
  channel binding confirmation and other existing mechanisms may not
  have it either.  Thus the draft specifies new mechanism attributes
  to express whether a mechanism has channel binding confirmation, and
  a request flag to filter SPNEGO mechanisms according to whether they
  can confirm channel bindings.  Nico has a plan to extend the krb5
  mech's mutual-auth response to confirm channel bindings, likely
  using GSS_C_CB_CONFIRM_FLAG (2048) in the 8003 checksum flags value
  as a negotiation bit.

* GSS_C_MA_CBINDING_MAY_CONFIRM does not really express where the krb5
  mech will wind up after the mutual auth response is extended, as
  channel bindings will only be confirmed if the initiator requests
  mutual auth, and possibly only if the mechanism choose an RFC 4121
  enctype.

* The specification of req_cb_confirmation_flag is wishy-washy as to
  whether it allows SPNEGO to choose a mechanism that might only
  possibly be able to confirm channel bindings.  I'm not sure that
  this request flag will ever wind up being useful to callers.

* "the pseudo-mechanism MUST NOT negotiate any mechanisms that lack
   the GSS_C_MA_CBINDING_CONFIRM or GSS_C_MA_CBINDING_MAY_CONFIRM
   mechanism attributes" reads to me as "mechanisms must have both
   attributes to qualify", but from context I think it is supposed to
   mean "mechanisms must have one of the two attributes to qualify".

Section 2.6:

* Here we specify the behavior of gss_delete_sec_context on an empty
  context.  We should also specify the behavior an the application
  tries to use an empty context with gss_inquire_sec_context(),
  gss_wrap(), etc..  Per discussion with Nico, doing so should result
  in GSS_S_NO_CONTEXT, just as if the application supplied
  GSS_C_NO_CONTEXT to those functions.

Section 3:

* The third requirement says that init_sec_context SHOULD be lax about
  the acceptor not providing channel bindings, but notes that "It is
  possible that not all security mechanism protocols can implement
  this requirement easily."  In contrast, the fourth requirement says
  that init_sec_context MUST be lax if the caller understands
  ret_channel_bound_flag.  The fourth requirement doesn't seem to
  square with the proviso on the third.  If a mechanism can conform to
  the fourth requirement, then it seems like it can also conform to
  the third requirement.

And here are a few copy-editing notes:

Abstract:

* "This document addresses both, the..." should not have the comma.

Section 2.2:

* undestands -> understands

* "individual elements -one-per-flag- instead" ->
  "individual elements--one per flag--instead"

Section 3:

* There are multiple instances of "Whenever [something] shall have
  [done something]", which doesn't look right to me.  Omitting the
  word "shall", and sometimes changing "have" to "has", makes them
  look better.

* "Whenever both, the initiator..." should not have the comma.

* "This is a restatement... restated for the reader's convenience"
  should either omit "a restatement of" or the final clause.

---

From: Benjamin Kaduk <[email-masked]>
Date: Sun, 25 Jun 2017 12:00:04 -0500

Thanks for the review, Greg.

Nico, will you be able to prepare a document update?

(one note inline)

>    mean "mechanisms must have one of the two attributes to qualify".

I agree that's the apparent intent.

-Ben