AD review of draft-ietf-radext-tcp-transport-05

["Romascanu, Dan (Dan)" <dromasca@avaya.com>]

Please find below the AD review of AD review of
draft-ietf-radext-tcp-transport-05. I believe that the document will be
ready for IETF Last Call only after the issues raised in this review are
addressed. 

The issues are divided into Technical (T) and Editorial (E). 

T1. The document is marked as EXPERIMENTAL. It would be useful if it
mentions in one of the introductory sections what are the goals and
limitations (if any) of the experimental deployment and what would be
the success criteria of the experiment that would allow for taking the
document to the standards track later. 

T2. Section 2.2: 

>  Since these ports are unused by existing RADIUS implementations, the
   assigned values SHOULD be used as the default ports for RADIUS over
   TCP.

Why is this a SHOULD and not a MUST? 

T3. Section 2.3 needs a complete re-writing: 

> The MIB Module definitions in [RFC4668], [RFC4669], [RFC4670],
   [RFC4671], [RFC4672], and [RFC4673] each contain only one reference
   to UDP.  These references are in the DESCRIPTION field of the MIB
   Module definition, and are in the form of "The UDP port" or "the UDP
   destination port".

Actually the count is not accurate. 4688 and 4670 have each two MIB
objects that refer to UDP ports, 4669 and 4671 have none, 4672 and 4673
have one each, but only in 4672 the port is referred to as UDP port
(although it is clear from the context that UDP is assumed). 

> Implementations of RADIUS over TCP SHOULD re-use these MIB Modules to
   perform statistics counting for RADIUS over TCP connections.
   However, implementors are warned that there is no way for these MIB
   Modules to distinguish between packets sent over UDP or over TCP
   transport.  

This does not work. One cannot know what current implementations do.
Some may behave as described here, but other may not and could count
statistics strictly over UDP as defined in the DESCRIPTION of the MIB
objects. You cannot count on that the agent implementations will do, so
there is no interoperability.  

>   Implementations of RADIUS over TCP SHOULD include the protocol (UDP)
   or (TCP) in the radiusAuthServIdent, radiusAuthClientID,
   radiusAuthClientIdentifier, radiusAccServIdent, radiusAccClientID, or
   radiusAccClientIdentifier fields of the MIB Module.  This information
   can help the administrator distinguish capabilities of systems in the
   network.

This is also broken. First it is a change in the semantics of the
respective objects. This cannot be done even in a MIB document that
updates the respective RFCs because of the SMIv2 rules. Then these
objects are of a syntax of SnmpAdminString - so including the protocol
is based on non-interoperable heuristics. Last, describing the
capabilities (UDP, TCP or both) does not indicate what runs and is
activated at a certain moment, not to speak cannot indicate anything
about specific sessions between clients and servers. 

The correct solution is for the MIB documents to be updated in the
future with new MIB objects of appropriate syntax. For the time being
the only thing that this section should say is that the MIB modules do
not support RADIUS over TCP and that they will need to be updated in the
future. 

T4. The following section in 2.6.1 is confusing: 

>  Much of the discussion in this section can be summarized by the
   following requirement.  RADIUS requests MAY be re-transmitted
   verbatim only if the following 5-tuple (Client IP address, Client
   port, Transport Protocol, Server IP address, Server port) remains the
   same. 

Actually this is not a retransmission on the same connection, so I do
not know why it needs to be mentioned at all. This aparently contradicts
the first paragraph of 2.6.1 which says 'As TCP is a reliable transport,
implementations MUST NOT retransmit RADIUS request packets over a given
TCP connection.'. 

T5. The last paragraph in 2.6.1: 

>   The above requirement is necessary, but not sufficient in all cases.
   Other specifications give additional situations where the packet is
   to be considered as a new request.  Those recommendations MUST also
   be followed.

Having a MUST requirement over undefined 'other specification' is not
implementable. I would suggest to either clarify what are these 'other
specifications' or take this out. 


E1. Please expand TLS at first occurrence. 

E2. The Introduction section speaks about 'the default Internet MTU
(576). Strictly speaking this value is not standardized in any place,
and later in section 1.1 reference is made to RADIUS packets being
restricted to 1500 octets in size. Looks like an unnecessary
inconsistency. 

E3. Section 1.2 - the syntax in the RADIUS server definition is
inconsistent with the syntax of the rest of the definitions

E4. Section 2.6.7: 

>  We RECOMMEND that implementors of this specification
   familiarize themselves with TCP application programming concepts.  We
   RECOMMEND also that existing TCP applications be examined with an eye
   to robustness, performance, scalability, etc.

The use of capitalized (a la 2119) keywords in a personalized phrase
does not make much sense. RECOMMEND is not a keyword actually. I suggest
rephrasing. 

Regards,

Dan



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