Re: [OAUTH-WG] Fwd: New Version Notification for draft-campbell-oauth-tls-client-auth-00.txt

2016-11-02 Thread Brian Campbell
On Sun, Oct 30, 2016 at 9:27 AM, Samuel Erdtman  wrote:

>
> I agree it is written so that the connection to the certificate is
> implicitly required but I think it would be better if it was explicit
> written since the lack of a connection would result in a potential security
> hole.
>

That's fair. I agree it can be made more explicit and that it be good to do
so.



> When it comes to the client_id I think subject common name or maybe
> subject serial numbers will be the common location, and I think an example
> would be valuable.
>
>

In my experience and the way we built support for mutual TLS OAuth client
auth the client_id value does not appear in the certificate anywhere. I'm
not saying it can't happen but don't think it's particularly common.

I can look at adding some examples, if there's some consensus that they'd
be useful and this document moves forward.



>
> I´m not saying it is a bad Idea just that I would prefer if it was not a
> MUST.
> With very limited addition of code it is just as easy to get the
> certificate attribute for client id as it is to get it from the HTTP
> request data (at least in java). I also think that with the requirement to
> match the incoming certificate in some way one has to read out the
> certificate that was used to establish the connection to do some kind of
> matching.
>
>
Getting data out of the certificate isn't a concern. I just believe that
the constancy of having the client id parameter is worth the potential
small amount duplicate data in some cases. It's just a -00 draft though and
if the WG wants to proceed with this document, we seek further input and
work towards some consensus.
___
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth


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

2016-11-02 Thread Samuel Erdtman
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.”
>>