Public Comment

[comment-form@oasis-open.org]

Comment from: trscavo@gmail.com

Name: Tom Scavo
Title: Research Programmer
Organization: National Center for Supercomputing Applications
Regarding Specification: SAML Attribute Sharing Profile for X.509 Authentication-Based Systems

Document identifier: sstc-saml-x509-authn-attrib-profile-cd-02

My comments re previous versions of this document (for reference):
http://lists.oasis-open.org/archives/security-services-comment/200503/msg00000.html
http://lists.oasis-open.org/archives/security-services-comment/200503/msg00001.html
http://lists.oasis-open.org/archives/security-services-comment/200606/msg00057.html


Comments/Suggestions:

[section 2.2] The use case in subsection 2.2 is applicable to basic mode, but it fails to motivate encrypted mode.  Why would you encrypt at the message level if there were only two endpoints taking part in the exchange?  Discuss a use case scenario that motivates encrypted mode.

[lines 124--126] Delete this sentence since the SP is not required to sign the attribute query, at least in basic mode, which is what this use case illustrates.  (However, the requester SHOULD authenticate to the responder, as mentioned below.)

[line 131] The phrase "after verifying that the service provider is a valid requester" is problematic since client authentication is not required, at least in basic mode, which is what this use case illustrates.

[section 3] Sections 6.3 and 6.4 of the SAML Profiles specification [SAMLProf] specifies that the exchange SHOULD be mutually authenticated and integrity protected, yet basic mode of this profile specifies MAY rather than SHOULD.  (There is no mention of server authentication by the way, which implies SHOULD in this case.)  I don't think this specification should water down the SAML Profiles specification in this way.

[section 3] Move lines 147--158 into the subsections with the rest of the normative text.

[section 3] Add section: "3.3 Use of Metadata" (see below for content).

[line 147] I suggest this sentence be changed to the following: "The service provider SHOULD authenticate to the identity provider."  In that case, however, this profile inherits from section 6.3.1 of the SAML Profiles spec [SAMLProf] so the sentence may be dropped and the issue of client authentication may be relegated to section 5.  (Note that server authentication is not mentioned in section 3, so it too is implied by section 6.3.2 of [SAMLProf].)

[line 174] Why MUST the response contain exactly one assertion?  This is overly restrictive.  Indeed, I have a use case that returns two attribute assertions to the SP, one an assertion of federated attributes and the other an assertion of VO attributes.  Unfortunatetly, this profile precludes such a scenario.

[lines 176--177] Why MUST the assertion contain exactly one attribute statement?  This is overly restrictive.  I don't have a use case for more than one attribute statement, but unless there's good reason for this, I'd suggest deleting these lines.

[section 4] I would recommend calling this mode "Enhanced Mode", not "Encrypted/Signed Mode".  There are numerous reasons for this.  First, the terms "Basic Mode" and "Enhanced Mode" are consistent.  Second, and more importantly, there is encryption and signing used in "Basic Mode" (at the transport level) so it's inaccurate to use the term "Encrypted/Signed Mode" exclusively.  Third, "Basic Mode" doesn't preclude encryption (as generally specified by SAML 2.0) so again it's inaccurate to use the term "Encrypted/Signed Mode" exclusively.

[section 4] There is a lot of redundancy in this section.  This section should build on the previous section, not repeat it.  In particular, this section MUST satisfy the requirements of section 3.3, Use of Metadata.

[section 4] Evidently, both the <Response> and the <Assertion> MUST be signed (lines 194--195 and lines 250--251, resp.).  Is this really necessary?  I suggest the <Assertion> MAY be signed if the situation warrants.

[section 4] Move lines 187--200 into the subsections with the rest of the normative text.

[section 4] Add section: "4.3 Use of Metadata" (see below for content).  Include in this section any metadata requirements that only apply to Encrypted Mode.

[line 245] Why MUST the response contain exactly one encrypted assertion?  This is overly restrictive.  Indeed, I have a use case that returns two attribute assertions to the SP, one an assertion of federated attributes (which SHOULD be encrypted) and the other an assertion of VO attributes (which MAY be encrypted).  Unfortunatetly, this profile precludes such a scenario.

[lines 248--249] Why MUST the assertion contain exactly one attribute statement?  This is overly restrictive.

[sections 4.1.2 and 4.2.2] Since both modes may employ encryption, most of the content in these sections should be pulled out of section 4 and put in sections 3 or 5.  Any content specific to the mode (such as the FIPS requirement) should remain in section 4, of course.

[sections 4.1.3 and 4.2.3] Since both modes may employ digital signatures, most of the content in these sections should be pulled out of section 4 and put in sections 3 or 5.  Any content specific to the mode (such as the FIPS requirement) should remain in section 4, of course.

[section 4.2.1] Does the name identifier in the response need to be encrypted?  Since the assertion itself is encrypted, this doesn't seem to be necessary, but you need to be explicit about this since [SAMLCore] does not specify what is to be done in this case.  (Note: This comment was included in a previous set of comments but is reworded here for clarity.)

[section 4.2.2] The IdP is allowed to reuse a previously established symmetric key only if the SP did not include a symmetric key in the request. As far as I can tell, this restriction is unwarranted.  If there is no symmetric key in the response, the SP won't know if the IdP used the symmetric key in the request or some other previously established symmetric key, so there is no point restricting the IdP's use of encryption.  From the perspective of this profile, the symmetric key in the request *is* a previously established symmetric key, and therefore the three encryption options for the IdP may be safely reduced to two.

[section 7] The reference to [SAMLMeta-Ext] will be out of date the moment this specification becomes an OASIS standard!

[general] Mimicking reference [RFC3280], replace the term "X.509v3 certificate" with "X.509 v3 certificate" throughout, or better yet, simply use the term "X.509 certificate" instead.  The latter term is intentionally vague so as to include X.509 public key certificates [RFC3280] (which includes X.509 v3 certificates), X.509 proxy certificates [RFC3820] (which are based on X.509 PKCs), and even X.509 attribute certificates [RFC3281].  Include a footnote to this effect early in the profile, perhaps in section 1.

[general] Security considerations are too tightly bound to the protocol bits.  The normative security language is either too lax (basic mode) or too strict (encrypted mode).  It would be better to relegate much of the security language to section 5.

[general] Encrypted mode is not properly motivated.  The use case in section 2 applies to basic mode, not encrypted mode.  It seems encrypted mode is geared more towards multi-hop web service frameworks, so perhaps such an example could be given (or at least mentioned) in section 2.

[general] Here is a list of metadata requirements for section "3.3 Use of Metadata":

Identity Provider:
- IdP metadata MUST include an <md:AttributeAuthorityDescriptor> element
- the <md:AttributeAuthorityDescriptor> element MUST include an <md:NameIDFormat> element with value "urn:oasis:names:tc:SAML:1.1:nameid-format:X509SubjectName"
- the <md:AttributeAuthorityDescriptor> element MUST include at least one <md:AttributeService> element with attribute xbasic:hasSupport="true"
- in any <md:AttributeService> element that supports basic mode, the value of Binding attribute MUST be "urn:oasis:names:tc:SAML:1.0:bindings:SOAP-binding"
- in any <md:AttributeService> element that supports basic mode, zero or more <saml:Attribute> elements MAY be included. Since a service provider may choose not to query the AA based on the attributes in this list, this list MUST be comprehensive.  Unless a method of dynamic metadata exchange exists, it is recommended that identity providers omit this list entirely.

Service Provider:
- SP metadata MUST include an <md:RoleDescriptor> element of type query:AttributeQueryDescriptorType
- the <md:RoleDescriptor> element MUST include an <md:NameIDFormat> element with value "urn:oasis:names:tc:SAML:1.1:nameid-format:X509SubjectName"

[general] Here are additional metadata requirements for section "4.3 Use of Metadata":

Identity Provider:
- the <md:AttributeAuthorityDescriptor> element MUST include at least one <md:AttributeService> element with attribute xencrypt:hasSupport="true"
- in any <md:AttributeService> element that supports encrypted mode, the value of Binding attribute MUST be "urn:oasis:names:tc:SAML:1.0:bindings:SOAP-binding"
- in any <md:AttributeService> element that supports encrypted mode, zero or more <saml:Attribute> elements MAY be included. Since a service provider may choose not to query the AA based on the attributes in this list, this list MUST be comprehensive.  Unless a method of dynamic metadata exchange exists, it is recommended that identity providers omit this list entirely.
- if the identity provider uses a previously established symmetric key, there SHOULD be at least one <md:KeyDescriptor> element with attribute use="encryption" in identity provider metadata.

Service Provider:
- if the service provider uses a previously established symmetric key, there SHOULD be at least one <md:KeyDescriptor> element with attribute use="encryption" in service provider metadata

----------------------
Tom Scavo
NCSA/University of Illinois