Hi Glen, See below... On Mon, Aug 24, 2009 at 1:16 PM, Glen Zorn<gwz@xxxxxxxxxxx> wrote: > Donald Eastlake [mailto:d3e3e3@xxxxxxxxx] writes: > >> I have reviewed this document as part of the security directorate's >> ongoing effort to review all IETF documents being processed by the >> IESG. Document editors and WG chairs should treat these comments just >> like any other last call comments. > > Sorry for the rather slow response, but I honestly didn't know what to say. OK. >> This document defines seven RADIUS Attributes to support the >> implementation of 802.16 (WiMax) PKMv1 (Privacy Key Management version >> 1). I would guess that RADIUS can be used between the 802.16 Base >> Station and an authorization server but I don't know how you could >> tell. >> Maybe I missed it but it looks like the RADIUS protocol isn't >> mentioned anywhere in 802.16-2004. From the text in some of these >> RADIUS attribute descriptions, it appears that they are not used >> between the Subscriber Station and the Base Stations but may be the >> basis of 802.16 Attributes that are used on that hop. Given this, I >> think a paragraph is needed (maybe even accompanied by a little ASCII >> art) at the beginning show what's going on would be useful. > > Your comments seem to suggest a lack of familiarity with RFC 2865 and the > RADIUS protocol in general. Leaving aside the question of how one could > expect to usefully review a document that _extends_ a protocol w/o > understanding the protocol being extended, RADIUS is only defined between a > NAS (in this case, an 802.16 Base Station) and a RADIUS server. You have missed my point entirely. To evaluate extensions to RADIUS whose purpose is to support 802.16 seems to me to require not just a knowledge of RAIDUS but some knowledge of how RADIUS fits into 802.16 and what is needed for RADIUS, with these extensions, to support 802.16 security. My point was that, so far as I can tell, how RADIUS fits into 802.16 isn't documented in your draft or any document referenced by it. Doing a little more research, 802.16e-2005, which you don't reference, does begin to touch on this by at least specifying how EAP fits into 802.16. If above you are saying that the security of these new RADIUS attributes can be evaluated entirely based on a knowledge of RADIUS, I do not agree with this. If above, you are saying is that there is no need for there to be some explanation, in your draft or in some document referenced by it, of how RADIUS fits into 802.16 and that people who don't have an a priori knowledge of this should just keep their noses out of your document, I don't agree with that either. RFCs are supposed to be for the Internet community and there is presumably a reason that, for example, the RFC Editor requires acronyms that are not widely known to be expanded. Of course, it is not necessary or possible for most RFCs to be self contained and understandable in isolation, but I think it is reasonable to expect them to refer to other documents so that someone willing read enough and follow the reference chains can get a clear picture of what is going on. Thinking about it, I suggest that the Abstract be changed to something like the following and a similar change be made in the Introduction. (I note that the Introduction has a fairly long paragraph giving many details about 802.16 PKMV1 while failing to shed much light on how RADIUS or EAP fit into 802.16.) "RADIUS can be used by an IEEE 802.16 Base Station, acting as an EAP Authenticator, to communicate with a remote Authentication Server to authenticate 802.16 Subscriber Stations and support 802.16 Privacy Key Management Version 1. This document defines a set of additional RADIUS Attributes which are designed to enable this support." If you make this change and, due to my inferior knowledge, I've made any errors in the above paragraph, I'm sure you can fix it. >> Many document have security considerations section that only refer to >> other documents and may be missing specifics to the document contents. >> I think this document has the opposite problem good security specifics >> in the security consideration section but could usefully add >> references to the 802.16-2004 and RADiUS security sections. > > I'm not at all sure what 802.16 security has to do with RADIUS, but I guess > I can add a reference to RFC 2869 in the Security Considerations section. Because you are extending RADIUS for the express purpose of supporting the 802.16 base station security function of authenticating subscriber stations. This is a base station function whose mechanism, as far as I can see, is glossed over in IEEE Standard 802.16-2004, which you reference but which has absolutely no mention RADIUS, NAS, AAA, EAP, Authentication Server, or anything along those lines. As above, by further research, I found that the IEEE 802.16e-2005 amendment to 802.1-2004 at least mentions EAP. I believe that 802.16e-2005 should be added to the references in the draft. Is it really such a burden, to provide some security context for what you are doing, to say something like: "Security consideration for IEEE 802.16 appear in Section 7 of 802.16-2004 and of 802.16e-2005. Security considerations for RADIUS extensions appear in RFC 2869." or the like? >> The security considerations section rightly warns to protect against >> modification of the PKM-Auth-Key attribute. But is it really clear >> there is no problem with modification of the Security Association ID >> attribute or the attribute listing cryptosuites? > > No, apparently not. I had originally thought that modifying the list of > supported cryptosuites would just result in DoS, but that's not right. I'll > fix it. > >> The wording in Sections 3.1 and 3.2 see to almost be designed to allow >> the possibility of the multiple *-Cert Attributes carrying a >> certificate to appear in more than one Access-Request message. But I >> would assume that's not meaningful and/or was not intended to allow >> that. > > There is no way to do such a thing in standard RADIUS. That's what I thought and why I was puzzled as to why there was a more complex wording that appears to permit this. I suppose it is just the way the words struck me at the time I read them. But I would, instead of If multiple PKM-SS-Cert Attributes are contained within an Access-Request packet, they MUST be in order and MUST be consecutive attributes in the packet. have said These multiple PKM-SS-Cert Attributes MUST appear consecutively and in order within an Access-Request packet. and similarly for PKM-CA-Cert. Looking at this more closely now, I also think that the final "s" should be removed from "instances of the PKM-SS-Cert Attributes" and "instances of the PKM-CA-Cert Attributes". >> The table of attributes in Section 4 that gives the number of times >> each attribute can occur in different message types seems to have >> problems. Since there is no key giving it another meaning, I assume >> "0-1" means zero or one. But PKM-SS-Cert and PKM-CA-Cert are described >> as possibly occurring multiple times due to fragmentation of >> certificates. If the table is supposed to be in terms of logical >> attributes so that multiple PKM-SS-Cert attributes only count as one >> if they have parts of one certificate, then the table should say so. >> On the other hand, the PKM-SA-Descriptor attribute is shown as "0+", >> which presumably means zero or more, but the text description in 3.6 >> clearly says it can occur one or more times, which presumably would be >> written "1+". > > The relevant text from section 3.6 says "One or more instances of the > PKM-SA-Descriptor Attribute MAY occur in an Access-Accept message." RFC > 2119 says about the keyword "MAY", "This word, or the adjective "OPTIONAL", > mean that an item is truly optional"; this says to me that zero, one or more > instances of the PKM-SA-Descriptor Attribute can be in an Access-Accept > message, just in a more compact and formal way. If this is not clear, > however, I'm open to suggestions for alternate text. The wording is OK but I would suggest making it one letter longer so it starts "Zero or more ...". >> This whole table needs to be carefully checked, the >> inconsistencies resolved, and it should be clear if literal binary >> attributes or some sort of logical aggregate attributes (in the case >> of the "Cert" attributes at least), is being counted. > > I can add notes to the table regarding the "logical" vs. "physical" nature > of the PKM-*-Cert Attributes, as well as a key to the meaning of "0+", etc. > Is that OK? Yes. >> The text between the Section 6. header line and the Section 6.1 header >> line as well as the Section 6.1 header line itself seem superfluous >> and can be deleted. > > Fine. > > ... > Thanks, Donald _______________________________________________ Ietf@xxxxxxxx https://www.ietf.org/mailman/listinfo/ietf