I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. This is an early review at WGLC last call. Document: draft-ietf-radext-dtls-07 Reviewer: Ben Campbell Review Date: 2013-11-18 Summary: This draft is on the right track, but there are significant open issues that should be resolved before it progresses. [Note: This review is different from the usual Gen-ART review due to the fact that it's an early (as in prior to pubreq) review, and that the draft is intended to become an experimental RFC. The gen-ART review process is tuned for drafts at IETF last call or later. It's a little hard to figure out what criteria should be used for drafts at an earlier point in the life-cycle. That being said, I reviewed this as if it were near-final, and apologize if that turns out to be too strict for an early review. I used similar review criteria as I would for a standards-track draft, since this draft still specifies protocol with the hope of interoperable implementations. The only difference comes from a few comments explicitly about the experimental status of the draft.] *** Major issues: -- General: The draft needs an overhaul of it's use of normative language. There appears to be redundant (and possibly contradictory) language for the same requirements sprinkled throughout. There are also cases of normative language being used for internal implementation guidance, which is not appropriate as described by RFC 2119. (See the minor issues section for details--most of the instances would not qualify as major issues by themselves, but I think they constitute a major issue in the aggregate.) -- 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. (Also applies to the reiteration in 10.1) *** 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? -- 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? -- Section 1, 4th paragraph: The second sentence mentions that firewall rules do not need to be changed. The following sentence recommends a change to firewall rules. 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?) -- section 2.1, 5th paragraph (3rd paragraph after bullet list) : "Implementations MUST support encapsulated RADIUS packets of 4096 in length..." Please be more precise than "MUST support". Specifically, does "MUST support" indicate you must support both sending and receiving of that size? (since 4096 would generally exceed PMTU even for RADIUS/UDP) -- 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. -- 3, 1st paragraph: Can you elaborate on the resulting "timeouts, lost traffic, and network instabilities"? -- 3.1: The section describes UDP/2083 as the "default port". But later sections describe port related behavior in a way that makes it it look like the impementations must always use 2083, which makes it more than just a default. Is the administrator allowed to choose some other port for any reason? 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.) -- 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? -- 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. -4, 3rd paragraph: Is the recommendation to use a local proxy for all clients, or just those implemented as multiple processes? -- 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? -- 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. -- 5.1.1, 6th paragraph: "The timestamp SHOULD be updated ..." Why not MUST? -- 5.1.1, 7th paragraph: Is the "idle time" configurable setting on a per peer or global basis? -- 5.1.1, 8th paragraph: What does it mean to "track" sessions? Also, it seems like the "SHOULD stop creating" guidance contradicts later guidance that least recently used sessions might get dropped instead? -- 5.1.1, 10th paragraph: Should the second sentence contain a normative MUST? Also, the 2nd and 3rd sentences taken together seem say "the server must keep the session open until it decides not to" How would an unauthenticated client close an active DTLS session? -- 5.2, 1st paragraph: The normative requirement for a client to use heartbeats _or_ the application level watchdog algorithm seems to contradict the normative guidance that a server SHOULD use both. -- 5.2, 3rd paragraph, 1st sentence: I would hesitate to phrase this that a client may violate the previous normative SHOULDs. It would be better to phrase this as describing th econsequences should the client ignore the SHOULD. -- 5.2, 2nd to last paragraph: Does this have actual interop or security issues beyond saying, "it's harder to implement and you might screw it up"? If not, it seems counter to the 2119 guidance on when normative language is appropriate. It would be more properly non-normative implementation guidance. -- 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. -- 6, 2nd and 3rd paragraph: The RECOMMENDation to allow administrative entry of keys and to derive keys from a PRNG seem contradictory. -- 6.1, 1st paragraph: Does the guidance to use connected sockets remove previous normative requirements about session management and tracking? If not, please indicate the difference. -- 6.1, 3rd paragraph: This seems to assume that all radius clients are implementd as multiple processes. Is that the intent? Also, it's better not to use normative requirements for internal implementation choices. Describe the externally visible behavior normatively. You can give implementation advice in the form of example strategies to fulfill the black-box normative requirements. -- 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. -- 10, last paragraph: You describe the use of null crypto as an implementation or configuration error. Was it intended to be forbidden from intentional use? Is there a need to remove the fixed secret requirement for null crypto, or to disallow null crypto entirely? -- 10.1, 3rd paragraph: It would be helpful to have guidance on how to match a certificate to a client or server identity, how to configure trust for a self-signed cert, etc. -- 10.1, 4th paragraph: -- 10.1, last paragraph: Why does the historic use of shared secrets matter, since this document requires all implementations to use a fixed value? Or do you mean the use of poor secrets as PSKs? -- 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.) -- 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? -- 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.) 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.) The last sentence refers to "the invalid server". What invalid server is that? None have been mentioned so far. -- 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. -- 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? -- 10.7 Redundant normative requirements (This is at least the third time the separate process issue and local proxy guidance has been described.) *** Nits/editorial comments: -- IDNits reports some issues; please check. -- Abstract: Please avoid citations in the abstract. Please consider removing the "scare quotes". -- 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. -- 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. -- 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). -- 2.2.1, paragraph 5: should TLS parameters be DTLS parameters? -- 2.2.2: Why a separate section to say sections 4 and 6 also apply? (The re-iteration seems unneeded.) -- 3.1, last sentence: Really, 2.2.1 doesn't describe that. 6114 describes that. Better to cite the source than to cite a citation to the source, especially when that citation doesn't mention the issues at all. -- 4, 3rd paragraph: Do you really mean multiple independent _implementations_? Or multiple independent instances or processes, perhaps running the same implementation? -- section 5 and children: In consistent use of "connection" vs "session". -- 5.1.1, 2nd paragraph: What is an "entry" session? -- 6.1, 2nd paragraph: Source port? Source address? Both? -- 7: Only the first paragraph seems to be about implementation experience. -- 10, 2nd paragraph: "All of the security considerations for RADIUS apply to the RADIUS portion of the specification." The next paragraph seems to contradict this. -- 10.3, 2nd paragraph: "... a client has a fixed IP address for a server ..." This is ambiguous. Do you mean the server knows the client address, or the client knows the server address? It sounds like the latter, but later text makes me wonder if you meant the former. -- 10.3, 7th paragraph: "... pre-configured with a list of known public CAs." By pre-configured, you mean by the manufacturer or vendor, right? -- 10.5, 2nd paragraph: Does this contradict guidance about using a private CA to identify multiple peers? I don't think you intend it that way; I assume you mean the cert presented by a client must be unique, but as written it's easy to assume that you mean the server has to have unique certs configured for each client, which it would not if it were to allow any arbitrary client that has a cert signed by a private CA. _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
