Subject: [secdir] Secdir review of draft-ietf-radext-dtls-06

From: Brian Weis <[email-masked]>
Date: Fri, 4 Oct 2013 19:45:24 -0700

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

---

From: Alan DeKok <[email-masked]>
Date: Sat, 05 Oct 2013 21:34:12 -0400

Brian Weis wrote:
> 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.

  DTLS is replacing UDP as a transport later for RADIUS.  So any text
referring to UDP decapsulation should be replaced with DTLS decapsulation.

  RFC 2865 Section 3, top of page 15 says:

   ...  Octets outside the range of the Length field
      MUST be treated as padding and ignored on reception.

  i.e. a UDP packet of 8K may only carry 32 octets of valid RADIUS data.
 The extra 8K-32 octets are discarded by the RADIUS layer.

> 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.)

  The text guides implementors to not be lazy.  The length of a RADIUS
packet is determined by the RADIUS header, and *not* by the amount of
data read from it's encapsulating protocol.

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

  Good point.  I'll make it SHOULD.

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

  It does apply.  I'll add text to that effect.

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

  Yes.  I'll fix it.

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

  I've deleted the old text, and fixed the surrounding text.

>   I assume this should be removed since accepting both on one port is forbidden. 

  Yes.

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

  Arguably yes.  I'm inclined to leave it as-is.  The existing text
motivates and frames the discussion of connection management.

>    on a key composed of the following 4-tuple...."

  I'm willing to make that change.  I'm curious as to how else the
connections would be tracked.  Would a single DTLS session send packets
using multiple source IPs / ports?  If so, how would the peer correlated
a random packet to a particular session?

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

  Perhaps "failing security requirements".  When a connection doesn't
meet the security configurations set by the host, the connection should
not be accepted.

> 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 RADI…

  When a RADIUS/UDP packet is malformed, it just means that the packet
gets discarded.

  When you've set up a DTLS connection with someone, and he then sends
you a malformed RADIUS packet, it means the entire DTLS session should
be discarded.

  There was some discussion in the WG about this.  In the end, I can't
see any reason to maintain a DTLS session when the other end isn't
sending RADIUS packets inside of it.

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

  They come from the WG discussion, if I recall.

> 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 erro…

  I'll add some text.

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

  OK.

> 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 someth…

  OK.

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

  The next paragraph explains.

>   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 s…

 Someone may allow administrator configuration of the shared secret
inside of DTLS, instead of fixing it as "radius/dtls".

 The two paragraphs should say:

Section 5.1.1, above, requires that DTLS sessions be closed when the
transported RADIUS packets are malformed, or fail the authenticator
checks.  The reason is that the connection is expected to be used for
transport of RADIUS packets only.

Any non-RADIUS traffic on that connection means the other party is
misbehaving, and a potential security risk.  Similarly, any RADIUS
traffic failing validation means that two parties do not have a common
same shared secret, and the session is therefore unauthenticated.

  I'll issue a new draft ASAP.

---

From: Jouni Korhonen <[email-masked]>
Date: Mon, 7 Oct 2013 07:45:03 +0300

Thanks Brian for a thorough review. We'll look into these comments and propose
a resolution (as Alan has already started to do).

- Jouni

---

From: Brian Weis <[email-masked]>
Date: Wed, 9 Oct 2013 20:38:19 -0700

Hi Alan,

The new draft looks good. I just have one comment below.

> data read from it's encapsulating protocol.

Thanks for the explanation.  Would it also be accurate to say this?

     (1) The Length checks defined in [RFC2865] Section 3 MUST treat the
      decrypted DTLS data octets outside the range of the Length field
      as padding and ignore it on reception.

I think something like that would be a lot clearer.

Thanks,
Brian

---

From: Alan DeKok <[email-masked]>
Date: Fri, 11 Oct 2013 14:01:35 -0400

> I think something like that would be a lot clearer.

  OK.  I'll add that in.