Subject: Review of draft-ietf-radext-design-01.txt

From: Bernard Aboba <[email-masked]>
Date: Sun, 2 Dec 2007 17:41:27 -0800

Review of draft-ietf-radext-design-01.txt
 
Designation
 
I thought that this document was a BCP, no?  -01 states thatit is "Standards Track". 
 
Sections 1 and 1.1
 
There is some overlap in coverage.  My recommendation is to reorganizethe discussion as follows:
 
"1.  Introduction
   This document provides guidelines for the design of RADIUS attributes   both within the IETF as well as within other Standards Development   Organizations (SDOs).  By articulating RADIUS design guidelines, it    is hoped that this document will encourage the development and   publication of high quality RADIUS attribute specifications.
   The advice in this document will not be helpful unless it is put to use.     As with "Guidelines for Authors and Reviewers of MIB Documents"   [RFC4181], it is expected that this document will enable authors to   check their document against the guidelines prior to requesting a   review (such an "Expert Review" described in [RFC3575]).  Similarly,   it is hoped that this document will be of use to reviewers (such as   WG participants or the AAA Doctors) in improving the consistency of   reviews.
 
   In order to meet these objectives, this document needs to cover not   only the science of attribute design, but also the art.  As a result,   in addition to covering the most frequently encountered issues, this   document attempts to provide some of the considerations motivating   the guidelines.
 
   In order to characterize current attribute usage, both the basic and   complex data types defined in the existing RADIUS RFCs are reviewed,   together with the ad-hoc extensions to that data model that have been   used in Vendor-Specific Attributes.
 
1.1.  Applicability
   As RADIUS has become more widely accepted as a management protocol,   its usage has become more prevalent, both within the IETF as well as   within other SDOs.  Given the expanded utilization of RADIUS,   it has become apparent that requiring SDOs to accomplish all their    RADIUS work within the IETF is inherently inefficient and unscalable.
   By articulating guidelines for RADIUS attribute design, this document    enables SDOs to design their own RADIUS attributes within the    Vendor-Specific Attribute (VSA) space, seeking review from the IETF.   In order enable IETF review of SDO RADIUS attribute specifications,   the authors recommend:
 
      * Development of a program to encourage SDOs to make their RADIUS      attribute specifications publicly available;
      * Review of IETF and SDO specifications according to the      guidelines proposed in this document;
 
   The advice in this document applies to attributes used to encode   data.  RADIUS protocol changes, or specification of attributes that   can be used to provide new RADIUS commands (such as Service-Type) are   out of scope.  Since protocol changes require greater expertise and   deeper review, such changes should not be undertaken outside the IETF   and when handled within the IETF require "IETF Consensus" for   adoption, as noted in [RFC3575] Section 2.1.
 
   As with protocol changes, this document does not provide guidance to   document authors seeking to change the RADIUS operational model.   While RADIUS server implementations may keep state, the RADIUS   protocol is stateless, although information may be passed from one   protocol transaction to another via the State Attribute.  As a   result, documents which require stateful protocol behavior without   use of the State Attribute are inherently incompatible with RADIUS as   defined in [RFC2865], and need to be redesigned.
   See [RFC5080] Section 2.1.1 for a more in-depth discussion of the use   of the State Attribute."
 
BTW, I would also suggest a statement that this document does not applyto new uses of RADIUS (e.g. uses of RADIUS that go beyond conversationsbetween a RADIUS client and server). 
Section 2.1.1
   IPv6 address   128 bit value, most significant octet first.   IPv6 prefix    8 bits of reserved, 8 bits of prefix length, up to                  128 bits of value, most significant octet first.
Judging by some recent specifications, there appears to be some confusionbetween these two types.  Should an IPv6 address type always be used torepresent an address (as opposed to the prefix type).  If so, you mightstate this explicitly using some normative language. 
 
   integer64      64 bit unsigned value, most significant octet first.
 
I believe that this type has also been used to express an IPv6 Identifiervalue, no? 
 
Section 2.1.2
 
"  New attributes SHOULD NOT use this tagging method because of the   limitatations described above."
 
limitatations -> limitations
 
Section 2.1.3
 
   The only other exception to the recommendation against complex types   is for types that can be treated as opaque data by the RADIUS server.   For example, the EAP-Message attribute, defined in [RFC3579] Section   3.1 contains a complex data type that is an EAP packet.  Since these   complex types do not need to be parsed by the RADIUS server, the   issues arising from policy language limitations do not arise.   Similarly, since attributes of these complex types can be configured   on the server using a data type of String, dictionary limitations are   also not encountered.  Section A.1 below includes a series of   checklists that may be used to analyze a design for RECOMMENDED and   NOT RECOMMENDED behavior in relation to complex types.
 
I think it may be worth expanding this discussion of opaque data typessomewhat and covering additional implications, security related onesin particular.  In general, RADIUS servers are highly valued targets sothat the introduction of complex data types brings withit the potential for introduction of security vulnerabilities such asbuffer overflows.  An extreme outcome would be a vulnerability thatwould allow an attacker to take over the RADIUS server.
 
The use of attributes representing opaque data does not reduce thisthreat, it merely moves it from the RADIUS server to an applicationthat consumes RADIUS attributes.  Should this application not beproperly isolated, or run with extended privileges, then the introduction of new opaque data types (or changes in the formatof existing opaque types) may introduce serious new security vulnerabilities. 
 
Section 3.1.3
 
   This change control can be obtained by requesting a PEC from the   Internet Assigned Number Authority (IANA), for use as a Vendor-Id   within a Vendor-Specific attribute.  
 
Can you expand "PEC" on first use? 
 
Rehosting may also require changes to the RADIUS data model   which will affect implementations that do not intend to support the   SDO specifications
Period needed at the end of the sentence. 
 
IDNITs check-------------
The tool found a few warnings:
 
idnits 2.05.02 
tmp/draft-ietf-radext-design-01.txt: - Failure fetching the file, proceeding without it.)
  Checking boilerplate required by RFC 3978 and 3979, updated by RFC 4748:  ----------------------------------------------------------------------------
     No issues found here.
  Checking nits according to http://www.ietf.org/ietf/1id-guidelines.txt:  ----------------------------------------------------------------------------
     No issues found here.
  Checking nits according to http://www.ietf.org/ID-Checklist.html:  ----------------------------------------------------------------------------
     No issues found here.
  Miscellaneous warnings:  ----------------------------------------------------------------------------
  == Using lowercase 'not' together with uppercase 'MUST' is not an accepted     usage according to RFC 2119.  Please use 'MUST NOT' (if that is what you     mean).          Found 'MUST not' in this paragraph:          This Attribute is available to allow vendors to support their own     extended Attributes not suitable for general usage.  It MUST not affect     the operation of the RADIUS protocol.
  Checking references for intended status: Proposed Standard  ----------------------------------------------------------------------------
     (See RFC 3967 for information about using normative references to     lower-maturity documents in RFCs)
  -- Looks like a reference, but probably isn't: '8' on line 1191     'The Text field consists of UTF-8 encoded 10646 [8] characters....'
  == Unused Reference: 'RFC4005' is defined on line 795, but no explicit     reference was found in the text     '[RFC4005] Calhoun, P., Zorn, G., Spence, D., and D. Mitton, "Diamete...'
  -- No information found for draft-ietf-radext-extended-attributes - is the     name correct?
     Summary: 0 errors (**), 2 warnings (==), 2 comments (--).--------------------------------------------------------------------------------

---

From: Alan DeKok <[email-masked]>
Date: Mon, 03 Dec 2007 13:24:17 +0100

> I thought that this document was a BCP, no?  -01 states that

  -00 was Standards Track, too.  The next version can be changed to a BCP.

> the discussion as follows:
...

  OK.

> BTW, I would also suggest a statement that this document does not apply
> to new uses of RADIUS (e.g. uses of RADIUS that go beyond conversations
> between a RADIUS client and server).
> Section 2.1.1

  OK.

>    IPv6 address   128 bit value, most significant octet first.
>    IPv6 prefix    8 bits of reserved, 8 bits of prefix length, up to
>                   128 bits of value, most significant octet first.
> 
> Judging by some recent specifications, there appears to be some confusion
> between these two types.  Should an IPv6 address type always be used to
> represent an address (as opposed to the prefix type).  If so, you might
> state this explicitly using some normative language.

  Hmm... suggestions?

> I believe that this type has also been used to express an IPv6 Identifier

  Yes.  That is mentioned in the following paragraph.  I'll add a line
here saying the same thing.

> limitatations -> limitations

  Fixed, thanks.

> Section 2.1.3
..
> I think it may be worth expanding this discussion of opaque data types
> somewhat and covering additional implications, security related ones
> in particular.

  Section 2.1.3 is already over two pages long.  I suggest adding a new
section immediately following it, "Complex attributes and Security":

2.1.4.  Complex Attributes and Security

   The introduction of complex data types brings the potential for the
   introduction of new security vulnerabilities.  Experience shows that
   the common data types have few security vulnerabilities, or else that
   all known issues have been found and fixed.  New data types require
   new code, which may introduce new bugs, and therefore new attack
   vectors.

   RADIUS servers are highly valued targets, as they control network
   access and interact with databases that store usernames and
   passwords.  An extreme outcome of a vulnerability due to a new,
   complex type would be that an attacker is capable of taking complete
   control over the RADIUS server.

   The use of attributes representing opaque data does not reduce this
   threat.  The threat merely moves from the RADIUS server to the
   application that consumes that opaque data.  Any such application
   SHOULD be properly isolated from the RADIUS server, and SHOULD run
   with minimal privileges.  Any potential vulnerabilities in that
   application will then have minimal impact on the security of the
   system as a whole.

  I've also added text in the security section referencing 2.1.4.

> Section 3.1.3
...
> Can you expand "PEC" on first use?

  Done in Section 2.2.

> Rehosting may also require changes to the RADIUS data model
>    which will affect implementations that do not intend to support the
>    SDO specifications
> 

  Fixed, thanks.

> IDNITs check
...
> ----------------------------------------------------------------------------
>   == Using lowercase 'not' together with uppercase 'MUST' is not an accepted
>      usage according to RFC 2119.  Please use 'MUST NOT' (if that is
> what you
>      mean).

  Text changed to MUST NOT.

>   -- Looks like a reference, but probably isn't: '8' on line 1191

  It's in a quote from another document.

>   == Unused Reference: 'RFC4005' is defined on line 795, but no explicit
>      reference was found in the text
>      '[RFC4005] Calhoun, P., Zorn, G., Spence, D., and D. Mitton,
> "Diamete...'

  That reference can be deleted.

>   -- No information found for draft-ietf-radext-extended-attributes - is the

  Yes.

  Alan DeKok.

--
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: <[email-masked]>
Date: Mon, 3 Dec 2007 07:42:18 -0800

>> state this explicitly using some normative language.

How about this?

"Where the intent is to represent a specific IPv6 address, the IPv6 address 
type
SHOULD be used.  Although it is possible to utilize the IPv6 Prefix type 
with a prefix
length of 128 to represent an IPv6 address,  this usage is NOT RECOMMENDED."

>   application that consumes that opaque data.

Suggest adding:  "The threat is particularly severe when the opaque data
is not originated or checked by the NAS, so that the RADIUS server is
potentially exposed to attack by malware residing on a host.  Applications
consuming opaque data that reside on the RADIUS server

---

From: "David B. Nelson" <[email-masked]>
Date: Mon, 3 Dec 2007 12:01:57 -0500

Bernard Aboba writes...

There is some overlap in coverage.  My recommendation is to reorganize
the discussion as follows:
 
"1.  Introduction
   This document provides guidelines for the design of RADIUS attributes
   both within the IETF as well as within other Standards Development
   Organizations (SDOs).  By articulating RADIUS design guidelines, it 
   is hoped that this document will encourage the development and
   publication of high quality RADIUS attribute specifications.
   The advice in this document will not be helpful unless it is put to use. 

   As with "Guidelines for Authors and Reviewers of MIB Documents"
   [RFC4181], it is expected that this document will enable authors to
   check their document against the guidelines prior to requesting a
   review (such an "Expert Review" described in [RFC3575]).  Similarly,
   it is hoped that this document will be of use to reviewers (such as
   WG participants or the AAA Doctors) in improving the consistency of
   reviews.
 
   In order to meet these objectives, this document needs to cover not
   only the science of attribute design, but also the art.  As a result,
   in addition to covering the most frequently encountered issues, this
   document attempts to provide some of the considerations motivating
   the guidelines.
 
   In order to characterize current attribute usage, both the basic and
   complex data types defined in the existing RADIUS RFCs are reviewed,
   together with the ad-hoc extensions to that data model that have been
   used in Vendor-Specific Attributes.

DBN: I think this text is very good.

1.1.  Applicability
   As RADIUS has become more widely accepted as a management protocol,
   its usage has become more prevalent, both within the IETF as well as
   within other SDOs.  Given the expanded utilization of RADIUS,
   it has become apparent that requiring SDOs to accomplish all their 
   RADIUS work within the IETF is inherently inefficient and unscalable.
   By articulating guidelines for RADIUS attribute design, this document 
   enables SDOs to design their own RADIUS attributes within the 
   Vendor-Specific Attribute (VSA) space, seeking review from the IETF.
   In order enable IETF review of SDO RADIUS attribute specifications,
   the authors recommend:
 
      * Development of a program to encourage SDOs to make their RADIUS
      attribute specifications publicly available;
      * Review of IETF and SDO specifications according to the
      guidelines proposed in this document;
 
   The advice in this document applies to attributes used to encode
   data.

DBN:  I think this last sentence could be a bit more specific.  All
attributes encode data.  The question is what kind of data.  I think what
you want to say is:

   The advice in this document applies to attributes used to encode
   service-provisioning or authentication data.

   RADIUS protocol changes, or specification of attributes that
   can be used to provide new RADIUS commands (such as Service-Type) are
   out of scope.

DBN: I recommend clarifying this last sentence just a bit by:

s/can be used to provide new RADIUS commands/can be used to, in effect,
provide new RADIUS commands/

   Since protocol changes require greater expertise and
   deeper review, such changes should not be undertaken outside the IETF
   and when handled within the IETF require "IETF Consensus" for
   adoption, as noted in [RFC3575] Section 2.1.
 
   As with protocol changes, this document does not provide guidance to
   document authors seeking to change the RADIUS operational model.
   While RADIUS server implementations may keep state, the RADIUS
   protocol is stateless, although information may be passed from one
   protocol transaction to another via the State Attribute.  As a
   result, documents which require stateful protocol behavior without
   use of the State Attribute are inherently incompatible with RADIUS as
   defined in [RFC2865], and need to be redesigned.
   See [RFC5080] Section 2.1.1 for a more in-depth discussion of the use
   of the State Attribute."

---

From: Alan DeKok <[email-masked]>
Date: Mon, 03 Dec 2007 20:57:07 +0100

[email-masked] wrote:
> How about this?
> 
> "Where the intent is to represent a specific IPv6 address, the IPv6
...

  Done.

> Suggest adding:  "The threat is particularly severe when the opaque data

  Word-smithing:

   The threat is particularly severe when the opaque data does not
   originate from, or is checked by the NAS.  In those cases, the RADIUS
   server is potentially exposed to attack by malware residing on an
   unauthenticated host.  Applications consuming opaque data that reside
   on the RADIUS server SHOULD be properly isolated from the RADIUS
   server, and SHOULD run with minimal privileges.  Any potential
   vulnerabilities in that application will then have minimal impact on
   the security of the system as a whole.

  (chopping long sentences, etc.)

  Alan DeKok.

---

From: Alan DeKok <[email-masked]>
Date: Mon, 03 Dec 2007 21:04:24 +0100

...
> DBN: I think this text is very good.

  Updated as recommended.
...
> attributes encode data.  The question is what kind of data.  I think what

  Updated

> DBN: I recommend clarifying this last sentence just a bit by:

  Updated.

  I'll post a new version before the meeting.

  Alan DeKok.