Re: [radext] [Gen-art] Gen-Art Early Review of draft-ietf-radext-dtls-07

[Ben Campbell <[e-mail]>]

Jouni poked me to review 08. In the process of doing that, I found your email that I had somehow missed. Apologies for the late response. I've removed sections that do not seem to require further discussion or comment.

Thanks!

Ben.

On Jan 23, 2014, at 1:05 PM, Alan DeKok <[e-mail]> wrote:

> Ben Campbell wrote:
>> *** Major issues:

[...]

>> -- section 3, 2nd paragraph:  "... long-term use of RADIUS/UDP is NOT RECOMMENDED." 
>> 
>> While I agree with the sentiment, that's not an appropriate assertion for an experimental RFC. It would either need to go into an standards-track update to the RADIUS spec, or a BCP.
> 
>  OK.

Version 08 partially addresses this by changing "NOT RECOMMENDED" to lower case. I don't think that's sufficient; it's not a good idea to rely on case alone to distinguish normative from non-normative text. I propose changing the sense of the statement to something along the lines of "The author recommends...". 

(Unlike some reviewers, I will not go so far as to say the word "should" should never be used non-normatively, since the resulting text tends to be awkward. But I think the NOT RECOMMENDED/not recommended difference can be handled with more clearly non-normative text without introducing the same awkwardness.)

> 
>> (Also applies to the reiteration in 10.1)
> 
>  I'm less sure of that.  10.1 doesn't re-iterate the point above.  It
> makes a security recommendation.  i.e. allowing secure and insecure
> traffic from the same client is bad.
> 
>  This imposes no change on existing RADIUS/UDP clients, as they will
> only be sending RADIUS/UDP.  It does impose security requirements on
> RADIUS/DTLS clients and servers, which is the intent of the section.
> 

The text to which I refer says "It is RECOMMENDED that all RADIUS clients and servers implement this specification, or [RFC6614]." That seems to to apply a new normative statement to all RADIUS clients and servers, and does not in any way seem scoped to "RADIUS/DTLS clients and servers".


>> *** Minor issues:
>> 
>> -- General:  It might be worth some text on why this is experimental rather than informational. Is there any plan to evaluate the implementation results? Is there an expectation that a future RADIUS/DTLS spec could become standards-track? Is there any plan to evaluate the outcome of this document?
> 
>  It's probably easiest to reference Section 1.3 of RFC 6614.  The
> issues are the same.

I don't think that's the best approach. That discussion is specific to 6614. Referencing it would leave the reader to infer how that discussion would apply to this draft. I think it would be better to include a similar discussion here.

> 
>> -- Section 1, 2nd paragraph:
>> 
>> Isn't this true for almost any use of IPSec? Is there some specific reason this is worse for RADIUS than for other protocols?
> 
>  It's true for most protocols.

It's worth mentioning that.

[...]

>> The firewall rule recommendations in the 3rd sentence seems odd, since it seems like any protocol over DTLS would pass. Also, does this imply a recommendation that firewalls with DPI be used in the first place, since the absence of such a firewall has the same effect as having one that doesn't enforce the protocol? (Is there a reason a protocol spec should recommend firewall rules in the first place, other than to mention places where certain firewall rules might prevent interfere with operation?)
> 
>  I'll clarify it, and move it to the Security Considerations section.

The clarification added at the end of  section 10 in version 08 mostly addresses this. But I assume that the point is to make sure no one sends non-protected RADIUS over that port, but it doesn't prevent someone from sending some other protocol over DTLS over the same port, correct? If so, I think that's worth a mention.

[...]

>> -- 2.2 and children:
>> 
>> I find this section confusing as to where to find the authoritative text. Some, but not all, of the material from 6614 is repeated as normative text in later sections.  The description of this draft as applying 'semantic "patches"' to 6614 seems to imply you mean the 6114 text, with these patches applied, to be normative, which creates some potential conflicts. If, on the other hand, 2.2 (.x) is intended more as a informative description of the differences, please say so.
> 
>  What conflicts occur?  The intent is show this specification as a
> revision / patch from 6614.

On a quick re-review, I have not found a specific example of conflicting or redundant normative text in version 08. I thought I had run into some around peer authentication before, but either I was mistaken or the 08 updates corrected it.

[...]

> 
>> If so, it might make more sense to refer to the port by role rather than number when discussing port related behavior. (I note that 6614 only mentions the port number in the initial assignment, the IANA considerations, and the appendix.)
> 
>  Maybe.  That makes the text a bit more awkward.  "UDP/2083" would get
> replaced by "that or any other port you happen to use".
> 

It doesn't need to be that hard. One can start by saying something like the RADIUS/DTLS port is UDP/2083 by default, but administrators can assign other, non-standard values. Then all future mentions refer to "the RADIUS/DTLS port" rather than UDP/2023. But in any case, this is only a mild suggestion; with the addition of the disclaimer to section 3.1 of version 08, the future UDP/2083 references are less troublesome.


>> -- 3.2, last paragraph:
>> 
>> Can you elaborate on the bid down attack? Given that the use of dtls is configured, rather than negotiated, how would an attacker bid it down?
> 
>  When secure and insecure packets are allowed from the same client, a
> malicious or broken client can choose the insecure one.  That's bad,
> when the intent is to use DTLS.

One assumes a "malicious or broken" client already has access to the cleartext messages. Are we talking about a MiTM "client"? Are there attacks where a third party can induce the client or server to send unprotected packets, even with no signaled negotiation? I can imagine one for the specific case where a node falls back to clear text if DTLS packets fail; an attacker could induce failures for DTLS packets. (Thus the next comment.)

> 
>> -- 4, 2nd paragraph:
>> 
>> Seems like an (or maybe even the) important point would be that a client should not fall back to cleartext if a server appears to not support it.
> 
>  That's reasonable.
> 
>> -4, 3rd paragraph:
>> 
>> Is the recommendation to use a local proxy for all clients, or just those implemented as multiple processes?
> 
>  Only ones with multiple processes.

The text seems to say something to the effect of " Some RADIUS clients have historically been implemented as multiple independent processes, therefore [all] RADIUS clients should use a local proxy".  I think you mean to say something more like "Some RADIUS clients have historically been implemented as multiple independent processes; clients that are implemented that way should use a local proxy." 

> 
>> -- 5.1.1, third paragraph: "In those cases, the implementation MAY delete the underlying session"
>> 
>> Can you elaborate on why that's a MAY and not a SHOULD or MUST?
> 
>  There was a long discussion about this on the RADEXT list.  In short,
> the session MUST be closed when there are security issues, or when the
> tunneled data isn't RADIUS.
> 
>  When security is OK and the tunneled data is RADIUS, it's OK to
> silently drop the unexpected RADIUS packets.  Doing so doesn't cause any
> problems.

It would be helpful to add a couple of sentences to that effect. 


> 
> 

>> -- 5.1.1, 4th paragraph:
>> 
>> This paragraph rephrases 2119 normative language with more normative language. That creates confusion about which text is authoritative. I suggest either keeping the second paragraph and deleting the first, or make the second non-normative.
> 
>  I'm not sure what you mean here.  The two paragraphs discuss different
> things, so deleting one isn't possible.

The opening sentence of the second paragraph says, "The above paragraph can be rephrased more generically", then appears to go on to do so. That makes it sound like the second paragraph asserting effectively the same normative requirements in more general terms. If you intend each paragraph to stand alone, then I suggest deleting that sentence.

> 
>> -- 5.1.1, 6th paragraph: "The timestamp SHOULD be updated ..."
>> 
>> Why not MUST?
> 
>  The granularity could be 1 second, with packets being received 1000's
> of times per second.
> 

It would be helpful to elaborate on that in the text. Your comment makes me thing it would be also worth having some guidance on a reasonable resolution for the time stamp.

If you really expect 1 second resolutions, then it might be better to say something like "The timestamp MUST be updated if a valid message or heartbeat is received during the last time interval."

[...]

> 
>> -- 6, 1st paragraph:
>> 
>> You mention that these are implementation guidelines, and not part of the protocol. But the section contains 2119 style normative requirements. in general, that's not appropriate unless non-compliance impact interoperability or create some other Bad Thing, such as insecure behavior, excessive bandwidth usage, congestion, etc. Implementation guidance for behavior that is not externally observable should use non-normative.
> 
>  OK.
> 
>> -- 6, 2nd and 3rd paragraph:
>> 
>> The RECOMMENDation to allow administrative entry of keys and to derive keys from a PRNG seem contradictory.
> 
>  The idea is that admins shouldn't be entering "0xabababab" as a key.
> Instead, they get secure keys from a secure location.  People are
> notoriously bad at creating randomness.
> 

So paragraph 3 is a requirement on the administrator, rather than on the implementor? Or do you mean to say the implementation should provide tools for random key generation?

[...]


>> -- 9
>> 
>> Why not choose a new port? Is there absolute certainty this won't conflict with the previous usage? I do note that 6414 made the same choice, so I guess consistency with radius/TLS has some value. But that draft talks about compatibility between radsec and radius/tls, which is not mentioned here.
> 
>  No one is using UDP/2083 for anything.  Re-using it is OK.

Does that mean that RadSec is not used at all by anyone? or that this draft is compatible with it?

[...]

>> -- 10.1, 3rd paragraph:
>> 
>> It would be helpful to have guidance on how to match a certificate to a client or server identity,
> 
>  I'll add a note to see RFC 6614 Section 2.5.

I see you added that to section 10.3, 3rd paragraph. But does that contradict the statement in 2.2.1 that 6614 section 2.5 does not apply to RADIUS/DTLS?

[...]


>> -- 10.2, 2nd paragraph:
>> 
>> This is redundant with previous normative requirements. (Also contradictory, since the previous text said "SHOULD", and contradictory guidance on what to do when the limit is exceeded.)
> 
>  I've fixed the text to be consistent.

The paragraph is still redundant with the new text in 5.1.1 (which says you MUST either drop old sessions or limit creation of new ones.)


> 
>> -- 10.3, 4th paragraph:
>> 
>> Does this need to be as strong as SHOULD? Is this likely to conflict with dynamic discovery, should it ever exist?
> 
>  RADIUS is a critical piece of infrastructure.  Exposing your RADIUS
> server to the entire IPv4 range is a bad idea.

That's true of many protocols that don't have normative requirements about IP filtering. This is more an operational issue than an implementation one, and it may not be the best choice for all (or even most) deployments. There may be some advantage in filtering packets before they get to the DTLS layer, but IP filtering is trivial to defeat, and it has real administrative costs. For example, any peer network renumbering has to be coordinated across all of your RADIUS nodes.

I agree it makes sense to point out that IP filtering might be helpful. But I think a normative SHOULD is excessive.

> 
>  The recommendation is a SHOULD so that it does not conflict with
> dynamic discovery.
> 

If it stays normative, It would be worth mentioning that as an example of why it's a SHOULD rather than a MUST.

>> -- 10.3, 5th and 6th paragraphs:
>> 
>> Paragraph 5 says credentials should be statically configured. To me, "credentials" means "the certificate" in this case. That seems to disallow things like statically configuring a name that must match a certificate (perhaps signed by a CA.)
> 
>  Credentials for the client should be statically configured.  It could
> be a PSK, or a certificate.
> 
>  I'm not sure what "name" would match a certificate.  The document
> doesn't refer to names, and doesn't use names for anything.

The whole point of a certificate is to bind a key pair to an identity (often, a name.)

Example: I enter the name "trusted-peer.example.com" in a table. If a (perhaps dynamically discovered) peer presents certificate with a CN or SAN that matches "trusted-peer.example.com", and that certificate is issued by a CA that I trust, then I trust that peer. 

> 
>> Paragraph 6 seems to entirely contradict paragraph 5 by recommending private CAs, and accepting any peer that presents a cert signed by that CA. That's pretty much the opposite of statically configured credentials. (Paragraph 8 also seems to contradict the static configuration part.)
> 
>  Both client and server still need to be configured with the private
> CA.  The alternative is to allow anyone to connect with any credentials,
> which seems odd.
> 

That's static configuration of your trusted CA(s). That's not the same thing as static configuration of peer credentials, and it doesn't imply the static relationship between clients and servers mentioned in paragraph 5.

[...]

> 
>> -- 10.4:
>> 
>> The guidance in the last paragraph does not make sense. The section seems to say that NATs will break radius/dtls, so you should use dtls when behind a NAT.
> 
>  RADIUS/UDP clients should not be behind a NAT.  I'll clarify.

Do you mean RADIUS/UDP or RADIUS/DTLS? If the former, then that brings back the problem of making normative statements about the base protocol. Also, I'm still confused by the fact the the 2nd sentence of the last paragraph says "If clients are located behind a NAT gateway, then a secure transport such as DTLS MUST be used." Given that the previous paragraph seems to say that RADIUS/DTLS has issues over NAT, I'm surprised to see it listed as mitigating NATs.

> 
>> -- 10.6, last paragraph:
>> 
>> Can you elaborate on this? Why would an unauthorized 3rd party be able to get packets past the DTLS layer? OTOH, If an authorized client is sending invalid radius packets, wouldn't  you want to terminate the session?
> 
>  That was a *long* discussion on the RADEXT list.
> 
>  A server may be at protocol rev X, and a client may be at protocol rev
> X+1.  If the client sends packets allowed by X+1, the server should
> ignore them, as it does today with RADIUS/UDP.  Closing the connection
> is not necessary.

I think your previous answer about sending packets with the same 4-tuple answers this as well.

> 
>> -- 10.7
>> 
>> Redundant normative requirements (This is at least the third time the separate process issue and local proxy guidance has been described.)
> 
>  It's a security considerations section, and needs to mention this.  It
> refers to Section 6.1, and adds new text explaining the security
> benefits of the approach.

I agree it should be reiterated, but not _normatively_. In generally, you don't want to have the same normative requirement stated more than once. It makes sense for the security consideration section to say something like "Section XX recommends that clients use a local proxy"  But it's best to avoid any language that obscures which bit of text is authoritative for a given normative requirement. (There can be only one...)

I also notice the language here also fails to account for the fact that the requirement only applies to clients implemented as multiple independent instances. This could be fixed by something really simple like saying "... such clients ..." instead of "... clients... " in the second paragraph.

> 
>> 
>> 
>> *** Nits/editorial comments:
>> 
>> -- IDNits reports some issues; please check.
> 
>  They're fine.
> 
>> -- Abstract:
>> Please avoid citations in the abstract. Please consider removing the "scare quotes".
> 
>  OK.
> 
>> -- Section 1, 5th paragraph:
>> 
>> it seems like the RADIUS problems that this does not solve are in generally solved by RADIUS/TLS. If so, it would probably be worth a mention.
> 
>  OK.
> 
>> -- section 2.1, 4th paragraph from end: 
>> 
>> Seems like there are other changes as well. For example, you don't include "Use DTLS" as one of the changes in RADIUS/DTLS from RADIUS/USP.
> 
>  OK.
> 
>> -- 2.2 and children:
>> 
>> In my strictly personal opinion, the approach of patching 6614 transfers a lot of effort from the author to the reader. A reader will basically need to keep both docs open side by side to understand this.  I understand the desire to avoid duplicating normative text, but I think that the balance here has swung too far away from readability.  (OTOH, see my related comment under "minor issues).
> 
>  I think re-doing the document is a non-starter at this point.
> 
>  Having done this myself, the bulk of the implementation work in doing
> DTLS is the UDP layer.  Most everything else can be re-used directly
> from RADIUS/TLS.
> 

[...]

>> -- 5.1.1, 2nd paragraph:
>> 
>> What is an "entry" session?
> 
>  The 4-tuple is logically a table tracking (4-tuple) -> (session data).
> Each one is an entry.
> 
>  I'll try to clarify the text.
> 

I don't see a change in version 08. In particular, I'm confused by "Sessions (both 4-tuple and entry) ..." That sounds like you have two kinds of sessions, 4-tuple sessions and entry sessions. I think you mean something more like "Both the 4-tuple and the session table entry for a session ...." right?

> 
>> -- 7:
>> 
>> Only the first paragraph seems to be about implementation experience.
> 
>  OK.

No change in version 08.
> 

[...]