Re: Comments regarding x509 authn based attribute profile (draft 3)

Thanks Tom, I will start applying these edits.

Rick, here are some comments related to both content and general edits which should probably be addressed prior to CD:

- The footer is incorrect (both filename, date, and copyright).

- Line numbers are per page.

- There are various references to the certificate distinguished name, or DN, (p 2 line 29, p 3 line 13, p 3 line 18) that should probably be qualified appropriately as "Subject DN" as there are other DNs in the cert.

- (p 2 line 30) s/.././

- (p 3 line 16) s/integrity/verify its integrity/

- (p 3 line 14, p 5 line 18) You reference the <Subject> element. Do you want to specifically say that the X509SubjectName format identifier should be used (it seems like that is what we want here). Additionally, this means mentioninig that the <Subject> should contain a NameID with that format whose value is the Subject DN from the principal's cert.

- (p 3 line 21) s/.././

- (p 4 line 1) s/5 and 6/1 and 2/

- (Section 2 vs. 3) In general there are some mis-matches here due to cut/paste. I would suggest reworking these sections, in that Encrypted/Signing Mode IS basic mode with these x items applied. This will remove all the redudant wording that is there, like SubjConfData, Audience, having one Assertion, one Attr Statement, etc... So section 3 becomes more like you must sign the request, sign the assertion, encrypt the NameID, encrypt the Assertion, Use SSL3/TLS, <Signature> comments, and the Use of XXX sections. But the basics of the profile would be in section 2. There are items below that would apply to both section 2 and 3 (where the text is equivalent -- in this case, I may only decribe them related to section 2).

- (p 5 lines 5 and 6) replace these with "The service provider MAY authenticate to the identify provider using this mode." I don't think you meant to require authentication in basic mode (as line 5 says MUST.

- (p 5 line 11 and 12) Replace with "The <AttributeQuery> and <Assertion> MAY be signed using this mode."

- (p 5 line 14 and 15, and p 5 line 21 and 22) I think you meant to say <Attribute> and not <Assertion> as there is NO assertion element inside of an Attribute query. Also, remove the reference to section 6 in SAMLProfile as I believe it does not reference these in any way. By allowing <Attribute>, the attr query requester can control which attributes and attribute value it wants returned. Is that correct? Vs. not including this and allowing the attr authority to control what it sends back.

- (p 5 line 19) Do you expect any SubjectConfirmationData in the request? Is it okay to send this?

- (p 5 line 24) So for any error, you just want no Assertions. The status is irrelavent -- and could be Success, Requester, or Responder? You may want to consider an UnknownPrincipal second level status (with a Success top level status) if the Subject DN is unknown. Also you may want to say that for the NO assertion case, either Requester or Responder is used to imply where the error is coming from. E.g., a repository error at the attr authority would imply Responder, whereas an invalid Subject format identifier of x509 subject dn would be Requester.

- (p 5 line 32) SubjectConfirmationData is only in basic mode and NOT enc/signing mode?

- (p 5 line 35, p 8 line 24) s/AudienceRestrictionCondition/AudienceRestriction/

- (p 5 line 27) You want to say that there should be only one <Assertion> and then one <AttributeStatement> inside the assertion -- similar to p 8 lines 17-21.

- (p 5 line 29) s/ConfirmationMethod/Method/

- (p 5 line 30) It's unclear if the decision of conf method is solely by the attr authority or can be implied by the requesting SubjectConfirmationData? Are you allowing either case?

- (p 5 line 34) If the requester asks for holder of key subj conf data, or for some reason the requester (out of band) asks for holder-of-key, you are assuming that the attr authority MUST store the user certificate, correct? Was that the intent?

- (p 6 line 2 to 4) I'm not sure I follow the phrase "as requested by the service provider." I don't believe there is any way the SP can request other conditions? If you are referring to SubjConfData or Attributes, then this should be mentioned in the prior bullets -- otherwise I'm not sure about this particular phrase.

- (p 6) In reference to othe SP having to understand and accept ALL the conditions -- I don't recall is this is similar to other profiles or not. I'm thinking that other profiles say you must adhere to condition A, condition B, etc... and other conditions are not relevant. For this profile you are saying that every single condition (even Condition extensions) must be valid or else the response must be rejected?

- (p 7 line 1) s/Mode Mode/Mode/

- (p 7 lines 5 and 6) s/provider MUST/provider MUST/

- (p 7 lines 5 and 6) Remove the line "In addition....". I'm not sure why this is here. I would have thought that the In addition would have said "Alternatively" It seems like you WANT the signature and that using the soap binding technique to authenticate the Attr Query is NOT sufficient -- which is this? I would suggest just saying "The service provider MUST authenticate to the identify provider as defined in [SAMLProf]." If you really want/require attr query signature then completely omit the soap client/server authn as this is just redundant (why bother?).

- (p 7, line 12 and 13) As above, why are you requirinig both soap binding authn and <Response> signing; particularly since you are requiring encryption.

- (p 7 line 17, p 8 line 11) add a reference to [RFC3280]. I'm not 100% sure, but isn't this

- (p 7 line 21) As before is an ID possible, or do we really want a NameID with x509 format.

- (p 7 line 30) s/previously established/previously established (out of band)/ If this is not the case, then how is the key established and maintained?

- (p 8 line 2) Delete this line -- it is 100% implied by the spec and only causes confusion as to why this is stated.

- (p 8 line 7 and line 34) s/that it was not modified in transit/its integrity/

- (p 8 line 23) s/provider./provider over the assertion./

- (p 8 line 35) Delete this line -- it is 100% implied by the spec and only causes confusion as to why this is stated.

- (p 8 and 9) Move section 3.2.2 after 3.2.3 (i.e., keep the order the same as in 3.1 -- encryption data then signature -- even though the technical processing occurs in a different order).

- (p 9 line 5) s/principal ./principal./

- (p 9 lines 10 to 13) I would suggest syncronizing the wording with p 7 lines 34-37.

- (p 10 line 10) s/cache user/cache, to a persistent store, user/ I assume this is what you mean, if not and you mean caching them in memory, then are you implying that the principal's identity (e.g., subject dn) must be encrypted in memory?

- (p 11 line 3 to 9) Update to using the standard docs.

- (p 13 line 13) update date.

Tom.

security-services message