Re: [OAUTH-WG] SAML 2.0 Bearer Assertion Profile for OAuth 2.0 draft

[Torsten Lodderstedt <[e-mail]>]

Am 19.07.2010 06:34, schrieb Brian Campbell:
> Torsten,
> Thanks for taking the time to review and comment.  I've tried to
> address your questions inline below (though in some cases only raising
> more questions).
>
> On Sun, Jul 18, 2010 at 9:48 AM, Torsten Lodderstedt
> <[e-mail]>  wrote:
>    
>> Why do you prescribe to include the token endpoint URL into the
>> SubjectConfirmationData and similar data also in the AudienceRestriction? I
>> would expect such data in the AudienceRestriction only.
>>      
> One of the primary motivators behind much of this document was wanting
> to align the assertion format and processing rules as much as was
> reasonable with requirements in SAML Web SSO.
>    

Sounds like you defining a profile of the OAuth assertion flow for using 
SAML assertions complying to the SAML "Web Browser SSO Profile". I think 
you should state that somewhere. There will probably be other assertion 
flow profiles for other SAML assertion "dialects".

> The seemingly redundant data in Reciepent of SubjectConfirmationData
> and in the AudienceRestriction is very similar to what is required in
> Web SSO.  That's the main reason they are both there.
>
> However, there is a subtle distinction between the two -  the
> SubjectConfirmationData/Recipient identifies the endpoint to which the
> assertion can be delivered (where) and the Audience identifies the
> more general entity to whom it can be delivered (who).   In general
> practice the two are tightly coupled but they need not necessarily be.
>
>    
>> Why do you prohibit NotBefore-Attributes?
>>      
> Again trying to follow what was done in Web SSO - the prohibition of
> the NotBefore in SubjectConfirmationData was taken directly from
> section 4.1.4.2 of saml-profiles-2.0-os.  Honestly, I've never
> understood the reason for the restriction and really the reason I had
> it here was just to mimic web sso in the hope of facilitating better
> reuse of existing software.   Anyway, that was my reasoning but I
> woudn't be opposed to removing that language from this document.
>
> Regardless which way we go with that - note that the use of a
> NotBefore attribute is allowed in the Conditions element.
>    

ok, seems I mixed both attributes up. The conditions element is more 
important.

>    
>> What the reason for that statement? "If the assertion was issued with the
>> intention that the client act autonomously on behalf of the subject, an
>> <AuthnStatement>  SHOULD NOT be included." Do you expect the OAuth
>> authorization server to authenticate the client anyway?
>>      
> Client authentication to the authorization server is an orthogonal
> concern.   This is about the subject of the assertion (which may be
> the client but probably won't be in most cases) authenticating with
> the issuer of the assertion.
>
> It seems like there are cases where the end user (or subject) is
> present during the transaction and will have authenticated directly
> with the issuer of the assertion.  But there may also be cases where
> the end user is not present and the client is trusted to act on their
> behalf.  That bullet and the one before it that says, "If the
> assertion issuer authenticated the subject, the assertion SHOULD
> contain a single<AuthnStatement>  representing that authentication
> event" were intended to try and accommodate those two situations and
> provide a way to tell the difference based on the content of the
> assertion.
>
> It also just seemed like the "right" thing to do - if the issuer
> authenticated the subject, say so in the assertion.  If not, say
> nothing about it in the assertion.
>    

understood.

>    
>> Your I-D states:
>> "The authorization server MUST ensure that bearer assertions are
>>        not replayed, by maintaining the set of used ID values for the
>>        length of time for which the assertion would be considered valid
>>        based on the NotOnOrAfter attribute in the
>>        <SubjectConfirmationData>."
>>
>> Why do you want to force a one-time usage for SAML assertions?
>> This is to  restrictive, in my opinion.
>>      
> This is also a carryover from web SSO via the POST binding.  I was
> actually leaning towards not including this language but a couple
> early reviews were interesting in having it in there.  My co-author on
> this draft, Mr. Chuck Mortimore, was one of those folks so maybe he
> could provide more insight into his reasoning there.  Chuck?
>
>    
>> Any issuing authority could enforce such a
>> policy by using the "OneTimeUse" element.
>>      
> Personally, I've always found the spec language around OneTimeUse to
> be confusing and somewhat ambiguous.   Maybe I'm wrong but take a look
> at "2.5.1.5 Element<OneTimeUse>" and the treatment of OneTimeUse in
> "2.5.1 Element<Conditions>" of saml-core-2.0-os.  There are parts of
> that language that do suggest OneTimeUse is intended to indicate that
> some replay check is required but other parts seem to say something
> entirely different.   I don't believe that OneTimeUse is widely used
> or supported.
>    

We don't use it, too :-)

>    
>> 2.3.  Error Response: I would suggest to return a 401 status code for
>> invalid assertions since I see them as invalid credentials.
>>      
> That's a good point.  I had 400 largely as a copy/paste error from
> another example.  However, as I take a closer look at the text in
> draft -10 of the core OAuth spec, it would seem that it prescribes a
> 400 for this case.   Section "4.3. Error Response" says,
>
>    "If the client provided invalid credentials using an HTTP
>     authentication scheme via the "Authorization" request header field,
>     the authorization server MUST respond with the HTTP 401
>     (Unauthorized) status code.  Otherwise, the authorization server
>     SHALL respond with the HTTP 400 (Bad Request) status code."
>     -- http://tools.ietf.org/html/draft-ietf-oauth-v2-10#section-4.3
>
>
> Now I'm not sure which makes sense.  Is it really an HTTP
> Unauthorized?  It's not an HTTP authentication that is failing but
> rather a higher level protocol.   Maybe this is a question to be
> addressed at the core spec level?
>
>    

You are right, status code 401 is used in case of invalid client 
credentials. An invalid assertion is an error on a higher protocol level.

> I might just drop the example I have in section 2.3 of this document
> and defer completed to the core spec :)
>
> -Brian
>
> BTW, I referenced the SAML specs a few times in this email and should
> say that http://saml.xml.org/saml-specifications is a nice place to
> find all the OASIS SAML specs, including the latest errata, in one
> place.
>    

Thank you for this hint, I'm more (SAML Core) or less (Profiles) 
familiar with the specs.

regards,
Torsten.