Re: [OAUTH-WG] Adam Roach's No Objection on draft-ietf-oauth-native-apps-11: (with COMMENT)

[William Denniss <[e-mail]>]

Adam,

Thank you for reviewing the draft.

On Mon, May 22, 2017 at 1:27 PM, Adam Roach <[e-mail]> wrote:

> Adam Roach has entered the following ballot position for
> draft-ietf-oauth-native-apps-11: No Objection
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> General
> =======
> The thesis of this document seems to be that bad actors can access
> authentication information that gives them broader or more durable
> authorization than is intended; and appears to want to mitigate this
> predominantly with a single normative statement in a BCP telling
> potential bad actors to stop doing the one thing that enables their
> shenanigans.  For those familiar with the animated series "The Tick," it
> recalls the titular character yelling "Hey! You in the pumps! I say to
> you: stop being bad!" -- which, of course, is insufficient to achieve the
> desired effect.
>

I see that there is nevertheless "strong consensus" to publish the
> document;


Two years ago when I posted version 00 of the draft, nearly every native
app OAuth interaction in the wild violated that MUST. It was quite a large
problem, and we've been working in the OAuth community for a while to get
to this point, and I do believe there is a strong consensus to publish.

I see this BCP not only as a single MUST of what you shouldn't do, but also
as a valuable document detailing exactly how to meet that requirement,
which is non-trivial. Given how widespread the old practice was, I think
it's worth very clearly detailing why the old way is bad (e.g. Section
8.7), and giving as much information as possible for how to do it the right
way which is the goal of the document.

in which case, I would encourage somewhat more detail around
> what the rest of the ecosystem -- and the authentication server in
> particular -- can do to mitigate the ability of such bad actors.
> Specifically, section 8.1 has a rather hand-wavy suggestion that
> authorization endpoints "MAY take steps to detect and block authorization
> requests in embedded user agents," without offering up how this might be
> done. The problem is that that the naïve ways of doing this (UA strings?)
> are going to be easy to circumvent, and the more advanced ones (say,
> instructing users to log in using a non-OAuth flow if the auth endpoint
> detects absolutely no cookies associated with its origin) will have
> interactions that probably warrant discussion in this document. (For
> example, such an approach -- while potentially effective -- would
> interact very poorly with the "SSO mode" described in section B.3;
> although I think that recommending the use of "SSO mode" should be
> removed for other reasons, described below).
>

One of the problems with the old way of doing things is that it was hard to
tell the good actors from the bad – they all used the same techniques. So I
do actually see value in naive UA filtering as even though the bad actor
can lie, it becomes a provable lie.

Thanks for your feedback here, I'll take another look and see if we can
build in some of your suggestions.

________
>
> Specific comments follow
>
> The terminology section makes distinctions about cookie handling and
> content access in generic definitions (embedded versus external UAs, for
> example) but doesn't do the same for specific technologies. It is
> probably worthwhile noting that the "in-app browser tab" prevents apps
> from accessing cookies and content, while the "web-view" does not (I had
> to infer these facts from statements much later in the document).
>
> Section 7.3 gives examples of IPv4 and IPv6 addresses for loopback. While
> I'm sympathetic to the deployment challenges inherent in getting entire
> network paths to upgrade to IPv6, this text discusses loopback
> exclusively, which means that only the local operating system needs to
> support IPv6. Since all modern operating systems have supported IPv6 for
> well over a decade, I suggest that the use of IPv4 addresses for this
> purpose should be explicitly deprecated, so as to avoid unnecessary
> transition pain in the future. Minimally, the example needs to be
> replaced or supplemented with an IPv6 example, as per
> <https://www.iab.org/2016/11/07/iab-statement-on-ipv6/>: "We recommend
> that existing standards be reviewed to ensure they... use IPv6
> examples."


Good points. Version 11 added an IPv6 example, and a discussion around
IPv4/6 compatibility. I believe with that we are now conformance with the
IAB guidelines.

While I agree with your statement in principle that all hosts should
support already IPv6 loopback, in practice I was dealing with a case
recently where some code that was written assuming local IPv6 availability
broke when a user had explicitly disabled IPv6 on their machine. It seems
users can still get away without IPv6 for now, meaning IPv6 loopback
availability is not completely guaranteed.

Due to this, I think documenting an approach where clients can support IPv6
only, IPv4 only, and both being available is probably best for now (and 7.3
in v11 does that in the last paragraph).  It looks like that approach we
took in v11 does follow the IAB recommendations for mixed environments.


Section 8.1 makes the statement that "Loopback IP based redirect URIs may
> be susceptible to interception by other apps listening on the same
> loopback interface." That's not how TCP listener sockets work: for any
> given IP address, they guarantee single-process access to a port at any
> one time. (Exceptions would include processes with root access, but an
> attacking process with that level of access is going to be impossible to
> defend against). While mostly harmless, the statement appears to be false
> on its face, and should be removed or clarified.
>

Will be removed in the next update. Thank you.

Section 8.4 indicates that loopback redirect URIs are allowed to vary
> from their registered value in port number only. If you decide not to
> deprecate the use of IPv4 loopback, I imagine that servers should also
> treat [::1] identical to 127.0.01 for this purpose as well.
>

The expectation here was that the client would register both I guess. That
said, they'd be no harm in treating them as identical for that purpose, so
that's good advice. I'll revise.


> Section 8.7 claims that users are likely to be suspicious of a sign-in
> request when they should have already been signed in, and goes on to
> claim that they will distinguish between completely-logged-out states and
> logged-in-but-needing-reauth states, and may even take evasive action
> based on associated suspicion. Based on what I know of user research for
> security indicators, the chances of these statements being true for any
> non-trivial portion of any user population is basically zero. I propose
> that this section simply highlight that this is effectively an
> intractable problem from the client end, without any illusions that users
> have the ability to distinguish between the two circumstances, and that
> authentication servers must be extra vigilant in detecting and avoiding
> these kinds of attacks.
>

I'll revise this, I agree the claim is a little too aspirational.

Section 8.11, third paragraph talks about keystroke logging; in practice,
> the attack here is far easier than that, as I believe that applications
> that embed a web view can simply extract authentication-related material
> directly from the DOM.
>

Yes, they can do that too (and I agree it's a simpler attack
implementation). I'll include in the next revision. I'll keep in the
keystroke logging text, as it's another vector, and people tend to have a
visceral reaction when learning this.

Section B.2 uses the phrase "Android Implicit Intends" where I believe it
> means "Android Implicit Intents."
>

Fixed in the next update, thanks.


>
> Section B.3 describes the use of a "Web Authentication Broker" in SSO
> mode, which provides an isolated authentication context. If the section
> 8.7 text regarding user detection of nefarious application behavior in
> the form of web-view embedding is not removed, this needs a very clear
> treatment of how users might be expected to distinguish between that
> behavior and the SSO mode behavior. On casual examination, it seems that
> there would be no way to do so. I'll note that this BCP also promotes the
> "already logged in" behavior as being a key benefit to OAuth (cf. the
> third paragraph of Section 4), which the described behavior seems to
> mostly defeat. I would strongly suggest either removing discussion of
> using this mode, or deprecating it in favor of the user's preferred web
> browser, so as to obtain the advantages described in section 4.
>

My understanding of the Web Authentication Broker is that it is effectively
a special-case browser designed for authentication. There is a single
cookie-jar which is retained and used with all apps that use the Web
Authentication Broker in "SSO mode". So logins are shared, and the
advantages of Section 4 apply.  It's a separate cookie-jar from the main
browser, which would imply a minimum of two sign-ins on the device (so not
quite "single" at a device level), but I'm not sure if this is enough to
disqualify it.

My goal here is to simply document the current state of the art of the
platforms, and I felt that the Web Authentication Broker qualified as an
external user agent per the BCP.  The user interface is arguably quite nice
too, which mitigates some of the downsides of using a special
authentication "browser" with a separate cookie jar.

I'm open to suggestions on this section.


Thanks again for your comments.

Best,
William