Re: [OAUTH-WG] Benjamin Kaduk's Discuss on draft-ietf-oauth-device-flow-11: (with DISCUSS and COMMENT)

[William Denniss <[e-mail]>]

Hi Benjamin,

Thank you for your detailed review, and suggestions. Version 13 was just
posted, and incorporates your suggestions and feedback.

Replies inline:

On Thu, Aug 2, 2018 at 5:21 PM, Benjamin Kaduk <[e-mail]> wrote:

> On Wed, Aug 01, 2018 at 05:16:52PM -0700, William Denniss wrote:
> > Benjamin,
> >
> > Thank you for the feedback. We just posted version 12 which addresses
> many
> > of your feedback points. Replies inline.
> >
> > On Tue, Jul 24, 2018 at 6:31 AM, Benjamin Kaduk <[e-mail]> wrote:
> >
> > >
> > > ----------------------------------------------------------------------
> > > DISCUSS:
> > > ----------------------------------------------------------------------
> > >
> > > Let me preface this by noting that I'm not sure that all of these
> points
> > > are actionable; I would, however, like to discuss them.
> > >
> > > I'm really unhappy to not see any hard numbers on the entropy needed
> > > in a user code to provide a reasonable security margin with given
> > > parameters, and how it compares to the guessability bounds considered
> best
> > > practices in general (across protocols).  For example, we think 128-bit
> > > symmetric keys are okay because an attacker has to put in 2**96 work to
> > > have a 2**-32 chance of guessing correctly via brute force; the rate
> > > limiting and finite lifetime on the user code places an artificial
> limit on
> > > the amount of work an attacker can "do", so if one uses a 8-character
> > > base-20 user code (with roughly 34.5 bits of entropy), the
> rate-limiting
> > > interval and validity period would need to only allow 5 attempts in
> order
> > > to get the same 2**-32 probability of success by random guessing.
> > > Section 5.1 would be a great place for such text, near the preexisting:
> > >    The user code SHOULD have enough entropy that when combined with
> rate
> > >    limiting and other mitigations makes a brute-force attack
> infeasible.
> > >
> > >
> > Thank you for the comment, the authors are still considering the right
> way
> > to address this feedback.
> >
> >
> >
> > > We talk about "the authorization server", but any given *user* may
> have a
> > > relationship with multiple such ASes.  Can the Introduction make it
> more
> > > clear that the AS is associated with the device/client, and as such the
> > > it may not be the user's most-trusted AS?
> > >
> >
> > Sometimes the device is really an app on the device. E.g. a Roku TV
> device
> > (a "tv stick") that has several apps. Hulu, where the AS is hulu, and
> > YouTube, where the AS is YouTube (these are both "first party" use-cases
> > where the app and the AS are owned by the same entity). The user may also
> > have a Canon printer, which has a device flow to Google, to authorize it
> to
> > print documents (a "third-party" use-case).
> >
> > I'm not sure exactly what we should say in the introduction to address
> you
> > feedback here.
>
> The document text is currently referring to a single AS (the definite
> article "the" in "the authorization server" as if everyone knows which one
> to use as a prerequisite of using this flow.  That is presumably true for
> the devices in question, but it's not necessarily true for the reader of
> the spec.  So, I'd propose to say something like "connect to the device's
> authorization server to approve the access request" or "connect to the
> authorization server that the device trusts to mediate authorization
> decisions".  (Gosh, it's hard to write the second one without using
> "grant"!)
>

I added a section in the introduction to clarify the relationship of the
device to the AS:

   The device typically chooses the set of authorization servers to
   support (i.e., its own authorization server, or those by providers it
   has relationships with).  It is not uncommon for the device
   application to support only a single authorization server, such as
   with a TV application for a specific media provider that supports
   only that media provider's authorization server.  The user may not
   have an established relationship yet with that authorization

   provider, though one can potentially be set up during the
   authorization flow.


Does that improve the understanding of this concept? I can add the text
your propose directly too.


>
> > It also seems like a large latent risk with this flow is when the
> > > verification_uri_complete response is used along with an AS that
> assumes an
> > > authenticated user making such a verification request has approved the
> > > authorization (i.e., without an explicit user interaction to confirm),
> when
> > > that AS uses cookies or other persistent state to keep the user
> > > authenticated across multiple requests.  I could not find any
> MUST-level
> > > requirement for user interaction to confirm the device being authorized
> > > (even in Section 3.3, which covers the regular verificat_uri
> workflow!);
> > > please let me know if I missed something.  I would like to see some
> > > explicit text that (matching the flow described in Section 3.1 that
> > > requires the user to input the code) explicit user approval of the
> > > authorization is required.  (I do note that Section 5.3 has text about
> > > "SHOULD display information about the device.)
> > >
> >
> > When verification_uri is used, the user has 2 steps (at least): 1. enter
> > code, 2. consent to the request. verification_uri_complete skips the
> first
> > step, but the presumption is that they would still consent to the request
> > in the second step. We can make this more clear if it isn't currently.
>
> Please do; it's an important property of the flow.
>

The section on verification_uri_complete was updated to reflect this
feedback.


>
> > I'm also unhappy about the text in Section 1 that merely requires of the
> > > device the ability to "make outbound HTTPS requests", which leaves
> room for
> > > an awful lot of sins in certificate validation (and, potentially,
> > > ciphersuite selection).  Can we get a MUST-level requirement for
> > > authenticating the server and a cite to RFC 7525?
> > >
> >
> > Reference to 7525 has been added.
> >
> >
> > > ----------------------------------------------------------------------
> > > COMMENT:
> > > ----------------------------------------------------------------------
> > >
> > > Please use the RFC 8174 boilerplate instead of the RFC 2119 one.
> > >
> >
> > Done, thanks!
> >
> >
> > >
> > > Section 3.2
> > >
> > > The example expires in 30 minutes?  That seems longer than needed;
> wouldn't
> > > 5 minutes do?
> > >
> >
> > Incidentally, 30 minutes is the value Google returns on the requests,
> which
> > was where this example came from.
> >
> > In general I think it's good to have a reasonable window of time as the
> > user does need to find another device, open the browser, login to the AS
> > (possibly recover their passowrd), etc. There are some edge-cases in the
> > device flow too where if you enter the code right when it expires, you'll
> > get an error and need to start-over, and the longer the window of
> > opportunity the less this occurs.
>
> Sure.  I'm mostly just thinking in terms of the rate-limiting/entropy issue
> that's part of my DISCUSS -- cutting this window down lets a shorter code
> be used for a given security margin, but of course only as a linear factor.
>

We've added a longer discussion on entropy in version 13 (using part of the
text you wrote, thank you for that!)

There are a lot of factors that go into this decision by the AS when
choosing the timeout, rate limiting, and the entropy. Another example
mitigation is requiring users to be logged in, thus any brute-forcing
behavior could be monitored at an account level, providing an additional
account-based rate limit. We've left the sample at 30 minutes for now,
please let us know what you think.


> >
> > >
> > > Section 3.3
> > >
> > > I agree with directorate reviewer that the MUST NOT requirement for
> > > displaying the device_code should justify that requirement by
> discussing
> > > the consequences of exposure.
> > >
> >
> >
> > Text has been updated, now reads:
> >
> >    It is NOT RECOMMENDED for authorization servers to include the user
> >    code in the verification URI ("verification_uri"), as this increases
> >    the length and complexity of the URI that the user must type.  The
> >    next section documents user interaction with
> >    "verification_uri_complete", which is designed to carry this
> >    information.
>
> Thanks!
>
> -Benjamin
>
> >
> >
> >
> > >
> > > Section 3.5
> > >
> > >    authorization_pending
> > >       The authorization request is still pending as the end-user hasn't
> > >       yet completed the user interaction steps (Section 3.3).  The
> > >       client should repeat the Access Token Request to the token
> > >       endpoint.
> > >
> > > I feel like we want to mention the 'interval' here or some other
> discussion
> > > of an inter-request delay.
> > >
> >
> > This text has been reorganized, with some duplication removed and I think
> > it reads better now.
> >
> >
> > > Also, please clarify "reasonable default polling interval", per
> multiple
> > > directorate reviews.
> > >
> >
> > Done, we set it to 5s.
> >
> >
> > >
> > > Section 5.2
> > >
> > > Please clarify the entities involved in "the backchannel flow" that
> can be
> > > MITM'd.
> > >
> >
> > The authors are still considering text to address this feedback.
> >
> >
> > >
> > > Section 5.6
> > >
> > > The "short-range" part of a "short-range wireless signal" partially
> depends
> > > on how big the receiver's antenna is.  So perhaps we should be careful
> > > about indicating that this has more security value than it does.
> > >
> >
> > Dropped the reference to "short-range wireless signal".
> >
> >
> > >
> > > Section 6.1
> > >
> > > I'm not sure I understand the usage of "case-insensitive", here -- how
> > > would the user have an expectation of case-insensitivity?  Perhaps it
> is
> > > better to just say "majuscule" or "upper case" or whatever.
> > >
> > >
> > This section was reworded to hopefully address your feedback.
> >
> > Best,
> > William
>

Best,
William