Re: [OAUTH-WG] Review of draft-ietf-oauth-native-apps-05

[Samuel Erdtman <[e-mail]>]

see inline

Hannes could you have a look at the comment on Security Considerations.

On Tue, Nov 1, 2016 at 7:01 PM, William Denniss <[e-mail]> wrote:

> Hi Samuel,
>
> Thanks for your review! I've replied inline:
>
> On Tue, Nov 1, 2016 at 9:41 AM, Samuel Erdtman <[e-mail]> wrote:
>
>> Hi,
>>
>> Thanks for the great work of putting this together. I have a few comments
>> on the current draft. See below
>>
>> Best Regards
>> Samuel Erdtman
>>
>>
>>
>> 5.  Using Inter-app URI Communication for OAuth
>> The end of this section is a bit confusing with first a MUST statement
>> and then a RECOMMENDED statement
>> “Native apps MUST use an external user-agent to perform OAuth”
>> and
>> “This best practice focuses on the browser as the RECOMMENDED external
>>    user-agent for native apps.”
>>
>>
> The browser is one external user-agent. Using an external agent is a MUST
> to comply with this BCP, and the browser is the RECOMMENDED user agent.
>

My comment is not that something is formally wrong, just that I had to read
it twice before I got it. I´m fine with keeping as is, just wanted to
hilight the potential confusion.


>
>
>>
>> 7.1.1.  Custom URI Scheme Namespace Considerations
>> “For example, an app that controls the domain name "app.example.com"
>>    can use "com.example.app:/" as their custom scheme.”
>> drop the slash in the custom schema.
>>
>
> Done.
>
>
>> 7.2.  App-claimed HTTPS URI Redirection
>> “When the browser encounters a claimed URL, instead of the
>>    page being loaded in the browser, the native app is launched instead
>>    with the URL supplied as a launch parameter.”
>> drop one “instead” changing it to:
>> “When the browser encounters a claimed URL, instead of the
>>    page being loaded in the browser, the native app is launched
>>    with the URL supplied as a launch parameter.”
>>
>>
> Good catch, thanks.
>
> 7.2.  App-claimed HTTPS URI Redirection
>> If this is the recommended way to do it when possible maybe it should be
>> first?
>>
>
> It's ideal in a security sense, but less broadly supported currently than
> custom URI schemes. The order is roughly based on popularity.
>

makes sense


>
> 8.1.  Embedded User-Agents
>> “Embedded user-agents are an alternative method for authorization
>>    native apps.”
>> change to
>> “Embedded user-agents are an alternative method to authorize
>>    native apps.”
>> or
>> “Embedded user-agents are an alternative method for authorization
>>    of native apps.”
>>
>
> Fixed in 06.
>
>
>
>> 8.  Security Considerations
>> I see normative language here (MUST, RECOMMENDED, SHOULD, etc.), and it
>> felt a bit odd to me to have that under Security Considerations. Not sure
>> if there are guidelines around that, but to me it would make sense to keep
>> the normative parts out of Security Considerations. And it says
>> “Considerations”, to me that sounds mor like things to think about then
>> this is how it works.
>>
>
> I actually thought it was common, but I might be wrong. I'll wait and see
> if others weigh in on this too.
>

Sounds like a plan.


>
> 8.2.  Protecting the Authorization Code
>>
>> “A limitation of using custom URI schemes for redirect URIs is that
>>    multiple apps can typically register the same scheme, which makes it
>>    indeterminate as to which app will receive the Authorization Code.
>>    PKCE [RFC7636] details how this limitation can be used to execute a
>>    code interception attack (see Figure 1).  Loopback IP based redirect
>>    URIs may be susceptible to interception by other apps listening on
>>    the same loopback interface.”
>>
>> I think it would be preferable to separate custom URI and Loopback IP
>> based redirect.
>>
>
> Can add a paragraph break.
>

Thanks


>
> 8.2.  Protecting the Authorization Code
>> “Loopback IP based redirect
>>    URIs may be susceptible to interception by other apps listening on
>>    the same loopback interface.”
>> Are you referring to an application listening to loopback traffic or an
>> application killing the original application and start listening on the
>> same port. The second alternative would be relatively intrusive and notable.
>>
>
> The former. The assumption is that other desktop apps may be able to
> observe all local HTTP traffic on the loopback interface.
>

Ok


>
> Appendix A.  Server Support Checklist
>> 1.  Support custom URI-scheme redirect URIs.
>> I could not see that this was required in section Section 7.1.
>>
>
> It's there in section 7:
> "To fully support this best practice, authorization servers MUST support
>         the following three redirect URI options. Native apps MAY use
>         whichever redirect option suits their needs best, taking into
> account
>         platform specific implementation details."
>

My bad, I missed it.


>
>
>>
>> Appendix A.  Server Support Checklist
>> 5.  Support PKCE.
>> in Section 8.2 it says MUST
>> “Authorization servers MUST support PKCE [RFC7636]
>>    for public native app clients.”
>>
>
> "Recommended" is used in normal sentence case here, but I can see how that
> might be confusing, perhaps I should delete that word. Will think about
> this section.
>
> Incidentally, the idea for Appendix A was a non-normative checklist of
> items for authorization servers to collect the core requirements of the
> BCP, and give developers a quick way to evaluate authorization server
> compliance.
>

I like the idea of the checklist.


>
> 06 changes staged: https://github.com/WilliamDenniss/oauth-native-
> apps/pull/3
>
>