Issue: WGLC comments about draft-ietf-radext-ieee802-00

[<Pasi.Eronen@nokia.com>]

WGLC comments about draft-ietf-radext-ieee802-00
Submitter name: Pasi Eronen
Submitter email address: pasi.eronen@nokia.com
Date first submitted: July 14, 2005
Reference: 
Document: draft-ietf-radext-ieee802-00
Comment type: T/E
Priority: 1
Section: 
Rationale/Explanation of issue:

Overall impression: NAS/QoS-Filter-Rule part is quite messy,
other parts are reasonably OK. I'll send my comments about
NAS-Filter-Rule in a separate email.


1) Section 1.1 contains some unnecessary terminology that is not
used anywhere later in the document: "Access Point (AP)",
"Association", "Port Access Entity (PAE)", "Station (STA)"


2) Section 2.2: "If accounting is enabled on the NAS, it MUST
generate an Accounting-Request(Stop) message upon session
termination."

So the NAS must send a Stop message, even though it has not sent
a Start message for this session yet? RFC 2866 seems suggest
that a session always has a Start message before Stop, but does
not seem to outright prohibit sending a stand-alone Stop
message. Is this a common practice, or are there any problems
associated with it?


3) Section 3.1: 802.1Q does not use the abbreviation "VLANID"
but rather "VID". Should this document also follow that
convention?  (but maybe VLANID is more understandable)


4) Section 3.1: "Padding bits SHOULD be 0 (zero)." Why "SHOULD"
instead of "MUST"? (according to RFC 2119, it would mean there
are valid reasons to send something else in some circumstances?)
(This same issue occurs also in Section 4.2.)


5) Section 3.3: If VLAN-Name has basically the same meaning as
Egress-VLANID, IMHO their names should reflect this. Perhaps
"Egress-VLAN-Name"?


6) Section 3.3: Using "START OF HEADING" (0x01) and "START OF
TEXT" (0x02) control characters to indicate tagged/untagged
might make it unnecessarily difficult to enter this attribute 
in a management interface... maybe something like 0x74/0x75
("t"/"u") or 0x31/0x32 ("1"/"2") would be easier?


7) Section 3.3: Probably should have reference to UTF-8? [RFC3629]


8) Section 3.4: "The table, expressed as an 8 octet string, maps
the incoming priority (if one exists - the default is 0) into
one of seven regenerated priorities." If the regenerated
priority is an integer in range 0-7, should that be "eight
regenerated priorities"?


9) A comment about the bit figures in Section 3 and 4. 
Currently, all the figures are basically identical: they contain
the RADIUS Type and Length fields, plus one other field
(variably called either "Integer", "String" or "Text"). 
The contents of this field are then described in English.

I think it would be much more understandable if the figure also
showed the structure. For instance, instead of this (Section 3.1),

+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
|     Type      |    Length     |            Integer          
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
|        Integer(cont.)         |  
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 

this would IMHO be more readable:

+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
|     Type      |    Length     |   Tag Option  |     (zero)   
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
 (zero) |   VLANID (12 bits)    |  
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 

(Note that I'm not proposing changing the attribute formats at all --
so no changes to any software --, just how they are presented in the
document. And we can keep all the zero padding there if we want...)


10) Section 5: If the semantics of Acct-EAP-Auth-Method are
identical to Diameter EAP (as Section 4.2 claims), then it can
also be included in Access-Accept messages. (Or if it can't,
then the semantics are not identical, and need to be described
in this document.)


11) Section 7: It looks like this security considerations
section was just copied from somewhere without much thought for
how well it actually applies to this document. Most of the text
looks redundant to me; we don't need to describe how to use
IPsec with RADIUS in every document that defines a new RADIUS
attribute.

On the other hand, there's exactly zero discussion about new
threats caused by the attributes defined in _this_ document.
And to me it looks like there are new attacks that e.g. a
malicious or compromised RADIUS server or proxy could do with
these attributes (compared to the situation where NAS does not
support these attributes). At least Egress-VLANID,
Ingress-Filters and NAS-Filter-Rule have some pretty obvious
possibilities for abuse... (and maybe some non-obvious as well)


12) References: Both 802.1Q and .1D look normative to me, but
RFC1321 and 802.11i probably are just informative.


13) References: Document cites [DiamEAP] and [NASREQ], but
they're not included in the references section. Both are
normative.


Best regards,
Pasi

--
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/>