Ben, Alan just posted -08 revision. Could you check it whether all your concerns have been addressed and you are OK with the revision.
- JOuni (doc shepherd) On Jan 23, 2014, at 9:05 PM, Alan DeKok <al...@deployingradius.com> wrote: > Ben Campbell wrote: >> *** 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. > > OK. > >> (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. > >> *** 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. > >> -- 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. > >> -- 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. > > That's left over from earlier drafts. I'll fix it. > >> 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. > >> -- 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) > > Yes. RADIUS/UDP has the same 4096 octet limit for sending and > receiving packets. > >> -- 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. > >> -- 3, 1st paragraph: >> >> Can you elaborate on the resulting "timeouts, lost traffic, and network >> instabilities"? > > RADIUS/UDP has no provisions for protocol negotiation. So simply > disabling UDP makes it look like the server is gone, rather than > transitioned to DTLS. > >> -- 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? > > Yes. As with most protocols, implementations allow the administrator > to use non0-standard ports. > >> 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". > >> -- 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. > >> -- 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. > >> -- 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. > >> -- 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. > >> -- 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. > >> -- 5.1.1, 7th paragraph: >> >> Is the "idle time" configurable setting on a per peer or global basis? > > Possibly both. That depends on the implementation. I don't see it as > being useful to either restrict the implementations, or make > recommendations here. > > The text here talks about idle timeouts per *session*. How the > implementation takes that from a configuration setting to a session is > up to the implementation. > >> -- 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? > > Perhaps "open sessions" is better than "tracked sessions". > > The idea is that it will either drop the new one, or drop an old one. > Either way, the "maximum sessions" value should not be exceeded. I'll > update the text. > >> -- 5.1.1, 10th paragraph: >> >> Should the second sentence contain a normative MUST? > > Yes. > >> Also, the 2nd and 3rd sentences taken together seem say "the server must >> keep the session open until it decides not to" > > It closes the session when the client asks for it to be closed, or > when the idle timeouts are reached. > > But really, the server is allowed to close sessions any time it wants. > I don't think we want to restrict that power. > >> How would an unauthenticated client close an active DTLS session? > > If a 4-tuple is re-used, AND the server closes an old session to > handle the "new" one. I'll clarify the text. > >> -- 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. > > I'll clarify the text. > >> -- 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. > > I'll clarify the text. > >> -- 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. > > OK. > >> -- 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. > >> -- 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. > > No. The previous text about client session management doesn't talk > about multiple sessions. It just talks about how to manage *one* session. > > This text is intended to suggest that managing multiple DTLS sessions > on one socket isn't necessary. > >> -- 6.1, 3rd paragraph: >> >> This seems to assume that all radius clients are implementd as multiple >> processes. Is that the intent? > > No. > >> 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. > > I'll copy the text from earlier in the document which discusses this. > >> -- 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. > >> -- 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? > > null crypto should be forbidden. > >> -- 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. > >> how to configure trust for a self-signed cert, etc. > > I'll avoid that, to be honest. > >> -- 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? > > I 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.) > > I've fixed the text to be consistent. > >> -- 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. > > The recommendation is a SHOULD so that it does not conflict with > dynamic discovery. > >> -- 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. > >> 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. > >> The last sentence refers to "the invalid server". What invalid server is >> that? None have been mentioned so far. > > The previous sentence talks about a "valid" server. I'll clarify. > >> -- 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. > >> -- 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. > >> -- 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. > >> >> >> *** 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. > >> -- 2.2.1, paragraph 5: >> >> should TLS parameters be DTLS parameters? > > Yes. > >> -- 2.2.2: >> >> Why a separate section to say sections 4 and 6 also apply? (The re-iteration >> seems unneeded.) > > OK. > >> -- 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. > > OK. > >> -- 4, 3rd paragraph: >> >> Do you really mean multiple independent _implementations_? Or multiple >> independent instances or processes, perhaps running the same implementation? > > All of that. > >> -- section 5 and children: >> >> In consistent use of "connection" vs "session". > > I'll fix that. > >> -- 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. > >> -- 6.1, 2nd paragraph: >> >> Source port? Source address? Both? > > Both. > >> -- 7: >> >> Only the first paragraph seems to be about implementation experience. > > OK. > >> -- 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. > > OK. > >> -- 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. > > The following sentence addresses this. The client has a fixed IP for > the server, and the server has a fixed IP for the client. > >> -- 10.3, 7th paragraph: "... pre-configured with a list of known public CAs." >> >> By pre-configured, you mean by the manufacturer or vendor, right? > > Yes. > >> -- 10.5, 2nd paragraph: >> >> Does this contradict guidance about using a private CA to identify multiple >> peers? > > No. > >> 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. > > That's what it says, so far as I can tell. > _______________________________________________ > radext mailing list > rad...@ietf.org > https://www.ietf.org/mailman/listinfo/radext _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
