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

Request Review of draft-ietf-radext-dtls
Requested rev. no specific revision (document currently at 13)
Type Early Review
Team Security Area Directorate (secdir)
Deadline 2014-05-13
Requested 2013-08-08
Authors Alan DeKok
Draft last updated 2013-10-10
Completed reviews Genart Early review of -07 by Ben Campbell (diff)
Genart Last Call review of -10 by Ben Campbell (diff)
Secdir Early review of -06 by Brian Weis (diff)
Secdir Last Call review of -10 by Brian Weis (diff)
Assignment Reviewer Brian Weis
State Completed
Review review-ietf-radext-dtls-06-secdir-early-weis-2013-10-10
Reviewed rev. 06 (document currently at 13)
Review result Has Issues
Review completed: 2013-10-10

Review
review-ietf-radext-dtls-06-secdir-early-weis-2013-10-10

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