Thanks for your review and feedback on this one too, Pete. Replies are inline below...
On Tue, Oct 14, 2014 at 7:56 PM, Pete Resnick <[email protected]> wrote: > Pete Resnick has entered the following ballot position for > draft-ietf-oauth-saml2-bearer-21: No Objection > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to http://www.ietf.org/iesg/statement/discuss-criteria.html > for more information about IESG DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > http://datatracker.ietf.org/doc/draft-ietf-oauth-saml2-bearer/ > > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > 2.1/2.2 - This paragraph shows why I don't like haphazard use of 2119. > Apologies for any haphazardness. This particular document was born out of my first I-D and, while I was and still am a little green on this stuff, I've endeavored to use 2119 language appropriately but it's not always easy for mere mortals such as myself. The "MUST be" that you mention below, for example, always felt somewhat silly to me too but I was trying to follow from the examples in RFC 6749 -> see grant_type in http://tools.ietf.org/html/rfc6749#section-4.1.3 for one such example. > The first "MUST be" is obviously silly and should simply be "is". OK > 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.) It's intended to be both definitional and restrictive - that it contains an assertion but not more than one. The Response document in SAML Web SSO allows for multiple assertions and it has turned out to be quite a pain to deal with in practice while not adding any real value. While it's not entirely clear how one might try and stick more than one assertion in this parameter given the serialization, I still wanted to explicitly preclude it. Given that explanation of the intent, would you suggest an alternative wording of that sentence? Or is it okay as is? > 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)? > Yes, the base64 decoder still has to be prepared to deal with line wrapping and padding. In my experience most decoders are very capable of it. And stripping the white-space and "="s prior to decoding is easy enough for those using a decoder that can't. The SHOULD NOT is about neatness but also in a way about interop in that it's intended to help avoid common implementation problems that can arise with urlencoding the parameter value (either not encoding or double encoding, etc.). Base64url without line wraps and padding dosn't need urlencoding and doesn't change if urlencoding is applied. That was the thinking behind the SHOULD NOTs there. As I try and articulate the reasoning, it does feel like maybe it should have been a MUST NOT. I guess I was looking to channel Postel a bit in being somewhat liberal in what is accepted with padding/no padding and line wraps/no line wraps while using the SHOULD NOTs to suggest that clients be conservative in what they send. Thoughts about what to do with it, given that reasoning? > > 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." > Yes, on seeing it written that way, I also like it better. Just to make sure I'm on the same page. The sentence you suggest would replace the second to last sentence in #2 that currently says, "Assertions that do not identify [...] MUST be rejected."? > > 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. > OK > > 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). > OK > > 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? > Perhaps I've misused the words but, yes, that's the idea. An autonomously acting client is meant to describe a client that's acting without the user being present. The act of direct user authentication with the assertion issuer seems like the way to differentiate between the user being present for things and the client doing things "in the background" for the user. Both are valid use cases. Item 7 is looking to provide the AS with some clue as to which is happening. I'm open to suggestions to clarify the text but I do believe there's no conflict. > > Subpoint 9: As I asked in the -assertions document, is this really a > requirement? > Short answer, yes it is. Longer answer, see the answer in that thread. > > 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. > OK > > 3.1/3.2 - s/MUST construct/constructs > OK > > 4 - s/Though non-normative// > OK > > 9 - Seems like OASIS.saml-deleg-cs and OASIS.saml-sec-consider-2.0-os are > Normative, not Informative. > > OK. I wasn't sure but will change them to Normative unless someone yells otherwise about it.
_______________________________________________ OAuth mailing list [email protected] https://www.ietf.org/mailman/listinfo/oauth
