[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: FW: Review of Radius Guidelines 11
From: Avi Lior [mailto:avi@bridgewatersystems.com]
...
> -------
>
> 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.
We can simplify that text. Do you have any suggestions, or comments
on the suggestions from Hannes?
> ---------------
> Section 2:
> Please define Standard Space
IANA managed RADIUS attribute space.
> Please define complex attributes. (you do so much later) or give a reference
> to where they are defined.
Attributes of type *other* than the 5-6 "basic" types already defined.
(RFC 2865, plus additions)
> "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...."
OK.
> -----------------
> 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.
The types should be defined before they are used in the document.
> 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?
The doc doesn't say it's OK to create new data types in the standard
space. It says that the IETF has change control over IETF protocols.
It says that no one else has change control over the "standard space".
> -----------------------
>
> " 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.
Yes.
> 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.
Solutions are explicitly *out of scope* of this document.
> --------------
> 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?
Grouping of attributes?
> What about guidance on how to achieve this?
Solutions are explicitly *out of scope* of this document.
> --------------------
> Section 2.1.3 Complex Types
>
> Can we not re-write this section to be a little more concise and to the
> point?
Suggested text is welcome.
> "However, some standard attributes do not follow this format."
> Please give an example of an attribute.
Hmm... I'm not sure what that text is doing there. I suggest simply
deleting it.
> "Attributes that use sub-fields instead of using a basic data type are
> known as "complex attributes". "
>
> What are sub-fields.
I'm not sure what's unclear about that.
> 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?
We don't. Solutions are explicitly *out of scope* of this document.
> ---------
>
> " * 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.
Of course. So why would we remove text that says entering data as the
proper type?
> --------
> * 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.
OK.
> --------------
>
> 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.
Sorry, but this point has been hashed to death.
Every single RADIUS server implementation supports at *least* the
functionality above. If your implementation supports a more complex
dictionary capability, then it is *also* covered by the above text.
There is no need to change the text.
> 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.
Of course. But that is outside of the scope of the guidelines docuemnt.
> ----------
>
> " 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 text is clear as-is. "new" attribute means "new attribute".
> ----
>
> "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.
OK.
> ----------
>
> "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.
Or if your dictionary definitions support more than existing types.
> --------
>
> "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?
Do you have a suggested definition, or is "policy language" unclear?
> RADIUS policy languages are not part of the IETF - the IETF did not conduct
> a survey and thus cannot make statements such as these.
Addressing implementation limitations is well within the scope of
IETF. The alternative is to pretend that we know nothing about
implementations because we didn't commission an official survey.
> 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.
Yes. That functionality goes back if not to the Livingston server,
then at least to multiple implementations after 1996.
> Remove this paragraph.
There is no demonstrated need.
> ------------------
> The paragraph starting with "Given these limitations..." seems out of place
> since it talks about new types, not complex types....
I don't see why. A signed 8-bit integer is a new type that could
require code changes. It's definitely not a complex type.
> Then it talks about attribute specific parsing which is not really related
> to new types.
That could be "parsing of new types", or refer to "ad hoc" attributes
that define a new complex type unique to that attribute.
> 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?
Yes.
> 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.
There is no need. The rest of the text in the document should make it
clear that design is a balancing act, not a set of hard and fast rules.
> ------
>
> " 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.
No. It is acknowledging that RADIUS servers may exist within a wider
framework.
Is this idea controversial?
> 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.
Defining new RADIUS architectures is *out of scope* of this document.
> 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.
The text contains no reference to an "application" layer. And there
is nothing in the text that restricts it to *only* an application layer.
So your comment is therefore not relevant to the document.
> ---------
> 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.
OK.
> -------
>
> "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.
Like... where?
> ----------
> 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"?
RFC 2865 defines a suggested VSA format. Vendors are free to do as
they wish, including defining formats not recommended by the standards.
So... this means "non-standard format".
> 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.
So the guidelines document shouldn't have guidelines for attribute design?
> 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.
If that's true, the *entire* guidelines document is unnecessary.
However... vendors have created *unnecessarily* different VSA
encodings. The IETF is well within it's right as owner of the RADIUS
protocol to recommend how vendors use it.
> ---------------
>
> " 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?
I'm unsure how to answer that. The definition should be clear.
> ---------
> Section 3 is not only about data model issues.
>
> We need a better name.
Do you have a suggestion?
> -----------
>
> 3.3
>
> Why are we repeating the recommendations fo 5080. Just remind people that
> they need to read it.
Do you have suggested replacement text?
> ---------
>
> 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
OK.
> -----
> 5.1
>
> The title should be Complex Types and New Data types no?
Sure.
> The text in this section should be fixed as well. Sometimes it refers to
> one where the other is also applicable.
Do you have suggested replacement text?
> -------
>
> "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.
Do you have suggested replacement text?
>
>
> 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.
Sure.
> 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?
Suggest some text.
Alan DeKok.
--
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/>