Additional comments on core-02

["Philpott, Robert" <rphilpott@rsasecurity.com>]

Here are some additional comments on core from our internal RSA reviews.  Higher-priority items are marked with ***:

 

1. *** Lines 613-622 re: subject confirmation: First, since this is really dealing with how to treat confirmations, I recommend moving it into the section on <SubjectConfirmation>.  Next, these are two very long run-on sentences and the phrasing is a bit confusing.  I suggest this alternate text:

A <Subject> element can contain both an identifier and one or more subject confirmations which a SAML relying party can verify when processing an assertion. Once verified, the relying party can treat the entity presenting the assertion as the entity that the SAML authority associates with the name identifier.

A <Subject> element can also contain one or more subject confirmations without an identifier being present. In this case, once verified, the relying party can treat the entity presenting the assertion as the entity that the SAML authority associates with the claims in the assertion.

 

2. *** Section 2.4.1.2 re: KeyInfoConfirmationDataType: How does this type get used?  There is no schema-defined element that uses this type.  I expected to see a <KeyInfoConfirmationData> element (or should it be <KeyInfoSubjectConfirmationData>?) defined that was of this type that could be present in the SubjectConfirmation element instead of a more general <SubjectConfirmationData> element.
3. Lines 652-654 describes an example using <SubjectConfirmationData> to hold  a <ds:KeyInfo> element.  It is a confusing example once you read section 2.4.1.2 on KeyInfoConfirmationDataType.
4. Lines 682-689: The use of the phrase “while confirming itself” is pretty confusing and could really use some additional explanatory text.  Does this mean the attributes are only used when the entity presenting the assertion is the “subject” of the assertion?
5. *** Section 2.5.1.5 re: <ProxyRestriction> uses statements like “a relying party MUST NOT issue an assertion…”.  This violates the definition of a relying party since only asserting parties issue assertions.
6. Lines 978-979 re: the note that AuthorityBinding was removed.  Should we clutter up the core spec with these notes?  I think they should just go in the 1.1/2.0 differences section of the tech overview. If we want to include them, we should go through the spec to mention them all.  For example, we don’t mention that the AttributeNamespace attribute was removed in section 2.7.3.1. Lines 1383-1384 do describe removal of <RespondWith>.
7. Line 1046: While I know what the “or both” clause is referring to once you read the schema, it’s not a very clear description.
8. Line 1057: The “in addition to the assertion issuer” clause is confusing. What does it mean?
9. The intro section of section 3 (Protocols) should mention that in some cases, responses can be created without a previous request being received.
10. Lines 1429-1431: Might want to add an explanation of why a responder might choose NOT to respond to a request.
11. Section 3.2.2 re: StatusResponseType.  Why isn’t it defined as abstract?
12. Lines 1549-1551: We should also mention why a responder might NOT use second-level status codes (e.g. to protect against probing attacks).
13. Line 1554: ErrorAttrNameOrValue – This is the only error status code with “error” in the name.  Shouldn’t this be “InvalidAttrNameOrValue” for consistency?
14. Line 1558: InvalidNameIDPolicy” – he description implies it’s just for an unsupported name ID “format”. Is “format” synonymous with “policy”?
15. *** Section 3.4: Why is the response to an <AuthnRequest> not an <AuthnResponse> instead of <Response>?  All other protocols have symmetry in their names. This was a major complaint by developers trying to understand the new specs.
16. Section 3.4: The list of actors does not include “Identity Provider”.
17. *** Lines 1880-1884 re: “Presenter”: What if the AuthnRequest is sent to the IDP via the Artifact binding?  In this case, the presenter is the entity that presents the <ArtifactResolve> request.  Throughout the section, the “Presenter” term is, I think, presumed to be carrying the <AuthnRequest> message. For example, in lines 1894-1896, a “presenter” does not really send an IDP an <AuthnRequest> message.  It’s the SP that sends it.  The SP may use the user agent to transfer the message, but the SP may also send it via the Artifact binding.  You might be able to get by with just redefining the term “presenter”, but if you are using the Artifact binding, the test isn’t really accurate.
18. Line 1903; “authenticate or authorize the requested subject to provide a service”? Makes it sound like the subject is providing a service.  I assume you mean that the relying party mentioned earlier in the sentence is what is providing the service.  If so, need to clean up the phrasing.
19. Line 1922: “expects to govern” – not sure what you mean by this.  Can it be clarified?
20. *** Lines 2011-2015 re: the “AllowCreate” attribute: The definition says that False is the default, which means you should not create new identifiers.  This attribute is not described in Profiles or in Bindings.  According to the current definition, this means that an IDP could never create identifiers (persistent, transient, or any other kind) for the principal.
21. *** Line 2071: <GetComplete> seems like an odd element name for this.  Why not something like <IDPListRef>?  There is also no explanation of what dereferencing of this URI would return.  Is it an XML fragment containing <IDPEntry> elements?  It’s included in an example in Profiles, but is never explained.
22. Lines 2970-2971: “the signature MUST first be calculated and in place, and then the element encrypted” doesn’t parse – “and in place”?

 

Rob Philpott
Senior Consulting Engineer 
RSA Security Inc.
Tel: 781-515-7115
Mobile: 617-510-0893
Fax: 781-515-7020
mailto:rphilpott@rsasecurity.com