[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

FW: Review of Radius Guidelines 11




-----Original Message-----
From: Avi Lior [mailto:avi@bridgewatersystems.com] 
Sent: Monday, February 22, 2010 11:32 AM
To: Bernard Aboba; Alan DeKok DeKok
Subject: Review of Radius Guidelines 11

Review of RADIUS Design Guidelines - this is the version that Alan sent to
me earlier.  The one uploaded may have some slight corrections

-------

I am a little confused about the purpose of section 2 RADIUS Data Model and
the purpose of Section 3 Data Model Issues.

Is the intent of section 2 to describe the data model then it should just do
that and move discussions of issues to section 3.

---------------
Section 2:
Please define Standard Space
Please define complex attributes. (you do so much later) or give a reference
to where they are defined.

"The format of this space is defined by individual vendors but the same TLV
encoding used by the standard space..."

Change to:

"The format of the payload of these attributes is defined by individual
vendors but the same TLV encoding...."
-----------------
2.1.1

   "However in order
   to prevent future ambiguities, it is recommended that future RADIUS
   attribute specifications explicitly define newly created data types
   at the beginning of the document, and indicate clearly the data type
   to be used for each attribute."

Do we care about the structure of the document.  I think the issue is that
specification should define newly created data types and indicate clearly
the data type to be used for each attribute.  

Second issue: It seems to me that we are okay with folks creating new data
types in standard space.  I think creation of new data types should be
discouraged right?
-----------------------

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

Why are we talking specifically signed integers?  This is already covered in
the discussion of creating new types.

But since we do bring this up - I suppose that we are talking about this
because this is a common issue.  After all there is no way to represent
negative numbers in RADIUS (or signed integers).  This creates a problem in
general and specific when translating Diameter signed integers to RADIUS.  I
think this document should either remove this paragraph or offer
guidance/solution.
--------------
2.1.2

" For these reasons, the tagging scheme described in RFC 2868 is NOT
   RECOMMENDED for use as a generic grouping mechanism."

What do you mean by generic grouping mechanism?

What about guidance on how to achieve this?

--------------------
Section 2.1.3 Complex Types

Can we not re-write this section to be a little more concise and to the
point? 

Complex types and New Types require code changes in the client and/or the
server.
Code changes creates security risks see security consideration section.

Thus one should avoid creating new types or using complex types.

--------

"However, some standard attributes do not follow this format."
Please give an example of an attribute.

 "Attributes that use sub-fields instead of using a basic data type are
   known as "complex attributes". "

What are sub-fields.
Is an attribute that codefies a bit map a complex attribute?  I think yes.
This is probably the most common complex type that people use.  What
guidance do we offer to designers that need a bit-field?  Each bit is an
attribute?  

---------

"   * it is easier for the user to enter the data as well-known
        types, rather than complex structures;"
This bullet should be removed.  Following good engineer practices of
seperation of concerns and in this case seperation of presentation and the
model (MVC).  We dont want to encourage users to edit raw data as codefied
by the RADIUS protocol.  For example, we dont want user to enter a IPv4
address as a 32bit value or an Address type. But rather as an IP address in
dot notation.

--------
      * it enables additional error checking by leveraging the
        parsing and validation routines for well-known types;

      * it simplifies implementations by eliminating special-case
        attribute-specific parsing.

These two bullets are the same and should be combined.

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

One of the fundamental goals of the RADIUS protocol design was to
   allow RADIUS servers to be configured to support new attributes
   without requiring server code changes.  RADIUS server implementations
   typically provide support for basic data types, and define attributes
   in a data dictionary.  This architecture enables a new attribute to
   be supported by the addition of a dictionary entry, without requiring
   other RADIUS server code changes.

This is totally misleading.  We should explain when this is true -- it is
mostly not true today since RADIUS is increasingly supporting dynamic
policies and thus always requires code change to handle the introduction of
any new attribute whether it is an attribute of an existing type or not.

As well the above appears under the complex attribute section 2.1.3 - It is
true also if we were to introduce a new attribute type.
----------

" On the RADIUS client, code changes are typically required in order to
   implement a new attribute. "

A new attribute or a new attribute type or a complex attribute? Which one?
----

  "The RADIUS client typically has to
   compose the attribute dynamically when sending.  When receiving, a
  RADIUS client needs to be able to parse the attribute and carry out
   the requested service.  As a result, a detailed understanding of the
   new attribute is required on clients, and data dictionaries are less
   useful on clients than on servers.

Many of the reasons given here are also true for servers. 

----------

  "The RADIUS server can be configured to send a new static attribute by
   entering its type and data format in the RADIUS server dictionary,
   and then filling in the value within a policy based on the attribute
   name, data type and type-specific value."

The above is only true if the new static attribute is an existing type.
--------

"Code changes can also be required in policy management and in the
   RADIUS server's receive path.  These changes are due to limitations
   in RADIUS server policy languages, which typically only provide for
   limited operations (such as comparisons or arithmetic operations) on
   the existing data types.  Many existing RADIUS policy languages
   typically are not capable of parsing sub-elements, or providing
   sophisticated matching functionality."

What is a RADIUS server policy language?  
RADIUS policy languages are not part of the IETF - the IETF did not conduct
a survey and thus cannot make statements such as these.
Most policy languages i know about contain REgExp that is sophisticated and
can extract any information out of any type. Typicially these have been
first added to be able to parse the RADIUS username.
Remove this paragraph.

------------------
The paragraph starting with "Given these limitations..." seems out of place
since it talks about new types, not complex types....
Then it talks about attribute specific parsing which is not really related
to new types.

Then it talks about the intoduction of new complex data types within RADIUS
attribute specification be avoided.

So now we have a new thing called a "new complex data type".    Dont we mean
new types and complex data types?


Also this paragraph repeats text made earilier  by the bullet points.
-----
"The only other exception to the recommendation against complex types
   is for types that can be treated as opaque data by the RADIUS server."

The assertions that the document is making in this sentence is wrong and we
should remove this statement.  We have seen examples like in RADIUS digest
where we flattened out an attribute to create more then a dozen attributes.
There are other cases as well.  Remove this paragraph.

------

 " If the RADIUS Server simply passes the contents of an attribute to
   some non-RADIUS portion of the network, then the data is opaque, and
   SHOULD be defined to be of type String.  A concrete way of judging
   this requirement is whether or not the attribute definition in the
   RADIUS document contains delineated fields for sub-parts of the data.
   If those fields need to be delineated in RADIUS, then the data is not
   opaque, and it SHOULD be separated into individual RADIUS attributes."


The document is attempting to define a RADIUS Server architecture.  We
should do this properly.  An equally valid architecture is the Diameter one
where the Server is composed of a lower layer defined by RFC 3588 and an
application layer.  We should state that as well.

Here the document gives an impression that the only "application" layer
possible is some foreign entity not part of the RADIUS server - that is just
not true today.

---------
2.1.4
" RADIUS specifications define how an existing service or protocol can
   be provisioned using RADIUS.  Therefore, it is expected that a RADIUS
   attribute specification will reference documents defining the
   protocol or service to be provisioned.  Within the IETF, a RADIUS
   attribute specification SHOULD NOT be used to define the protocol or
   service being provisioned.  New services using RADIUS for
   provisioning SHOULD be defined elsewhere and referenced in the RADIUS
   specification."


I am confused by what we are trying to say in this section.

-------

"New attributes, or new values of existing attributes, SHOULD NOT be
   used to define new RADIUS commands.  RADIUS attributes are intended
   to:"

Shouldn't this be a different section.

----------
2.2

" However, instead of doing this, vendors have defined the following
   non-standard VSA formats:"

The standard allows Vendors to define their VSA as they wish.  How can you
say "non-standard VSA formats"?

Howecer instead of following the recommendation vendors have defiend the
following:

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

  "All VSA schemes that do not follow the [RFC2865] recommendations are
   NOT RECOMMENDED.  These non-standard formats will typically not be
   implementable without RADIUS server code changes.  This includes all
   the above formats, as well as Vendor types of more than 8 bits,
   vendor lengths of less than 8 bits, vendor lengths of more than 8
   bits and Vendor-Specific contents that are not in Type-Length-Value
   format."

Its up to the vendors or SDO to decide and the specification allows for
that.  RFC2865 makes a recommendation and we should just leave it at that.
You can state the problems that this may create but I think the document is
out of bounds when it states that it is not recommended.  2865 makes a
recommendation and that is enough.

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

" Although [RFC2865] does not mandate it, implementations commonly
   assume that the Vendor Id can be used as a key to determine the on-
   the-wire format of a VSA.  Vendors therefore SHOULD NOT use multiple
   formats for VSAs that are associated with a particular Vendor Id.  A
   vendor wishing to use multiple VSA formats SHOULD request one Vendor
   Id for each VSA format that they will use."

What do you mean by on-the-wire format?  
     
---------
Section 3 is not only about data model issues.

We need a better name.
-----------

3.3

Why are we repeating the recommendations fo 5080.  Just remind people that
they need to read it.

---------

Section 5

"Where new RADIUS attributes encapsulate complex data types, or transport
opaque data, the security considerations discussed in Sectiont 2.1.4 SHOULD
be addressed"

The contents of 2.1.4 was moved to the security section
-----
5.1

The title should be Complex Types and New Data types no?

The text in this section should be fixed as well.  Sometimes it refers to
one where the other is also applicable.

-------

   "Any system consuming opaque data that originates from a RADIUS system
   SHOULD be properly isolated from that RADIUS system, and SHOULD run
   with minimal privileges.  Any potential vulnerabilities in the non-
   RADIUS system will then have minimal impact on the security of the
   system as a whole."

fear mongering a bit here and not true???  I can inject code in the opaque
data which could cause the system to run at higher privelages. Can we make
more concrete recommendations such as use care to validate the opaque data
for example to ensure no buffer overruns etc etc....It is pretty standard
practice to validate data coming from potentially untrusted sources or that
can be compromized along the way.
 


Appendix A

A.1

Simple Data vs Radius Data types.  They are the same no?  We need to use the
term consistantly throughout the document.

A.1.2

*Complex data types...which the RADIUS servers are expected to parse..."

I think you need to explain the model here.  This is a common theme
throughout the document.  What is a RADIUS server?

A.3





--
to unsubscribe send a message to radiusext-request@ops.ietf.org with
the word 'unsubscribe' in a single line as the message text body.
archive: <http://psg.com/lists/radiusext/>