Re: [radext] WGLC #2 for draft-ietf-radext-dtls-04

[Alan DeKok <[e-mail]>]

Peter Deacon wrote:
> 2.1 says min and max packet length MUST be unchanged when using
> RADIUS/DTLS.
...
> s/corrsponding/corresponding/
> s/totalling/totaling/

  Fixed.

> It is confusing to say decrease is allowed while also previously saying
> min/max packet lengths MUST be unchanged.  Would expect there be no
> difference in min/max size of RADIUS payload within DTLS period.

  It's a trade-off with implementations.  The specs say max 4K RADIUS
packets.  However, implementations may interpret this as max 4K buffer
for UDP application data.  In which case the DTLS overhead requires that
the encapsulated RADIUS packet is smaller.

  Any choice here isn't perfect.  Do you have suggestions for better text?

> 2.2.2 "We re-iterate that much of [RFC6614] applies to this document.
>    Specifically, Section 4 and Section 6 of that document are applicable
>    in their entirety to RADIUS/DTLS."
> 
> RFC 6614 section 6 dedicates a few sentences to TCP specific properties
> which do not apply to RADIUS/DTLS.

  Do you have suggested text for the draft?

> 4. "Adding these
>    parameters means that the client MUST start using DTLS to the server
>    for all new requests.  The client MUST, however, accept RADIUS/UDP
>    responses to any outstanding requests."
> 
> MUST does not seem appropriate.  We have no business in what client
> elect to do with outstanding requests after a security configuration
> change.

  Yes, we do.  We're writing the standards here, which means we must
address security, migration, implementation cost, etc.

  In this case, there are no known issues with a client accepting
responses to packets it previously sent.  The goal here is to ensure a
safe and productive transition between RADIUS/UDP and RADIUS/DTLS.

> 5. "We note that [RFC5080] Section 2.2.2 already mandates a duplicate
>    detection cache.  The connection tracking described below can be seen
>    as an extension of that cache, where entries contain DTLS sessions
>    instead of RADIUS/UDP packets."
> 
> I think bringing this up is likely to cause more confusion than
> necessary. Tuples and authenticator usage are different, session
> lifecycle is different and state logic is different (You would not
> ignore anything while a response is pending)

  Do you have suggested text for the draft?

> I think it might be helpful to note RFC5080 in the context of continuing
> to support this mechanism and to continue to do it at the RADIUS packet
> layer rather than DTLS or you're likely to end up on the wrong side of
> the DTLS sequence window.

  I'm not sure what that means.

> 5.1 "Last Packet
>      A variable containing a timestamp which indicates when the last
>      valid packet was received for this connection.  Packets which are
>      "silently discarded" MUST NOT update this variable."
> 
> As long as the packet was valid while being silently discarded it should
> count for the purpose of last packet.

  Why?  What benefit does that offer?

> 5.1.1 - I still think we can do better on the UDP / DTLS disambiguation
> using the 4 byte header I described earlier or by explicitly requiring a
> server knob to declare what protocol would be accepted from a given
> source address.

  The draft already defines a "DTLS Required" flag.  Servers use it to
decide which protocol is accepted from a given client.

>    "Sessions (both key and entry) MUST deleted when a TLS Closure Alert
>    ([RFC5246] Section 7.2.1) or a TLS Error Alert ([RFC5246] Section
>    7.2.2) is received.  When a session is deleted due to failed
>    security, the DTLS session MUST be closed, and any TLS session
>    resumption parameters for that session MUST be discarded, and all
>    tracking information MUST be deleted."
> 
> Not all TLS Error Alerts are fatal.  Recommend "or a fatal TLS Error Alert"

  OK.

>    "Sessions MUST also be deleted when a RADIUS packet fails validation
>    due to a packet being malformed, or when it has an invalid Message-
>    Authenticator, or invalid Request Authenticator.  There are other
>    cases when the specifications require that a packet received via a
>    DTLS session be "silently discarded".  In those cases,
>    implementations MAY delete the underlying session as described above.
>    There are few reasons to communicate with a NAS which is not
>    implementing RADIUS.
> 
>    The above paragraph can be rephrased more generically.  A session
>    MUST be deleted when non-RADIUS traffic is received over it.  This
>    specification is for RADIUS, and there is no reason to allow non-
>    RADIUS traffic over a RADIUS/DTLS connection.  A session MUST be
>    deleted when RADIUS traffic fails to pass security checks.  There is
>    no reason to permit insecure networks.  A session SHOULD NOT be
>    deleted when a well-formed, but "unexpected" RADIUS packet is
>    received over it.  Future specifications may extend RADIUS/DTLS, and
>    we do not want to forbid those specifications."
> 
> This is redundant.

  As is noted in the text.

>    "Once a DTLS session is established, a RADIUS/DTLS server SHOULD use
>    DTLS Heartbeats [RFC6520] to determine connectivity between the two
>    servers."
> 
> s/server(s)/peer(s)/ ?

  Maybe "systems".

>   "A server may also use watchdog packets from the client to
>    determine that the connection is still active."
> 
>    "The
>    timestamp SHOULD be updated on reception of a valid RADIUS/DTLS
>    packet.  The timestamp MUST NOT be updated in other situations."
> 
> The RADIUS packet layer does not see heartbeats. Should this cause a
> change in Last Packet?

  No.  That is for RADIUS packets, not DTLS heartbeats.

>  Do you intent for sessions to idle out and
> expire even with active DTLS layer keepalives?

  Yes.  If there's no RADIUS traffic for a long time, there are few
reasons to keep the session up.

>    "This session "idle timeout" SHOULD be exposed to the administrator as
>    a configurable setting.  It SHOULD NOT be set to less than 60
>    seconds, and SHOULD NOT be set to more than 600 seconds (10 minutes).
>    The minimum value useful value for this timer is determined by the
>    application-layer watchdog mechanism defined in the following
>    section."
> 
> The recommended maximum idle timeout is too low in my view.  Resetting
> connections should be as rare as possible as clients now have the added
> burden of correctly guessing whether packets were dropped on wire or
> dropped on DTLS stack in addition to possibility server may not be alive.

  Did you read the text about RADIUS watchdog packets and DTLS
heartbeats?  No "guessing" is required.

> There are no timing guidelines provided for transition to "idle" state.

  Do you have suggested text for the draft?

> Mismatch of idle expectations between client and server could trigger
> unnecessary delay which could be mitigated by separating client and
> server expectations so there is no overlap in the recommended settings.
> Clients severely outnumber servers.

  Do you have suggested text for the draft?

> 5.1.3
> 
>    "For RADIUS/DTLS, any RADIUS packets which are subsequently silently
>    discarded MUST result in the removal of the associated entry and key."
> 
> 5.1.1
> 
>    "There are other
>    cases when the specifications require that a packet received via a
>    DTLS session be "silently discarded".  In those cases,
>    implementations MAY delete the underlying session as described above."
> 
> These statements appear to conflict with MUST vs MAY language.

  I'll delete the text in 5.1.3.  It's redundant.

  Alan DeKok.