Subject: [kitten] Barry Leiba's No Objection on draft-ietf-kitten-sasl-oauth-22: (with COMMENT)

From: "Barry Leiba" <[email-masked]>
Date: Tue, 26 May 2015 08:57:34 -0700

Barry Leiba has entered the following ballot position for
draft-ietf-kitten-sasl-oauth-22: 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 https://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:
https://datatracker.ietf.org/doc/draft-ietf-kitten-sasl-oauth/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Another excellent, informative shepherd writeup, which helped with my
review of the document.  (And, I'll say, most of the really useful
writeups, as this one, seem to use the "essay" format, rather than the
Q&A format.)  Thanks, Ben, for the writeup.

-- Section 1 --

   way to use the Simple Authentication and Security Layer (SASL)
   [RFC4422] to gain access to resources when using non-HTTP-based
   protocols, such as the Internet Message Access Protocol (IMAP)
   [RFC3501] and the Simple Mail Transfer Protocol (SMTP) [RFC5321],
   which is what this memo uses in the examples.

When I first read this, I thought the last phrase was bound to SMTP...
but then I see that both IMAP and SMTP are used in the examples.  So, a
small thing, but I'd end the sentence after the 5321 reference, and make
the last phrase into another sentence, like, "This document gives
examples of use in IMAP and SMTP."

-- Section 2 --

   Note that the IMAP SASL specification requires base64 encoding, see
   Section 4 of [RFC4648], not this memo.

Nit: That seems kind of an odd way to put it, in addition to its having a
comma splice.  Why not this?:

NEW
   Note that the IMAP SASL specification requires base64 encoding, as
   specified in Section 4 of [RFC4648].
END

-- Section 3.2 --
Nit: "vaialble" -> "available"

-- Section 3.2.2 --

      scope (OPTIONAL):  An OAuth scope which is valid to access the
         service.  This may be omitted which implies that unscoped
         tokens are required.  If a scope is specified then a single
         scope is preferred, use of a space separated list of scopes is
         NOT RECOMMENDED.

Why is it "NOT RECOMMENDED"?  What interoperability or security issue is
in play here that makes this "SHOULD NOT"?

         The server MAY return different URLs for users in
         different domains and the client SHOULD NOT cache a single
         returned value and assume it applies for all users/domains that
         the server suports.  The returned discovery document SHOULD
         have all data elements required by the OpenID Connect Discovery
         specification populated.

Similar questions for the SHOULD NOT and SHOULD here.  In this case, I
don't understand why they aren't "MUST NOT" and "MUST": for the first, I
don't understand how a client can interoperate with a server if it
violates this, and for the second, if they data elements are "required",
why isn't including them a "MUST"?

(Nit: The "if available" at the end of the paragraph is unnecessary; if
one is not available, it certainly can't be used.)

-- Section 4.1 --

   The
   underlying TLS establishment is not shown but is required for using
   Bearer Tokens per that specification.

   S: * OK IMAP4rev1 Server Ready
   C: t0 CAPABILITY
   S: * CAPABILITY IMAP4rev1 AUTH=OAUTHBEARER SASL-IR
   S: t0 OK Completed

The problem with this is that if the underlying TLS establishment is done
through STARTTLS on port 143, that would come between the first line ("S:
* OK IMAP4rev1 Server Ready") and the second.  And that makes the example
a bit odd.  The easiest way out of that is simply to remove the first
line.  Or, better, to replace the first line with something like
"[Initial connection and TLS establishment...]".  (This applies to the
subsequent sections as well.)

I wonder why you don't wrap the decoded payload here the same way you do
in Section 4.2 (or there the same as here).  I suggest that there'd be
less room for any confusion if you did them all (4.3 also) the same way
(I don't care which way you choose).

In the SMTP part, you say this:

   Note
   that line breaks are inserted for readability, and that the SMTP
   protocol terminates lines with CR and LF characters (ASCII values
   0x0D and 0x0A), these are not displayed explicitly in the example.

The same is true for the IMAP protocol and example, but you don't say
this there.  Actually, I don't think you need to say it in either place,
and suggest simply removing it here (and in Section 4.4).

-- Section 4.4 --
In the SMTP protocol, you should have "[Negotiate TLS...]" before the
SMTP AUTH command, as you do in Section 4.1.

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 26 May 2015 16:41:14 -0400 (EDT)

Hi Barry,

You're welcome :)

> examples of use in IMAP and SMTP."

"which is what this memo uses" is certainly wrong, since 'is' is singular
but should be plural to apply to both IMAP and SMTP.  But I think your
version is better, maybe with the addition of "both" before "IMAP and
SMTP" in the last sentence.

>    specified in Section 4 of [RFC4648].

I think I had noted the comma splice in a previous review but it slipped
through the cracks.  Your version looks fine.

> Nit: "vaialble" -> "available"

Thanks.

> in play here that makes this "SHOULD NOT"?

I am a little hazy on this, but I seem to recall that some deployed
software assumes that only a single scope will be used, so this is for
interoperability.  (Bill, please jump in if I'm confused.)

> why isn't including them a "MUST"?

This section was one of the repeated areas of contention over the last
calls, so I am somewhat reluctant to bring it back to the WG for
discussion.  I think there would be somewhat more support for SHOULD NOT
--> MUST NOT; the SHOULD for discovery is, IIRC, something of a
compromise, since some parties did not want to bring in a hard dependency
on OpenID Connect's choices.  The only reason I can come up for the SHOULD
NOT, off the top of my head, is if some vendor distributed a mail client
that was only ever going to talk to that same vendor's server.

> one is not available, it certainly can't be used.)

That's true, but I like the flow of the sentence with the extra
redundancy better than without it.

> subsequent sections as well.)

I would be fine with "[Initial connection and TLS establishment...]".

> (I don't care which way you choose).

The examples were added at different times, so keeping them in sync has
been a bit tough.  Hopefully we can enlist the help of the RFC Editor in
ensuring consistency of formatting.

> and suggest simply removing it here (and in Section 4.4).

Hmm, it is probably okay to not mention this, as you propose.  Anyone
implementing SMTP or IMAP will need to be conversant with the relevant
specifications...

> SMTP AUTH command, as you do in Section 4.1.

The running text does have "TLS negotiation is not shown but as noted
above it is required for the use of Bearer Tokens", but I support making
the examples consistent (and explicit about the use of TLS).

-Ben

---

From: Bill Mills <[email-masked]>
Date: Tue, 26 May 2015 20:49:04 +0000 (UTC)

> ----------------------------------------------------------------------> COMMENT:> ----------------------------------------------------------------------> > Another excellent, informative shepherd writeup, which helped with my review of the document.  (And, I'll say, most of the really useful writeups, as this one, seem to use the "essay" format, rather than the Q&A format.)  Thanks, Ben, for the writeup.> > -- Section 1 --> >    way to use the Simple Authentication and Security Layer (SASL)>    [RFC4422] to gain access to resources when using non-HTTP-based>    protocols, such as the Internet Message Access Protocol (IMAP)>    [RFC3501] and the Simple Mail Transfer Protocol (SMTP) [RFC5321],>    which is what this memo uses in the examples.> > When I first read this, I thought the last phrase was bound to SMTP...> but then I see that both IMAP and SMTP are used in the examples.  So, a small thing, but I'd end the sentence after the 5321 reference, and make the last phrase into another sentence, like, "This document gives examples of use in IMAP and SMTP."
WFM.  Done.
> > -- Section 2 --> >    Note that the IMAP SASL specification requires base64 encoding, see>    Section 4 of [RFC4648], not this memo.> > Nit: That seems kind of an odd way to put it, in addition to its having a comma splice.  Why not this?:> > NEW>    Note that the IMAP SASL specification requires base64 encoding, as>    specified in Section 4 of [RFC4648].> END
WFM. Done.
> > -- Section 3.2 --> Nit: "vaialble" -> "available"
Fixed
> > -- Section 3.2.2 --> >       scope (OPTIONAL):  An OAuth scope which is valid to access the>          service.  This may be omitted which implies that unscoped>          tokens are required.  If a scope is specified then a single>          scope is preferred, use of a space separated list of scopes is>          NOT RECOMMENDED.> > Why is it "NOT RECOMMENDED"?  What interoperability or security issue is in play here that makes this "SHOULD NOT"?
This isn't my preference for language, but some folks have concerns about usingmultiple scopes and how that all gets resolved and whether the client needs to understand scope strings to determine if they already have the scope they need.

> >          The server MAY return different URLs for users in>          different domains and the client SHOULD NOT cache a single>          returned value and assume it applies for all users/domains that>          the server suports.  The returned discovery document SHOULD>          have all data elements required by the OpenID Connect Discovery>          specification populated.> > Similar questions for the SHOULD NOT and SHOULD here.  In this case, I don't understand why they aren't "MUST NOT" and "MUST": for the first, I don't understand how a client can interoperate with a server if it violates this, and for the second, if they data elements are "required", why isn't including them a "MUST"?
There are implementations now that don't support Dynamic Client Reg. and requiring it would make no sense for them.
> > (Nit: The "if available" at the end of the paragraph is unnecessary; if one is not available, it certainly can't be used.)
We were leaving the door open for other discovery mechanisms so that this isn't MTI if there are other options.
> > -- Section 4.1 --> >    The>    underlying TLS establishment is not shown but is required for using>    Bearer Tokens per that specification.> >    S: * OK IMAP4rev1 Server Ready>    C: t0 CAPABILITY>    S: * CAPABILITY IMAP4rev1 AUTH=OAUTHBEARER SASL-IR>    S: t0 OK Completed> > The problem with this is that if the underlying TLS establishment is done through STARTTLS on port 143, that would come between the first line ("S:> * OK IMAP4rev1 Server Ready") and the second.  And that makes the example a bit odd.  The easiest way out of that is simply to remove the first line.  Or, better, to replace the first line with something like "[Initial connection and TLS establishment...]".  (This applies to the subsequent sections as well.)
WFM.
> > I wonder why you don't wrap the decoded payload here the same way you do in Section 4.2 (or there the same as here).  I suggest that there'd be less room for any confusion if you did them all (4.3 also) the same way (I don't care which way you choose).
Because the example for SMTP is wrong and I hadn't noticed that.  Fixing the port number and including the decoded text.
> > In the SMTP part, you say this:> >    Note>    that line breaks are inserted for readability, and that the SMTP>    protocol terminates lines with CR and LF characters (ASCII values>    0x0D and 0x0A), these are not displayed explicitly in the example.> > The same is true for the IMAP protocol and example, but you don't say this there.  Actually, I don't think you need to say it in either place, and suggest simply removing it here (and in Section 4.4).
Done
> > -- Section 4.4 --> In the SMTP protocol, you should have "[Negotiate TLS...]" before the SMTP AUTH command, as you do in Section 4.1.
Done> 
Changes committed to my working copy of the pending -23, which can be found at https://github.com/sweetums/idrafts/commit/98314659561cdfed59d39ac0e193f09f93a75da3
Thank you.
-bill

     On Tuesday, May 26, 2015 8:57 AM, Barry Leiba <[email-masked]> wrote:

-- Section 1 --

-- Section 2 --

NEW

-- Section 3.2 --

-- Section 3.2.2 --

-- Section 4.1 --

  The

  Note

-- Section 4.4 --

_______________________________________________
Kitten mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/kitten

---

From: Bill Mills <[email-masked]>
Date: Tue, 26 May 2015 20:50:22 +0000 (UTC)

Replied to Barry's just now, but hadn't seen this yet.  I think everything here is addressed in the other mail.
-bill 

     On Tuesday, May 26, 2015 1:41 PM, Benjamin Kaduk <[email-masked]> wrote:

 Hi Barry,

You're welcome :)

Thanks.

-Ben

---

From: Barry Leiba <[email-masked]>
Date: Thu, 28 May 2015 14:27:15 -0400

Bill and Ben, thanks for the reply.  Mixing my replies to the two of
you here, since it's mostly the same.  And I've eliminated anything
that there's complete agreement on.

> This isn't my preference for language, but some folks have concerns about
> using multiple scopes and how that all gets resolved and whether the client
> needs to understand scope strings to determine if they already have the
> scope they need.

> interoperability.  (Bill, please jump in if I'm confused.)

OK, both wrinkles are fine.  If you don't mind, please put a sentence
or so of explanation, then.  Maybe making it something like "is NOT
RECOMMENDED because multiple scopes are problematic for currently
deployed software," or whatever makes sense to say here.

>> don't understand how a client can interoperate with a server if it violates
>> this, and for the second, if they data elements are "required", why isn't
>> including them a "MUST"?
>
> There are implementations now that don't support Dynamic Client Reg. and
> requiring it would make no sense for them.

> that was only ever going to talk to that same vendor's server.

Given that it's been discussed and decided, you guys should do the
right thing, noting that my comment is non-blocking.  I'll just add my
further comment that I'm not a fan of using "SHOULD" (or its negative)
as a sort of compromise.  I very much prefer specifying the protocol
the way we think it should be specified, and accepting that not all
implementations will meet every aspect of the spec.

For Ben's explanation (at the end), for example, I think we should not
aim at those sorts of implementations, but at the normal,
open-Internet, multi-vendor scenario, which is what the protocol is
meant for.  And, of course, if a vendor wants to tailor their
implementation to a closed scenario, they're always free to do so.

But, again: resolve this however you think is right to make the
protocol right and to represent the consensus of the working group.
No need to further explain to me.

> redundancy better than without it.

Cool.  Let it remain as written.

Barry

---

From: Bill Mills <[email-masked]>
Date: Thu, 28 May 2015 18:47:33 +0000 (UTC)

OK.  Modified the scope language to say 
"If a scope is specified then a single scope is preferred. At the time  this document was written there were several implementations that do  not properly support space separated lists of scopes, so the use of  a space separated list of scopes is NOT RECOMMENDED." 
-bill

     On Thursday, May 28, 2015 11:27 AM, Barry Leiba <[email-masked]> wrote:

Barry

---

From: Barry Leiba <[email-masked]>
Date: Thu, 28 May 2015 15:16:50 -0400

> "If a scope is specified then a single scope is preferred. At the time
> this document was written there were several implementations that do
> not properly support space separated lists of scopes, so the use of
> a space separated list of scopes is NOT RECOMMENDED."

Thanks!

Barry