Document: draft-sterman-aaa-sip-03.txt Reviewer: Michael A. Patton Date: 18 augusti 2004 Summary: I'm not actually sure what to say. There is apparently some confusion as to what's up with this document... The agenda Harald sent out shows this as an individual submission going for Standard. As such it is clearly an end-run of IETF policy since it hasn't met the requirements for Standard. Even if it meant to say Proposed, it would seem to be an end run of at least the AAA or RADext WG, and possibly SIP and/or HTTP related WGs as well. Further, I'm not sure I have enough knowledge in this area to say more than that it seems to be written in a style that can be hard to follow. I guess my summary is that I may not be qualified to judge this, but if I am I think it would be ready for Experimental with some minor wordsmithing (see below), but for standards track I'd like to see some more extensive cleanup and possibly some tweaks to the protocol itself to improve the complexity (i.e. simplify the interdependancies). As is it wouldn't be the worst spec to make it to PS, but it might be close. Details: ======== Understanding this document requires understanding of RADIUS, DIAMETER, HTTP Digest authentication, and several other references. Since I understand none of these (I have only a vague understanding of one of the six or seven Normative references, and I'm not even sure why that one is Normative as my understanding didn't really apply anywhere), my review is only on how readable and clear this document is in isolation and I just assume that someone knowledgeable about each of those has checked consistancy with them. Also, I'm assuming that someone knowledgeable in the security area has verified that this is secure, it appears so on the surface, but I am not an expert... I encourage Harald to make sure that all of those have, in fact, been covered by someone. Having said that, I would note that overall it seems a very clunky design. However, it may just be a combination of the fact that it isn't explained well and that complete understanding takes knowing several other disparate areas which leads to that conclusion, all of the festoons and qualification clauses may be a result of these other specifications that I don't have the time to understand before the telechat... One specific small example -------------------------- I'm confused as to why the actual value in 2.1 is sent as 32 hex characters representing 16 octets rather than just sending the octets themselves. Minor clarifications -------------------- In section 2.14 there is no directive name, which appears in all (most) of the other similar sections that talk about copying values from the HTTP-style request to the RADIUS Access-Request message. And (I assume this is one of those things I'd know if I understood the underlying specs), what does "auts" mean? Section 2.16 confuses me somewhat. Digest-Stale is something you put in an ACCEPT to say that you REJECT the request. So why isn't it in a REJECT message? This paragraph in section 3.1 confused me for a while: The RADIUS client examines the (Proxy-)Authorization header of an incoming HTTP-style request message. If this header is present and contains HTTP digest information, the RADIUS client checks the 'nonce' parameter. If the 'nonce' value has not been sent by the RADIUS client, it responds with a 401 (Unauthorized) or 407 (Proxy Authentication Required) to the HTTP-style client. In this error response, the RADIUS client sends a new nonce. after some thought, I think I got it, but could I suggest the following improved version? If the change does not reflect what was meant, then the paragraph definitely needs rewriting to say what is meant. The RADIUS client examines the (Proxy-)Authorization header of an incoming HTTP-style request message. If this header is present and contains HTTP digest information, the RADIUS client checks the 'nonce' parameter. If the 'nonce' value is not one that was sent by the RADIUS client, it responds with a 401 (Unauthorized) or 407 (Proxy Authentication Required) to the HTTP-style client. In this error response, the RADIUS client sends a new nonce. The title of section 4 seems to be misleading. I think the section is talking about gateways between RADIUS and DIAMETER, so a better title might be "Interoperation with DIAMETER" or "Translation to DIAMETER". I would also like to see a short paragraph at the start of the section stating that's what the section is and describing the scenario it envisions. In particular is the translation one-way (i.e. for backwards support of RADIUS devices when the backend has been converted to DIAMETER, or can it actually support translation the other way? Because of my lack of understanding of both RADIUS and DIAMETER, I find it hard to decide if the described translation supports either, let alone both... RFC3310 is listed as Informative Reference, but I think it's actually Normative. In 2.14 it's referenced for the content of one of the attributes. Of course, if you reference my notes below you'll see that this was one of the sections I really didn't understand, so I may be wrong, but I'm pretty sure it needs to be Normative. But it's also referenced in 3.2 as "see for details" of the server behaviour. Typos ----- Section 1.2 "whishing" => "wishing" Section 1.5 "User- Password" => "User-Password" (remove extraneous space) Section 1.5 "in the document" => "in this document" Section 2.12 "Th HTTP-style client" => "The HTTP-style client" The page break at page 21 that separates the caption for the table from the table itself is very unfortunate. In the final RFC preparation a little attention to making this come out on the same page would be nice. Section 4 "The gateway construct a" => "The gateway constructs a" Appendix A "/or" => "for" Finally ------- Presumably between the I-D and RFC publication, sections 8 and 9 will be deleted renumbering section 10 to 8.