Early Review of draft-ietf-radext-dtls-06
review-ietf-radext-dtls-06-secdir-early-weis-2013-10-10-00

I have reviewed this document as part of the security directorate's ongoing
effort to review all IETF documents being processed by the IESG. These comments
were written primarily for the benefit of the security area directors. Document
editors and WG chairs should treat these comments just like any other last call
comments.

This document describes requirements and implementation details regarding using
DTLS as a transport layer for RADIUS packets. It is a companion to RFC 6614
("TLS Encryption for RADIUS"), and this I-D references many of the sections in
that RFC rather than re-defining them. While the security considerations of
encapsulating RADIUS in TLS and DTLS are very similar there are a number of
operational issues where a UDP protocol is more advantageous than a TCP, and
vice versa. Both documents are worth specifying; providing more secure
alternatives to the simple RADIUS MD5 integrity checks is critical.

I have the following suggestions to tighten up the document.

1. Section 2.1, bottom of page 6. It is surprising that RADIUS/DTLS would cause
a change to the RADIUS data packet handling. If item (1) is referring to the
checks described in the RADIUS Length description (Section 3 of RFC 2865), I
don't understand why the RADIUS description would change. RFC 2865 seems to
clearly say that the RADIUS length check only covers bytes within the RADIUS
data packet, and so the length carried in the UDP packet would not be relevant
to RADIUS. If a RADIUS packet is decapsulated from DTLS I would expect exactly
the same situation. Is the use of a UDP packet length actually an
implementation issue, or is there another RADIUS length check that I'm missing?
(Note that if a change is made to this text, it might also need to be reflected
in Section 2.2.1.)

2. Section 2.2.1, top of page 8. Is the statement "Client identities can be
determined from TLS parameters" strong enough? Using the TLS parameters seems
to be a much stronger statement, especially in the wildcarding case mentioned
in the Security Considerations. Perhaps s/can be/SHOULD be/.

3. Section 2.2.1. There's no comment on whether RFC 6614 Section 3.4 item (2)
applies to RADIUS/DTLS. It would be good to be consistent and add something
here.

4. Section 3.2. The second paragraph says:

  "Servers MUST NOT accept DTLS packets on the old RADIUS/UDP ports.
   Early drafts of this specification permitted this behavior.  It is
   forbidden here, as it depended on behavior in DTLS which may change
   without notice."

This is in line with WG consensus, so is correct. But there are some other
sections that seem to conflict with this and probably need to be updated as
well:

- Section 2.2.1 has three instances of "When DTLS is used over the existing
RADIUS/UDP ports ...", which seems to be forbidden now. Should these all just
say "Section 3.4 item (N) applies to RADIUS/DTLS"?

- Section 5.1.1 says "The requirement to accept RADIUS/UDP and RADIUS/DTLS on
the same port makes this recommendation difficult to implement in practice."
This seems out of date.

- Section 10 says "
  "The main portion of the specification that could have security
   implications is a servers ability to accept both RADIUS and DTLS
   packets on the same port.  The filter that disambiguates the two
   protocols is simple, and is just a check for the value of one octet.
   We do not expect this check to have any security issues."
  I assume this should be removed since accepting both on one port is forbidden.

5. Section 5. I understand that RADIUS/DTLS creates connections where
RADIUS/UDP had none, so there are some new considerations around keeping track
of connections. But the section begins by saying that implementations are free
to choose their method. Since the implementation details of this section aren't
vital to understanding RADIUS/DTLS, and are frankly quite complex, it might be
better organization to put the text in Section 6 ("Implementation Guidelines").
This would make it easier to understand the protocol details.

6. Section 5.1. Why is this a MUST? Is it required for interoperability? If
not, I think it should be a recommendation.

  "A RADIUS/DTLS server MUST track ongoing DTLS client connections based
   on a key composed of the following 4-tuple...."

7. Section 5.1.1, bottom of page 11. What does the term "failed security" mean?
Is there a more precise way to say this?

8. Section 5.2.2, top of page 12. I assume the errors mentioned here are errors
in the decrypted RADIUS data packet. Why is it necessary to mention these?
Aren't they also necessary checks for RADIUS/UDP?

9. Section 6. Can you cite a rationale for choosing these PSK key sizes? Do
they represent a particular strength?

10. Section 10. Having a fixed key always comes with threats. It would be
helpful to mention that if the RADIUS/DTLS session becomes a RADIUS/UDP session
due to configuration or implementation error, that the fixed RADIUS MD5 key
would provide no security since the key was trivially known.

11. Section 10.1. It would be fair to say that using either RADIUS/DTLS or
RADIUS/TLS is recommended. I suggest adding "or RFC 6614" to the end of the 2nd
sentence.

12. Section 10.4. I don't think this document can make normative requirements
on previous RFCs so "NOT RECOMMENDED" should probably be in lower case. I would
also reword the second paragraph something like this to get the point across
better: "The use of RADIUS/DTLS can allow for the safe usage of wildcards. When
RADIUS/DTLS is used with wildcards clients MUST be uniquely identified using
TLS parameters, and any certificate or PSK used MUST be unique to each client".

13. Section 10.5. The first paragraph says "This requirement is due to security
considerations.". Could you state them please?

14. Section 10.5. The second paragraph says:

  "Similarly, any RADIUS traffic failing validation
   means that two parties do not share the same security parameters, and
   the session is therefore a security risk."

  Aren't the security parameters all in DTLS? This seems like a security
  consideration of the normal operation of DTLS. DTLS is going to detect if
  there's a mismatch between the two endpoints and so there won't be a security
  risk. I think this sentence should be omitted.

Hope that helps,
Brian