Subject: AD review of draft-zorn-radius-pkmv1-04.txt

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Wed, 22 Jul 2009 15:01:09 +0200

I have reviewed as shepherding AD draft-zorn-radius-pkmv1-04.txt and I
believe that the document is clear and stable. As the document requires
IANA to allocate attributes from the standard RADIUS type space
according to the RFC 5226 "IETF Review" policy, an IETF Last Call is
required. Please consider the comments below together with the other
IETF Last Call comments. 

1. The document makes repeated references to IEEE 802.16-2004. As this
IEEE specification is a 800+ pages document I think that readers and
users of the future RFC would appreciate if more exact references to
sections in IEEE 802/16-2004 are provided. For example in section 3.3
when referring to the IEEE 802.16 "PKM Configuration settings" attribute
the reader should be sent to section 11.9.19 of IEEE 802.16-2004, then
when talking about the "Authorize wait timeout" field section 11.9.19.1
of IEEE 802.16-2004 could be referred, etc. 

2. In the introduction section please take out "(or at least access to)"
- the access to the IEEE 802 document is now free over the Internet and
in fact the references section of this document already provides an URL
for free download. 

3. Section 3.4, first paragraph - s/contains only the 3 octet/contains
only the list of 3 octet/

Regards,

Dan

--
to unsubscribe send a message to [email-masked] with
the word 'unsubscribe' in a single line as the message text body.
archive: <http://psg.com/lists/radiusext/>

---

From: Alan DeKok <[email-masked]>
Date: Thu, 23 Jul 2009 11:43:05 +0200

Romascanu, Dan (Dan) wrote:
> I have reviewed as shepherding AD draft-zorn-radius-pkmv1-04.txt 

  Following on Dan's review, I've reviewed the document for agreement
with the RADIUS design guidelines document [1]

  Both the PKM-SS-Cert and PKM-CA-Cert attributes provide 'ad-hoc'
extension of the RADIUS attribute size, much like the EAP-Message
attribute.  It would have been preferable to follow the extended
attribute format [2].  This provides a standardized way of carrying data
larger than a 253 bytes.

  However, that document has not yet been published.  My question is
that if there are no current implementations of the PKM specification,
it may be preferable to wait until the extended attributes document is
finished, and then to use that format.

  Section 3.3 defines the PKM-Config-Settings.  This is a "complex
attribute" within the meaning of Section 2.1.3 of the design guidelines
document, which states:

   As a
   result, the introduction of new complex data types within RADIUS
   attribute specifications SHOULD be avoided, except in the case of
   complex attributes involving authentication or security
   functionality.

  I do not see any pressing need to make this attribute carry seven
independent fields.  The RADIUS attribute space has sufficient room to
allocate 7 attributes.  If the extended attribute format is used, then
nearly all space limitations are removed.  I would recommend that these
independent values be split up into independent attributes that follow
the RADIUS data model.

  In Section 3.4.  PKM-Cryptosuite-List, can the attribute be longer
than 253 bytes?    If so, do the same ad-hoc rules as above apply?  The
IEEE specification seems to permit attributes up to 32768 octets in length.

  It would also be good to reference a specific page or section in the
IEEE spec, rather than just referring to the document as a whole.

  Section 3.5.  PKM-SAID, defines an attribute containing 2 octets of
data.  It would be preferable to use a 4-octet type, and to specify that
the upper 2 octets are zero.  This would allow the attribute to fit
within the existing RADIUS data model, as discussed in Section 2.1.1 of
the design guidelines document:

   It is worth noting that since RADIUS only supports unsigned integers
   of 32 or 64 bits, attributes using signed integer data types or
   unsigned integer types of other sizes will require code changes, and
   SHOULD be avoided.

  Section 3.6.  PKM-SA-Descriptor defines another complex attribute as
discussed above.  It would be good to define this as a 64-bit integer,
which would fit within the RADIUS data model.

  Section 3.7.  PKM-AUTH-Key defines a complex attribute for
authentication or security, which fits within the design guidelines.
However, the encryption of the attribute is specified by reference to
[IEEE.802.16-2004].  This encryption is unlike anything previously used
in RADIUS, but it appears to be mandated by the IEEE specificiation.

  It would also be good to reference a specific section of
[IEEE.802.16-2004] for this definition, rather than referencing the
document as a whole.

  I also have some comments on the Security Considerations section:

   If the Access-Accept message is not subject to strong integrity
   protection, an attacker may be able to modify the contents of the
   PKM-Auth-Key Attribute.  For example, the Key field could be replaced
   with a key known to the attacker.

  Would it be sufficient to require that the Access-Accept contains a
Message-Authenticator for integrity protection?

   Although it is necessary for a plaintext copy of the Key field in the
   PKM-AUTH-Key Attribute to be transmitted in the Access-Accept
   message, this document does not define a method for doing so
   securely.

  How is inter-operability to be achieved if there is no specification
for how to carry a necessary field?

  I suggest changing the suggestion to use MS-MPPE-Send-Key into a
mandatory requirement, and making it part of the specification.

  Alan DeKok.

[1] http://tools.ietf.org/html/draft-ietf-radext-design-07
[2] http://tools.ietf.org/html/draft-ietf-radext-extended-attributes-08

---

From: Alan DeKok <[email-masked]>
Date: Thu, 23 Jul 2009 17:01:52 +0200

Glen Zorn wrote:
> No.  There _are_ implementations as I rather clearly stated at the meeting
> in SF, using the Experimental attribute space.

  So the implementations have to be updated to use the IANA code points,
independent of them possibly using the extended attributes format.

> Given that there are already multiple interoperable implementations and all
> of the attributes are security related, I see no pressing need to make
> people change running code to satisfy your personal preferences.

 The contents of the design guidelines document reflect the WG consensus
after years of effort and review.  It by *no* means reflects solely my
personal preference, as seems to be implied here.

>  Sorry if
> these attributes were designed to be easy to implement for people writing
> PKM code, rather than RADIUS code.

  There's no need to follow the RADIUS design guidelines, because the
attributes refer to data that interacts with non-RADIUS systems...

  What value, then, is in the design guidelines, WG consensus, or IETF
review?  Can we just over-ride them willy-nilly because a vendor has an
implementation of a spec?

>> than 253 bytes?    
> No.  There are exactly 

  This response seems to have been cut off.

> If you consider MD5 to be "strong integrity protection".

  It appears to be good enough for the rest of RADIUS, at least until
the TLS-based methods are standardized.

> MPPE-Send-Key is broken; I don't see the value in requiring the use of
> broken technology.  There are much better ways to encrypt attributes for
> transmission, in which (once again) the radext WG has shown near zero
> interest.  Maybe it should be a cisco VSA?

  If it's documented, sure.

  Alan DeKok.

---

From: [email-masked]
Date: Thu, 23 Jul 2009 20:21:29 +0000 (UTC)

Glen Zorn writes... 

> > As a  result, the introduction of new complex data types within 

> > RADIUS attribute specifications SHOULD be avoided, except 

> > in the case of complex attributes involving authentication or 

> > security functionality. 
> 
> All of the attributes are security related. 

Yeah.  I've always been a bit uncomfortable with the "security functionality" escape clause in the RADIUS Design Guidelines draft.  Lots of things can reasonably be claimed to be "security related".  I would have preferred the exception to be crafted a bit narrower, just for this reason.  But, unless wording of Design Guidelines is changed, you have a legitimate argument. 

> people change running code to satisfy your personal preferences.  Sorry if 

No one has ever claimed that following the RADIUS Design Guidelines is the only way to interoperable implementations.  Lots of designs, good, bad and indifferent, can be made to interoperate . 

If we are to dismiss the Design Guidelines as a "personal preference" how are they to be taken seriously?  I understand that following someone else's design guidance is not always popular with developers, but following regular design patterns is a well recognized practice in software engineering, that is generally attributed with increasing product quality and maintainability.  I think we simply need to "get with the program" here.

---

From: Alan DeKok <[email-masked]>
Date: Fri, 24 Jul 2009 17:06:49 +0200

[email-masked] wrote:
> Yeah.  I've always been a bit uncomfortable with the "security
> functionality" escape clause in the RADIUS Design Guidelines draft. 
> Lots of things can reasonably be claimed to be "security related".  I
> would have preferred the exception to be crafted a bit narrower, just
> for this reason.  But, unless wording of Design Guidelines is changed,
> you have a legitimate argument.

  I believe the intent was "related to RADIUS security".  The guidelines
document could be updated to address this.

  RADIUS could transport parameters for *another* protocol.  Those
attributes are not security related.  They either follow the RADIUS data
model (int, IP address, etc.), or they are "opaque data" that RADIUS is
simply transporting on the behalf of the other protocol.

  Alan DeKok.

---

From: Bernard Aboba <[email-masked]>
Date: Sun, 26 Jul 2009 16:06:59 -0700

> document could be updated to address this.

The rationale behind the original exemption was that security required
changes on both the RADIUS client and server.  Therefore the RADIUS
server would need a code change anyway, regardless of whether the
attributes were complex or simple.   That rationale applies not just
to "RADIUS security" but also to authentication mechanisms, many of
which were previously implemented with complex attributes (e.g. CHAP). 
So I'm not clear that we're talking about a "loophole" here. 

> encapsulation using RFC 2548 MPPE-Key attributes...

I was unclear about how this is supposed to work.  Is the idea to apply
the MPPE-Key encryption mechanism to the attribute specified
in the draft, or is the idea to actually use the MPPE-Key attributes 
themselves?  If the former, more detail should be provided.  If the latter,
is it necessary to define two attribute formats or would it be simpler to
go with one?  If the RFC 2548 MPPE-Key attributes are used, is the format
the same as that defined in RFC 2548 (just a wrapped key) or is the wrapping
applied to a complex attribute?  

> a four octet Integer should be used instead of a two octet data type
> (which doesn't exist in RADIUS)

As I recall, the security exemption didn't apply to creation of new RADIUS
data types, correct?