review of draft-ietf-radext-ieee802-01.txt

[Jari Arkko <jari.arkko@piuha.net>]

I reviewed this draft. Overall, this draft is in good
shape, but some minor mistakes and question
marks are noted below. In addition, the document
is silent on the use of these attributes in Diameter,
which needs further work.

The title should probably mention redirection, too.

Is this for informational or standards track? In some
sense the latter would make more sense, as this is
an important functionality that many people will need.

Technical:

>         org-url        =  http_URL  
>                           ;Note: Syntax for http_URL defined in  
>                           ;[RFC2616], section 3.2.2 
>         redir-url      =  http_URL 
>  
s/_/-/g

And please run ABNF checker from tools.ietf.org:

l2-filter-body = ( l2-proto " from " l2-address " to " l2-address ) / l2-rmon-str
*; HEXDIG UNDEFINED*
l2-proto = "l2:ether2" [ ":0x" 1*4HEXDIG ]
*; DIGIT UNDEFINED*

l2-rmon-str = "l2:" 1*DIGIT *( "." 1*DIGIT )

*; DQUOTE UNDEFINED*
tunnel-id = DQUOTE *( TEXTDATA / ( "%" 2HEXDIG ) / ( "\" 1*( "\" ) *1*"\"* ) / ( "\" DQUOTE ) ) DQUOTE
log-rule = "cnt"
*; LF UNDEFINED*
rule-delim = LF****

**

I think it would be preferrable to employ a full BNF (with
the exception of http-url), i.e., define these four items, too.

>   5.  IANA Considerations 
Do you want to say something about allocation policy
on new enumerated values?

>   2.5.  NAS-Filter-Rule 
This is quite complicated. Do we have implementation
experience that this works and is well defined?

>      The security mechanisms in [RFC2865] and [RFC3576] are primarily 
>      concerned with an outside attacker who modifies messages in 
>      transit or inserts new messages. They do not prevent an authorized 
>      RADIUS server or proxy from inserting attributes with a malicious 
>      purpose in message it sends. 
>  
Or deleting, for that matter...

> The 
>      filtering attributes specified in this document enable explicit 
>      description of layer 2 and layer 3 filters as well as redirection, 
>      and therefore extend the filter language described in [RFC3588]. 
This is OK, but the document is silent on what effect this
has in Diameter. These effects are important because, for
instance, 3GPP wireless LAN interworking is based on running
Diameter on the 3GPP operator side and RADIUS in the access network
side. Also, the RADEXT charter says: "All RADIUS work MUST be
compatible with equivalent facilities in Diameter. Where possible,
new attributes should be defined so that the same attribute can
be used in both RADIUS and Diameter without translation. In
other cases a translation considerations section should be included
in the specification."

Technically, we have two alternatives to get there. The first
alternative is to make NAS-Filter-Rule(R) an extension of
NAS-Filter-Rule(D). Given the same the same attribute
name we appear to intend to do this already... The other
approach would be to define NAS-Filter-Rule-Radius as a
new Radius attribute and then allow its use in Diameter as
well.

I prefer the former approach. This is because I'd hate to see
incremental improvements in various things such as filters
always re-create new attributes upon every modification.
But we still need language in this draft to explain the
situation. And does that mean this document "updates RFC
3588 and 4005"?

Editorial:

>      In this document when we refer to Blocking of IP traffic we mean 
>      Filtering of IP traffic.
Not sure if Capitalization is Needed.

>       0                   1                   2                   3 
>       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
>      |     Type      |    Length     |  Tag Option   |   Pad (12-bit)          
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
>        Pad   |    VLANID (12-bit)    |  
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
>
>      Type 
>    
>         TBD 
>    
>      Length 
>    
>         6    
>    
>      Integer 
>    
The picture and text do not match. Show two pictures:
one with "Integer" and then another one with the
detailed substructure.

>         "l2:"       Prefix on any base encapsulation rule to designate 
>                     as rule as such. 
>  
"a rule as such"?

> TODO / TBD

Two different placeholders are used. Just use TBD, or better yet,
TBD-by-IANA so that they can better search for these.



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