Re: [OAUTH-WG] Pete Resnick's No Objection on draft-ietf-oauth-saml2-bearer-21: (with COMMENT)

2014-10-16 Thread Brian Campbell
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 presn...@qti.qualcomm.com
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
 

Re: [OAUTH-WG] Pete Resnick's No Objection on draft-ietf-oauth-saml2-bearer-21: (with COMMENT)

2014-10-16 Thread Pete Resnick

On 10/16/14 7:56 AM, Brian Campbell wrote:
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 
presn...@qti.qualcomm.com mailto:presn...@qti.qualcomm.com wrote:


2.1/2.2 - This paragraph shows why I don't like haphazard use of 2119.


Apologies for any haphazardness.


No worries. As you've discovered, throughout the organization we're 
pretty bad about consistent use of it, so if you use other RFCs for 
examples, you get weird results. I'm very happy to see that you're 
trying to get this right.



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.


Ah, good. Then some sort of MUST is appropriate.

Given that explanation of the intent, would you suggest an alternative 
wording of that sentence? Or is it okay as is?


I think it's OK as is, but would be better if you had the requirement on 
the right thing:


   The value of the assertion parameter contains a single SAML 2.0
   Assertion. It MUST NOT contain more than one SAML 2.0 assertion.

That makes it clear that you're not simply saying Put the SAML 2.0 
Assertion in here. You're saying, Don't try to squeeze in more than one.


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?


I agree with your gut: If implementations are going to bump into other 
implementations that fail to encode properly or double encode when 
encountering line wraps and padding, then you should say MUST NOT. You 
might even want to say, Due to some poor implementations, you MUST NOT 
use line wrapping or padding when you create these things, but you MUST 
decode them if you receive them.



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.?


Correct.


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.


Ah, so what you mean by the Assertion issuer 

[OAUTH-WG] Pete Resnick's No Objection on draft-ietf-oauth-saml2-bearer-21: (with COMMENT)

2014-10-14 Thread Pete Resnick
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.
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.


___
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth