[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Gen-ART LC review of draft-ietf-radext-tcp-transport-06.txt



Glenn Kowack wrote:
> Major issues: None.
> 
> Minor issues:
> 
> Abstract:
> “The Remote Authentication Dial In User Server (RADIUS) Protocol has
> traditionally used the User Datagram Protocol (UDP) as its underlying
> transport layer.”
> 
> Does ‘traditional’ here mean as defined by the specification or is UDP
> use optional?

  It means "as defined by the spec".  Until now, UDP has been mandatory.

> “It is not intended to define TCP as a transport protocol for RADIUS in
> the absence of TLS.”
> 
> This might be interpreted by some as disingenuous.  Would it be clearer
> to say: “The specification only provides for use of TCP with TLS.  Other
> uses of TCP are out of scope.” ? 

  Later reviews suggested:

It is not intended to define TCP as a transport protocol for RADIUS in
the absence of a secure transport layer.

> *1. Introduction*
..
> That is, the document says there are inherent reasons why TCP is not
> appropriate, and then says the reason is lack of experience.  This
> should be unwound and clarified.  

  Later reviews suggested:

Since "bare" TCP does not provide for confidentiality or enable
negotiation of credible ciphersuites, its use is not appropriate for
inter-server communications where strong security is required.  As a
result "bare" TCP transport MUST NOT be used without TLS, IPsec, or
other secure upper layer.

"Bare" TCP transport MAY, however, be used when another method such as
IPSec [RFC4301] is used to provide additional confidentiality and
security.  Should experience show that such deployments are useful,
this specification could be moved to standards track.

  I think that text is clearer.

> *1.1. Applicability of Reliable Transport *
> 
> “For a system with a million logins a day running EAP-TLS mutual
> authentication with 15 round-trips, and having a packet loss probability
> of P=0.01%, we expect that 0.3% of connections will experience at least
> one lost packet. That is, 3,000 user sessions each day will experience
> authentication failure. This is an unacceptable failure rate for a
> mass-market network service.”
> 
> Can the document state an acceptable level of failure, and a supporting
> citation?

  Umm... no.  No one is willing to share data.  The above numbers are
based on hallroom conversations with roaming implementors.

> 
> Nits/editorial comments:
> *
> *
> 
> *1. Introduction*
> 
> *“The RADIUS Protocol has been defined in [RFC2865] as using the User
> Datagram Protocol (UDP) for the underlying transport layer.”*
> 
> Present tense would be clearer, unless 2865 is no longer applicable. 

  Agreed.

> “2.4, there are also some limitations:
> 
> * Unreliable transport. As a result, systems using RADIUS have to
> implement application-layer timers and re-transmissions, as described in
> [RFC5080] Section 2.2.1.”
> 
> Add a blank line between the ‘2.4’ line and the first bullet item, per
> the rest of the document.

  Done.

> “As RADIUS is widely deployed, and has been widely deployed for well
> over a decade, these issues have been minor in some use-cases, and
> problematic in others.”
> 
> The leading ‘as’ is awkward and not necessary, per this example: “RADIUS
> has been widely deployed for well over a decade, and still
> is.  Experience shows issues that are minor in some use-cases, and
> problematic in others.”

  Agreed.

> *2.3. Management Information Base (MIB)*
> 
> “The MIB Module definitions in [RFC4668], [RFC4669], [RFC4670],
> [RFC4671], [RFC4672], and [RFC4673] ……… SHOULD NOT re-use these MIB
> Modules to perform statistics for RADIUS over TCP connections.w”
> 
> Typo: trailing ‘w’ at end of paragraph. 

  Fixed.

> *2.4. Detecting Live Servers*
> 
> “As RADIUS is a "hop by hop" protocol, a RADIUS proxy effectively
> shields the client from any information about downstream servers.”
> 
> ‘effectively’ adds nothing here. However, this change is stylistic and
> at the discretion of the author. 

  I'll delete it.

> “When UDP was used as a transport protocol…”
> 
> Present tense is more appropriate.

  Agreed.

> *2.6.1. Duplicates and Retransmissions*
> 
> “In that situation, RADIUS request MAY be retransmitted to another
> server that known to be part of the same pool.”
> 
> s/that known/that is known/

  Fixed.

> [ Running a grammar checker (e.g., in MSWord) should have found this. ]

  vi + nroff? :(

> *2.6.4. Malformed Packets and Unknown Clients*
> 
> “After applying the above rules, there are still situations where the
> previous specifications allow a packet to be "silently discarded". In
> these situations, the TCP connections MAY remain open, or MAY be closed,
> as an implementation choice. However, the invalid packet MUST be
> silently discarded.
> 
> * Packet with an invalid code field
> 
> * Response packets that do not match any outstanding request
> 
> These requirements minimize the possibility for a misbehaving client or
> server to wreak havoc on the network.”
> 
> This section would be clearer if the two bullets immediately followed
> where they are first mentioned, after “…allow a packet to be “silently
> discarded.”"

  Agreed.

> Also, does the author mean ‘minimize’ or just ‘reduce’, above?

  "reduce".  I'll fix it for the next revision.

> *2.6.5. Limitations of the ID Field*
> 
> “…there would be no free Id to use…”
> 
> s/Id/ID/
> 
> This reviewer would like to observe that this is one of the most astute
> and downright clever Freudian observations about the IETF that he has
> seen in many years.  He takes his hat off to the author. Sadly,
> accurately using upper-case-D ‘ID’ regrettably MUST win out over
> literary invention.

  Yes... fixed for the next revision.

> “should reserve ID zero on each”
> 
> s/ID zero on each/ID zero (0) on each/

  Fixed.

>  “Implementors may be tempted to extend RADIUS to permit more than 256
> outstanding packets on one connection.  However, doing so will
> likely require fundamental changes to the RADIUS protocol, and as such,
> is outside of the scope of this specification.”
> 
> Two points are confounded here, as illustrated by:
> 
>      Implementors may be tempted to extend RADIUS to permit more than
> 256 outstanding packets on one connection.  However, doing so is a
> violation of a fundamental part of the protocol and SHOULD NOT be
> done.  Making that extension here is outside of the scope of this
> specification.

  Agreed.

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