Subject: Fwd: draft-zorn-radius-err-msg-10 (fwd)

From: [email-masked]
Date: Mon, 15 Dec 2008 18:47:19 +0000 (UTC)

Forwarding for a non-list member: 

>From [email-masked] Thu Dec 11 19:01:22 MEZ 2008 
Received: from ZEUS.TR-Sys.de by w. with ESMTP ($Revision: 1.37.109.26 $/16.3) 
      id AA023378481; Thu, 11 Dec 2008 19:01:21 +0100 
Return-Path: <[email-masked]> 
Received: (from ah@localhost) by z.TR-Sys.de (8.9.3 (PHNE_25183)/8.7.3) 
      id TAA22555; Thu, 11 Dec 2008 19:01:20 +0100 (MEZ) 
From: Alfred H�nes <[email-masked]> 
To: [email-masked] 
Message-Id: <[email-masked]> 
Date: Thu, 11 Dec 2008 19:01:20 +0100 (MEZ) 
Subject: draft-zorn-radius-err-msg-10 

Hello again, 
I also have reviewed your Internet-Draft draft-zorn-radius-err-msg-10 
and would like to submit a few comments. 

I have a couple of concerns regarding the structure of the document. 

First of all, it looks like Section 3 was intended as a summary 
of related pre-existing specifications for RADIUS. 
Thus, it comes to surprise that this section contains two inserts 
giving details for the new protocol elements that will be introduced 
later in the memo, in Sections 4 ff. 
To avoid confusion, I suggest to move these parts to the proper 
places later in the document; this will save text and improve 
the clarity of the document. 
Very few generally applicable text should be moved in the opposite 
direction, as well. 
I'll give the details in the document walk-through below. 

Subsequently, Sections 4..6 are structured for extensibility, 
which obviously is not needed in this document. 
The headlines always let the reader expect to come more, where 
there's always only a single new protocol object ro define. 
I strongly suggest to update the headlines and amalgamate these 
sections with their single subsection each. 

(1)  Section 3 

The draft text starts with: 

 3.  RADIUS Packet Format 

   Exactly one RADIUS packet is encapsulated in the UDP Data field 
   [RFC0768] where the UDP Destination Port field indicates 1812 
   (decimal). 

   When a reply is generated, the source and destination ports are 
   reversed. 

The topic of these two paragraphs, transport of radius messages, 
is not directly related to the "Packet Format" announced by the 
section headline. 
I suggest to move these two paragraphs to the end of the section, 
at best in a new subsection, 

 3.1.  RADIUS Transport 

There, it should at least be mentioned that TCP (and TLS-secured) 
transport of RADIUS packets is currently pursued in the RADEXT WG 
as well (for instance, see draft-ietf-radext-tcp-transport). 

I propose to move the short text currently in Section 4 next here 
in Section 3, in front of the diagram, and add a more fundamental 
explanation. In the 'Administrative Note' at the end of the section, 
the term "RADIUS proxy" is used as well, without priop explanation. 
Therefore, I recommend to also introduce this concept here. 

For instance: 

|  RADIUS is a request-reply protocol, where each request packet sent 
|  from a "RADIUS Client" to a "RADIUS Server" is responded to with a 
|  reply packet.  A "RADIUS proxy" relays, as an intermediate agent, 
|  requests and reponses between an original RADIUS client and the final 
|  RADIUS server, behaving as a server for the forme and as a client for 
|  the latter.  The RADIUS Packet type is determined by the Code field 
|  in the first octet of the Packet. 
| 
|  A summary of the RADIUS packet format is shown below.  The fields are 
                           ^^^^^^ 
   transmitted from left to right. 

Having that statement in front of the diagram, the sentence immediately 
below the diagram, 

         The Code field is one octet, and identifies the type of RADIUS 
         packet. 

... could be simplified to only say: 

|        The Code field is one octet, and identifies the packet type. 

... or even: 

|        The Code field is one octet. 

The second part of the explanation for 'Code' is not of general nature. 
IMHO, it should be removed from this section, and elaborated upon in 
Section 4: 

DELETE: 

|        The RADIUS Codes (decimal) defined in this document are as 
|        follows: 
| 
|           <MSG1> Error-Notification 

Again, in the explanation of 'Authenticator', the second part with 
the headline 'Notification Authenticator' comes to surprise here. 
I suggest to move it entirely into Section 4: 

DELETE: 

|        Notification Authenticator 
| 
|           The value of the Authenticator field in the Error- 
|           Notification packet is called the Notification 
|           Authenticator, and contains a one-way MD5 hash calculated 
|           over a stream of octets consisting of: the RADIUS packet, 
|           beginning with the Code field, including the Identifier, the 
|           Length, the Authenticator field from the packet to which 
|           this packet is a response, and the response Attributes, 
|           followed by the shared secret.  That is, 
| 
|           Notification Auth = 
|                     MD5(Code+ID+Length+RequestAuth+Attributes+Secret) 
|           where '+' denotes concatenation. 

In the 3rd paragraph of the 'Administrative Note', I suggest to add a 
few clarifying words in order to disambiguate the scenario described: 

         When using a forwarding proxy, the proxy must be able to alter 
         the packet as it passes through in each direction - when the 
         proxy forwards the request, the proxy MAY add a Proxy-State 
         Attribute, and when the proxy forwards a response, it MUST 
|        remove its Proxy-State Attribute if it added one.  [...] 
--- 
         When using a forwarding proxy, the proxy must be able to alter 
         the packet as it passes through in each direction - when the 
         proxy forwards the request, the proxy MAY add a Proxy-State 
         Attribute, and when the proxy forwards a response, it MUST 
|        remove its Proxy-State Attribute if it added one to the 
         vvvvvvv                                         ^^^^^^^ 
|        request.  [...] 

(2)  Section 4 

As noted above, I propose to simplify and clarify the structure 
of this section: 

|4.  Packet Types 
| 
|  The RADIUS Packet type is determined by the Code field in the first 
|  octet of the Packet. 
| 
|4.1.  Error-Notification 

The prose already has been moved inTo section 3, so the replacement 
is simply: 

|4.  New Packet Type: Error-Notification 

The explanation for 'Notification Authenticator' currently says: 

      Notification Authenticator 

|        The Notification Authenticator value is calculated from the 
|        Error-Notification packet, as described above. 

This sentence IMO does not match reasonably what was currently 
described in Section 3. 
As mentioned above, I strongly recommend to move that text (DELETED 
above) to this location; doing so allows to drop the above two-liner 
entirely: 

      Notification Authenticator 

|        The value of the Authenticator field in the Error-Notification 
|        packet is called the Notification Authenticator, and contains a 
|        one-way MD5 hash calculated over a stream of octets consisting 
|        of: the RADIUS packet, beginning with the Code field, including 
|        the Identifier, the Length, the Authenticator field from the 
|        packet to which this packet is a response, and the response 
|        Attributes, followed by the shared secret.  That is, 
| 
|           Notification Auth = 
|                   MD5(Code+ID+Length+RequestAuth+Attributes+Secret) 
| 
|        where '+' denotes concatenation. 

As I understand that (unchanged) text, with the single exception of the 
original Authenticator field from the errored packet, all these fields 
are the fields of the Error-Notification packet; that was not clear 
from the two-liner above! 

Another concern:  This new 'signature' again is stuck with the old- 
fashioned Keyed-MD5 construction; obviously, doing so violates the 
directive from the IESG and the IAB to equip all IETF protocols with 
capabilities for crypto-algorithm agility. 

(3)  Section 5 

Again, as indicated above, I strongly recommend to simplify the 
section structure: 

|5.  Attributes 
| 
|5.1.  Error-Code 
--- 
|5.  New Attribute: Error-Code 

(4)  Section 6 

Currently the text in Section 4 says: 

                           vv      vv 
|  The following sub-sections defines a new value for the Acct-Status- 
   Type Attribute [RFC2866]. 

That's bogus grammar anyway.  However, following the spirit of the 
above proposals for simplification, I suggest to change that entirely: 

|6.  Attribute Values 
| 
|  The following sub-sections defines a new value for the Acct-Status- 
|  Type Attribute [RFC2866]. 
| 
|6.1.  Acct-Error-Notification 
--- 
|6.  New Attribute Value: Acct-Error-Notification 

   This section defines a new value for the Acct-Status-Type Attribute 
   [RFC2866], 'Acct-Error-Notification' (<VAL>). 

(5)  Section 7 

Since this document does not define any new (sub-)namespaces to be 
managed by IANA, the present text seems to be misleading. 
Instead, the three specific assignments should be detailed there; 
for instance, following this pattern (cf BCP 26, RFC 5226): 

|  The criteria to be used by the Internet Assigned Numbers Authority 
|  (IANA) for assignment of numbers within namespaces defined within 
|  this document are identical to those given in [RFC3575]. 
--- 
|  IANA is requested to assign three code points within the RADIUS 
|  Parameters registry defined in [RFC3575], currently located at 
|  <http://www.iana.org/assignments/...> : 
| 
|  o  One RADIUS Packet Type 
| 
|     Type     Name                    Reference 
|     ------------------------------------------ 
|     <MSG1>   Error-Notification      [RFCthis] 
| 
|  o  ... 
| 
| 
|  o  ... 

(6)  Section 8 

|8.  Security Considerations 
| 
|  None. 

I bet the IESG will not be a friend of such brevity ... 

Kind regards, 
  Alfred H�nes. 

-- 

+------------------------+--------------------------------------------+ 
| TR-Sys Alfred Hoenes   |  Alfred Hoenes   Dipl.-Math., Dipl.-Phys.  | 
| Gerlinger Strasse 12   |  Phone: (+49)7156/9635-0, Fax: -18         | 
| D-71254  Ditzingen     |  E-Mail:  [email-masked]                     | 
+------------------------+--------------------------------------------+