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 wrote:
> Hi Samuel,
>
> Thanks for your review! I've replied inline:
>
> On Tue, Nov 1, 2016 at 9:41 AM, Samuel Erdtman 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.”
>>