Security Assertion Markup Language (SAML) 2.0 Profile for OAuth 2.0 Client Authentication and Authorization Grants
draft-ietf-oauth-saml2-bearer-23

I cleared my DISCUSS on the basis that RFC 6755 will be moved to an informative reference in response to this process issue: IDnits complains of a normative reference to Informational document RFC 6755, which was not noted in the Last Call announcement.


Editorial Nits:

S2.2: The paragraph before the actual example uses terminology inconsistent with RFC 6749:

 s/authorization code grant/authorization grant/

2.1/2.2 - This paragraph shows why I don't like haphazard use of 2119. The first "MUST be" is obviously silly and should simply be "is". But the second one buries what *might* be a proper and important use of MUST (you MUST NOT try to stick in two SAML Assertions) with a simple definitional one. (And that assumes that it's even plausible to try to use more than one SAML Assertion. If you simply can't, it's just s/MUST contain/contains.) The base64url encoding MUST is fine, because you don't want people sticking in raw XML, but the SHOULD NOTs for line wrapping and pad I am curious about: Isn't a parser going to have to check for line wrapping and pad anyway and undo it (because it's not a MUST NOT), and therefore this SHOULD NOT really isn't about interoperability so much as neatness (in which case they SHOULD NOTs are not appropriate)?

3 - Subpoint 2: Just for clarification, I like the non-passive sentence better: "The Authorization Server MUST reject any assertion that does not contain its own identity as the intended audience."

Subpoint 5:
OLD
        The <SubjectConfirmation> element MUST contain a
        <SubjectConfirmationData> element, unless the Assertion has a
        suitable NotOnOrAfter attribute on the <Conditions> element, in
        which case the <SubjectConfirmationData> element MAY be omitted.

That one's sure to get misquoted somewhere and confuse someone. Instead:
NEW
        If the Assertion does not have a suitable NonOnOrAfter attribute
        on the <Conditions> element, the <SubjectConfirmation> element
        MUST contain a <SubjectConfirmationData> element.

Subpoint 6:
OLD
        The authorization server MUST verify that the NotOnOrAfter
        instant has not passed, subject to allowable clock skew between
        systems.  An invalid NotOnOrAfter instant on the <Conditions>
        element invalidates the entire Assertion.  An invalid
        NotOnOrAfter instant on a <SubjectConfirmationData> element only
        invalidates the individual <SubjectConfirmation>.
NEW
         The authorization server MUST reject the entire Assertion if
         the NotOnOrAfter instant on the <Conditions> element has passed
         (subject to allowable clock skew between systems). The
         authorization server MUST reject the <SubjectConfirmation> (but
         MAY still use the rest of the Assertion) if the NotOnOrAfter
         instant on the <SubjectConfirmationData> has passed (subject to
         allowable clock skew).

Subpoint 7: Are you sure those SHOULDs and SHOULD NOTs are not conflicting? Can you not have an authenticated subject with an autonomously acting client?

Subpoint 9: As I asked in the -assertions document, is this really a requirement?

Subpoint 11: Again, it would be better to put the MUST on the action (e.g., "MUST reject") to make it clear who is doing what.

3.1/3.2 - s/MUST construct/constructs

4 - s/Though non-normative//

9 - Seems like OASIS.saml-deleg-cs and OASIS.saml-sec-consider-2.0-os are Normative, not Informative.

"keyed message digest" -> "MAC"

- intro para2: might be nice (no more) to add some refs to
other protocols that use SAML. 

- 2.2: What are "padding bits" in 4648? I don't recall such.
(But may be misremembering.)

- section 3, list item 2: This doesn't quite say that the
token endpoint URL MUST (in the absence of another profile) be
in an Audience element. Why not? The text seems to me to allow
for the AS to map the token endpoint URL to any value in an
Audience element that the AS finds ok. I suspect that might be
unwise, but it at least needs to be clear. Is that the text
being ambiguous or me being paranoid/wrong? Same point seems
to apply elsewhere too: 
   = in item 3.A where it says "typically identifies" but
   does not say how. 
   = in item 5 "or an acceptable alias"

- section 3, item 7: How might an AS know that "the Assertion
was issued with the intention that the client act autonomously
on behalf of the subject"?