Subject: [Emu] Working Group Last Call for draft-ietf-emu-chbind-09

From: Joe Salowey <[email-masked]>
Date: Wed, 28 Sep 2011 10:53:12 -0700

This is the working group last call for draft-ietf-emu-chbind-09.  Please send your comments to the list by October 21, 2011.  

Thanks,

Joe

---

From: Alan DeKok <[email-masked]>
Date: Wed, 28 Sep 2011 22:04:08 +0200

> This is the working group last call for draft-ietf-emu-chbind-09.  Please send your comments to the list by October 21, 2011.  

  I've read the document, and have comments as follows.

Section 4.3 Page 11:

   For example, information carried
   in AAA attributes such as the NAS IP address may have been lost in
   transition and are thus not known to the EAP server.

 Even worse, that information might exist, but be entirely useless.  For
example, a NAS can be in a private network, and send a NAS IP address of
192.168/16.  Many AAA proxy hops later, the EAP server receives that IP.

  As the next piece of text says:

   However, often verification of the MAC or IP address of the NAS is
   not useful for improving the overall security posture of a network.

 Channel bindings can be used to verify that the NAS is not lying (i.e.
it is sending the same information to the client and to the EAP server.)
 But that information has no useful context, and is therefore useless
for any purpose.

  Bottom of page 11, there is a typo "lyes" should be "lies".

  Page 11 and 12 again:

   As discussed in the next section, some of the most important
   information to verify cannot come from AAA attributes but instead
   comes from local configuration.  For example in the mobile phone
   case, the expected roaming rate cannot come from the roaming provider
   without being verified against the contract between the two
   providers.  Similarly, in an enterprise, the SSID a particular access
   point is expected to advertize is a matter of configuration rather

  typo: "advertise"

   than something that can be trusted because it is included in an AAA
   exchange.

  I think that this point could be clearer.  The EAP peer and
authenticator do not share trust, but all other parties do.  Channel
bdingins allows the EAP peer and authenticator to gain trust in each other.

5.1.  Protocol Operation

  It may be useful to point out that specifications have traditionally
treated the AAA server and EAP server as separate entities.  However,
there is no specification for any protocol which allows those two
servers to communicate.  i.e. there is no equivalent to EAPoL for servers.

  Much of the rest of the text in this section is a round-about way of
saying that we assume the AAA server and EAP server can communicate, via
some mechanism not specified here

5.2.  Channel Binding Consistency Check

  It may be useful to add the note from Section 4.3, where the EAP
server verifies the information against it's local database.  This
prevents the EAP peer and authenticator from colluding to cheat the EAP
server.

5.3.  EAP Protocol

  For clarity, it would be good to add some blank lines around the
definitions of NSID, Length, and NS-Specific.  The use of NS-Specific
should be made consistent between the text and the diagrams.

  The diagram also has two instances of the "NSSID, Length, NS-Specific"
data, but only one instance of Code.  Is it meant to show that multiple
instances of channel binding data can appear in the packet?  I presume so...

  I recommend changing the assigned code point for RADIUS from 0 to 1.
The value of "0" is often used in code for uninitialized data.  It may
be useful to distinguish that data from the intention to send RADIUS.

  It says:

 Length:  Two octets of length in network byte order, not including
      the length or the namespace ID.

  I suggest replacing this with:

  Length: two octets of length in network byte order, indicating the
number of octets of data in the NS-Specific field.  The octets for NSSID
and Length are not included in this count.

  It says:

   A given NSID MUST NOT appear more than once in a channel binding data
   or channel binding response.

  I suggest adding a following sentence:

    Instead, all NS-Specific data for a particular NSSID MUST occur
    within the context of one (NSSID, Length, NS-Specific) field.

  It says:

   In channel binding data, the code is set to 0 (channel binding data)

  As above, I suggest leaving the value "0" as unused, and starting
assignments from "1".  If we want to get cute, we can re-use the values
from EAP:

	2 data from client
	3 success
	4 fail

---

From: Alan DeKok <[email-masked]>
Date: Wed, 28 Sep 2011 22:14:09 +0200

Continuing from the previous message...

  It says:

5.3.1.  Channel Binding Codes

	2 data from client
	3 success
	4 failure

5.3.2.  Namespace Identifiers

  Again, I suggest using "1" for RADIUS.

  A related question is should we allocate a code for Diameter? :)

5.3.3.  Radius Namespace

  I suggest leaving out all re-definition of RADIUS AVPs.  Instead,
refer to RFC 2865 Section 5.  Say that the NS-Specific data is RADIUS
attributes, in the RADIUS attribute format.  But the RADIUS packet
header (code, length, authenticator) is not included.

  It also says:

   RADIUS AVPs are encoded with a one-octet attribute type followed by a
   one-octet length followed by the value of the RADIUS attribute being
   encoded.  The length includes the type and length octets; the minimum
   legal length is 2 for an attribute with no value.

  This last bit is not true.  RFC 2865 forbids attributes with "Length"
of two.  Instead, the entire attribute is suppressed, and never placed
into a packet.

  The same should apply here.

  It says:

   Some RADIUS container specifications forbid sending
   attributes with no value.

  No, the base RADIUS protocol forbids sending attributes with no value.
 It is not clear what they mean, or how it is better to send them than
to send nothing at all.

   This prohibition not withstanding, these
   attributes SHOULD be sent with no value in channel binding responses.

  I am not clear why the document makes that suggestion.  If it's a
SHOULD, there should be explanation as to why it's a good idea.

  Alan DeKok.

---

From: Sam Hartman <[email-masked]>
Date: Wed, 28 Sep 2011 17:52:39 -0400

I'm happy to make changes regarding your comments; a couple of them were
not directly actionable but I can make best guesses.  Most of them were
directly actionable and I agree the changes improve the text.
I'd appreciate comments from the groups on whether we start codes at 2
or 1.
My default will be 1 unless a couple of other people come forward and
ask for 2.

I do disagree with a couple of comments however; see below.

>>>>> "Alan" == Alan DeKok <[email-masked]> writes:

    Alan>   Continuing from the previous message...  It says:
    Alan>   A related question is should we allocate a code for
    Alan> Diameter? :)

Exactly when someone plans to use it and works through the details.
I would object to that allocation now.

    Alan> 5.3.3.  Radius Namespace

    Alan>   I suggest leaving out all re-definition of RADIUS AVPs.
    Alan> Instead, refer to RFC 2865 Section 5.  Say that the
    Alan> NS-Specific data is RADIUS attributes, in the RADIUS attribute
    Alan> format.  But the RADIUS packet header (code, length,
    Alan> authenticator) is not included.

I'd prefer to keep the definition in.

    Alan>   It also says:

    Alan>    RADIUS AVPs are encoded with a one-octet attribute type
    Alan> followed by a one-octet length followed by the value of the
    Alan> RADIUS attribute being encoded.  The length includes the type
    Alan> and length octets; the minimum legal length is 2 for an
    Alan> attribute with no value.

    Alan>   This last bit is not true.  RFC 2865 forbids attributes with
    Alan> "Length" of two.  Instead, the entire attribute is suppressed,
    Alan> and never placed into a packet.
    Alan>   The same should apply here.

Unfortunately, as discussed in the text, we need to be able to send
empty attributes in channel binding response.
I understand RADIUS has no meaning for empty attributes, but I think we
have a justified meaning for expressing that.
I think the text is correct as is.

    Alan>   It says:

    Alan>    Some RADIUS container specifications forbid sending
    Alan> attributes with no value.

    Alan>   No, the base RADIUS protocol forbids sending attributes with
    Alan> no value.  It is not clear what they mean, or how it is better
    Alan> to send them than to send nothing at all.

    Alan>    This prohibition not withstanding, these attributes SHOULD
    Alan> be sent with no value in channel binding responses.

    Alan>   I am not clear why the document makes that suggestion.  If
    Alan> it's a SHOULD, there should be explanation as to why it's a
    Alan> good idea.

It's a SHOULD because we may specify cases where you include value or
where you don't fully understand the container format.
The definition of channel binding responses indicates that by default
you SHOULD send the attribute without a value and explains why. (You
don't want to echo back all that data.)