Subject: [OAUTH-WG] draft-ietf-oauth-saml2-bearer-17

From: Hannes Tschofenig <[email-masked]>
Date: Sat, 02 Nov 2013 10:07:37 +0100

Hi Brian, Hi all,

I read through the SAML Bearer draft and here are a few comments:
(actually some of the comments that I made yesterday regarding 
draft-ietf-oauth-jwt-bearer-06)

Section 3:

Item #1: You wrote: "Issuer
         values SHOULD be compared using the Simple String Comparison
         method defined in Section 6.2.1 of RFC 3986 [RFC3986], unless
         otherwise specified by the application."

I would put a MUST here. You can also use the text I proposed for 
draft-ietf-oauth-jwt-bearer-06, which is text I copied from the TLS 
specification.

Also the comment regarding the comparison operation I made in 
draft-ietf-oauth-jwt-bearer-06 is applicable here.

Item #2: You wrote:

"
Section 2.5.1.4 of Assertions and Protocols for the OASIS
         Security Assertion Markup Language [OASIS.saml-core-2.0-os]
         defines the <AudienceRestriction> and <Audience> elements and,
         in addition to the URI references discussed there, the token
         endpoint URL of the authorization server MAY be used as a URI
         that identifies the authorization server as an intended
         audience.  Assertions that do not identify the Authorization
         Server as an intended audience MUST be rejected.
"

The 'MAY' is extremely weak here. If you make it a MUST that there has 
to be the endpoint URL of the authorization server in there then that 
would provide so much more interoperability. As a reader I wouldn't know 
what other options I have and systems that provision necessary databases 
/ tables to ensure that the comparison takes place will also struggle.

Then, there is again this SHOULD regarding the comparison operation, see
"
  Audience
         values SHOULD be compared using the Simple String Comparison
         method defined in Section 6.2.1 of RFC 3986 [RFC3986], unless
         otherwise specified by the application.
"

I would replace it with a MUST, as I argued in 
draft-ietf-oauth-jwt-bearer-06.

Item #3: As in the draft-ietf-oauth-jwt-bearer-06 this part is extremely 
fluffy, except for the case where it talks about the client-id. What 
exactly do I put into the field in the case of an authorization grant?

Item #5: You write:

"
The <Subject> element MUST contain at least one
         <SubjectConfirmation> element that allows the authorization
         server to confirm it as a Bearer Assertion.
"

What do you mean that the AS confirms that it is a bearer assertion? I 
think what you rather want to say is that the AS indicates that it is a 
bearer assertion.

Item #7+8: T I think you should combine the two items since they relate 
to the same issue, namely when to include the <AuthnStatement> element. 
There are two questions:

     Q1: Has the subject been authenticated?

     If 'no', then the <AuthnStatement> cannot be populated.

     If 'yes', then
         Q2: Has the subject requested to be anonymous?

         If 'no', then the <AuthnStatement> element is populated
         with the subject's identity.

         If 'yes', then the <AuthnStatement> MUST NOT be populated.
         (or populated with a field that indicates that the subject
          is anonymous; I don't know SAML enough to tell what the
          right approach here is).

Then you write:
"
The presenter SHOULD
         be identified in the <NameID> or similar element in the
         <SubjectConfirmation> element, or by other available means like
         SAML V2.0 Condition for Delegation Restriction
         [OASIS.saml-deleg-cs].
"

Who is the presenter? Is the presenter the subject?

Item #10: You write:

"
    10.  The Assertion MUST be digitally signed or have a keyed message
         digest applied by the issuer.  The authorization server MUST
         reject assertions with an invalid signature or keyed message
         digest.
"

To my knowledge SAML assertions only support digitial signatures and no 
keyed message digests.

Security Consideration section:

I believe the section needs to say two things into addition to the 
reference to the other specifications, which are already included in the 
security consideration section:

a) The specification does not mandate replay protection for the SAML 
assertion usage for neither the authorization grant nor for the client 
authentication. It is an optional feature.

b) There is actually no authentication happening when these SAML 
assertions are used for client authentication and for the authorization 
grant (in the classical definition of authentication). This may be 
surprising to some why typically assume that the client would have to 
demonstrate proof of possession of a secret, which isn't the case here.

It would have been possible to provide more enhanced funtionality (and 
SAML supports this as well) but it is not provided in the specification. 
Maybe a future specification will provide that functionalility.

I think it is worth pointing out.

Ciao
Hannes