Telechat Review of draft-ietf-abfab-gss-eap-
review-ietf-abfab-gss-eap-secdir-telechat-hutzelman-2012-07-19-00

I have reviewed this document as part of the security directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written primarily for the benefit of the security area
directors.  Document editors and WG chairs should treat these comments just
like any other last call comments.

This document defines a family of GSS-API mechanisms which wrap EAP,
allowing applications which use GSS-API or SASL for authentiation to take
advantage of EAP mechanisms.  It is a core part of the ABFAB architecture,
which brings mechanism-agile federated authentication to a wide variety of
applications without requiring that a user's preferred (required)
authentication mechanism be supported by the services that he wishes to
use.

I've talked with the authors and others a lot about how ABFAB in general
and this mechanism in particular are supposed to work, so I'm already
comfortable with the general approach.  Thus, this review will be confined
to particular protocol specifics and some editorial issues.

This protocol combines existing security protocols and frameworks in new
ways, and thus invokes all of the security issues surrounding EAP, the
GSS-API, the per-message services of RFC4121, the cryptographic framework
defined in RFC3961, and the AES enctype defined in RFC3962.  It also
introduces new considerations related to layering, combination, and
involving additional parties in the authentication process.  Many of
these are incorporated by reference to the relevant documents, while
others are discussed directly.

Overall, I think this is basically done.  However, I did find a couple of
things, included in my comments below, which the authors should probably
address before the document is published.  In particular, I believe there
is a potential gap in the MN format (though this may be deliberate), an
ambiguity in how import of Kerberos principal names should be handled, and
a missing step in key derivation.  I also make suggestions related to
mechanism names and to integrity protection of the context establishment;
these may be things which the authors have already considered or don't
feel it is appropriate to pursue at this time.

-- Jeff


========================================

Section 3.1 discusses the format of the GSS-API Mechanism Names (MNs)
used by the family of mechanisms defined in this document.  However, it
sort of glosses over the notion that, in GSS-API jargon, "Mechanism Name"
means a name of an initiator or acceptor in a mechanism-specific format,
and _not_ the name of a mechanism, as one might otherwise assume.  I
suggest the following change to the first paragraph of this section:

  OLD:

   Before discussing how the initiator and acceptor names are validated
   in the AAA infrastructure, it is necessary to discuss what composes a
   name for an EAP GSS-API mechanism.  GSS-API permits several types of
   generic names to be imported using GSS_Import_name().  Once a
   mechanism is chosen, these names are converted into a mechanism name
   form.  This section first discusses the mechanism name form and then
   discusses what name forms are supported.

  NEW:

   Before discussing how the initiator and acceptor names are validated
   in the AAA infrastructure, it is necessary to discuss what composes a
   name for an EAP GSS-API mechanism.  GSS-API permits several types of
   generic names to be imported using GSS_Import_name().  Once a
   mechanism is chosen, these names are converted into a
   mechanism-specific name form, called a "Mechanism Name".  Note that a
   Mechanism Name is the name of an initiator or acceptor, not the name
   of a mechanism.  This section first discusses the mechanism name form
   and then discusses what name forms are supported.

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

At the end of section 3.1, you write "Mechanisms MAY support the
GSS_KRB5_NT_KRB5_PRINCIPAL_NAME name form", but not how names of this form
should be converted to GSS-EAP MNs.  This results in ambiguities and also
some potential for unexpected results, including importing invalid names.
Some Kerberos principal names will be invalid as EAP-GSS MNs, particularly,
those using principal name forms which contain at-signs or realm name forms
contianing slashes (the latter are not likely to be a practical problem,
but the former might be).  Others will be interpreted not as intended, or
may not be appropriate transformations (for example, user "instances" with
multiple principal name components).

To address this, I would recommend the following:

  1) Introduce an escaping syntax, such as the use of backslash as in
     RFC1964 section 2.1.1, to allow representation of name-strings which
     contain slash and at-sign characters.
  2) Admonish implementations which support importing of names of type
     GSS_KRB5_NT_KRB5_PRINCIPAL_NAME that when doing so they must process
     the escaping described in RFC1964 section 2.1.1 and convert to a
     canonical form in which only slash, backslash, and at-sign are
     escaped.
  3) Warn that direct import of Kerberos principal names may have
     unintended effects due to differences in name structure, and that this
     feature, if implemented, should be used carefully (possibly disabled
     by default).

  As an alternative to (1), you could continue to simply prohibit names
  containing slashes and at-signs as parts of name-strings.  In this case,
  implementations which support import of GSS_KRB5_NT_KRB5_PRINCIPAL_NAME
  names MUST verify that the imported name is valid and otherwise fail the
  import.

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

I am a bit confused by section 3.2.  You seem to be saying that the
representation of names and components of names transported in other
protocols is up to the protocol in question, but that the representation
when a name is sent as part of this protocol is UTF-8.  However, the ABNF
in section 3.1 permits only characters up to 0xff.  Is it the intent that
MNs be treated as UTF-8 strings?  If so, it would be better to specify the
MN form in two laters, first indicating that it is a UTF-8 string and then
using ABNF to define the permitted sequences of Unicode characters.

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

Why define a family of mechanisms parameterized by enctype, instead of
defining a single mechanism, specifying a mandatory-to-implement enctype,
and negotiating the enctype to be used as part of context establishment?
This would also work around a situation with SASL mechanism naming, which
is that you are effectively defining an entire family of GSs-API mechs but
specifying SASL mechanism names for only one member of that family.  This
means that either other enctypes cannot be used at all, or else they must
forever have non-friendly names encoded as per RFC5801 section 3.1.

Alternately, you could register a family of SASL mechansim names, of the
form EAP-<enc> and EAP-<enc>-PLUS, where <enc> is a numeric enctype.  This
is a bit uglier than EAP-AES128[-PLUS], but prevents future
interoperability problems due to SASL mechanism name mismatches and at
least is reversible, unlike the RFC5801 section 3.1 encoding.

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

In section 5.4, may the acceptor's message include a vendor subtoken?

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

In section 5.5, what is a "protected result indication" ?

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

Is it possible for the Extensions state to involve more than one round trip?

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

In section 5.6.1, initiators should be REQUIRED to send zeros for all flag
bits other than GSS_C_MUTUAL_FLAG, in order to guarantee that these bits
are available for future extension.

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

The MIC token described in section 5.6 currently protects only the
extension token containing it.  Is there any value to protecting the entire
context establishment exchange?

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

In section 6, the descriptions of the derivation of the GMSK and CRK
seem incomplete.  In particular...

- What happens if the EAP master session key is not large enough to
  satisfy the requirements of the GMSK enctype's random-to-key?
- What is the CRK enctype?
- I think you mean to indicate that the CRK is the result of applying
  the CRK enctype's random-to-key to the output of the indicated truncate
  call.  A mere string of random bits is not an RFC3961 protocol key.
- The definition of L is garbled.  I think you mean it is the length of
  data required by the CRK enctype's random-to-key.

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

It is frequently important to GSS-API initiators that they are talking to
the expected acceptor.  In the present mechanism, that requires not only
verifying the acceptor/NAS's identity with the EAP server (by means of EAP
channel bindings), but also verifying that the verified NAS identity agrees
with the GSS-API target name provided by the initiating application, if
any.  This issue is discussed in detail in sections 3.4 and 3.5, but could
bear an additional mention in the security considerations.




========================================

I also found a number of editorial issues:

Abstract:

  "... when using the EAP mechanism" should probably be "when using the
  Extensible Authentication Protocol (EAP)", at which point the second
  expansion of EAP could be removed.

Section 1, graf 6 (page 5): s/prospective/perspective/

Section 1.3, last graf (page 7): The phrase "security association
protocols" is used twice, where I believe "secure association protocols" is
intended.

Section 3.1 ABNF: This attempts to define name-char to match any character
except the slash (/) and at-sign (@), which are used to separate name
components.  However, it incorrectly prohibits uppercase G (decimal 71, hex
0x47) instead of slash (decimal 47, hex 0x2f).

Section 3.1, graf 3 (page 10, after ABNF): In "specifications of this
mechanism MUST NOT prepare the user-or-service according to these rules",
I think you mean "implementations", not "specifications".

Section 3.1, bottom of page 10: s/proceeding/preceeding/

Section 3.3, last graf: "canonicalizing it to a mechanism" should be
"canonicalizing it to a mechanism name".

Section 3.5, graf 4 (bottom of page 13): In "The EAP server MUST assure",
s/assure/ensure/.

In section 5, the table describing the format of an innerToken shows
the first subtoken body as occupying octets 10..10+n, which is a total
of n+1 bytes.  It should be 10+n-1, and the second subtoken type should
be at octets 10+n..10+n+3.

Sections 5.5.1, 5.5.2; the subtoken types are missing zeros.

Section 5.6; this state is variously called Extension (singular) or
Extensions (plural).

Section 7.2: The GSS mech parameters registry was indeed created by
RFC6542.  Also, it might be clearer if the last two entries in the table
explicitly referred to "this RFC", so someone doesn't think you mean
RFC4121 section 5.

Section 7.3: The table should be sorted by type, not defining section.

Section 7.4: draft-ietf-radext-radius-extensions is up to -06, and the
relevant section is now 10.3, not 9.3.

Section 7.5: s/EAP-ES128/EAP-AES128/

Section 10.1: Do you need a normative reference to RFC3962?