Subject: Re: [OAUTH-WG] Review of oauth-mtls-07

From: Brian Campbell <[email-masked]>
Date: Mon, 9 Apr 2018 17:10:28 -0600

Thanks for the responses and additional suggestions. Also sorry for the
slow reply on my end - I was away on a nice long spring break with the
family, which was immediately followed by some other travel.

I totally get what you are saying about DN comparison and agree with the
sentiment. I've just struggled a bit with wording that would be both
appropriate for a spec and also helpful to implementers. Having looked at
it just a little more I see that RFC 2253 is obsoleted by RFC 4514. But RFC
4514 says that it "does not define a canonical string representation for
DNs" and points to RFC 4517 for comparison of DNs. So this change
https://github.com/ietf-oauth-mtls/i-d/commit/73e42346c90c3e
25ec9248fbc0e52bee76afe7bd, which just a non normative bit with an
informational reference, is what I came up with for a note about DN
comparison that is consistent with those RFCs. Suggestions on improvements
or alternatives are welcome as always.

Looking again at the current text about authentication method metadata
values and your proposed alternative text, what I have does seem a bit
superfluous. I'm leaning towards using your suggestion for 2.1.1 & 2.2.1.

I'll take another pass over Appendix A and consider incorporating some of
your text and/or the sentiment.

On Wed, Mar 28, 2018 at 3:57 PM, Justin Richer <[email-masked]> wrote:

> Thanks for the responses. I’ve cut out places where we seem to agree here
> and responded to the rest inline below.
>
>
>
>>
>> §2.1¶1: It would be helpful to have a pointer on methods of comparing
>> DNs. In our implementation we serialize them to strings using a canonical
>> format (RFC2253) and doing a string comparison based on that. There are
>> probably other ways, but it would be good to help developers avoid doing
>> something naive like comparing two different serializations as strings.
>>
>
> That's really an implementation detail but I can note that some kind of
> normalization is likely needed in comparing DNs.
>
>
> Might be worth pointing to to RFC4514 in a non-normative example here. The
> thing is, there are equivalent DNs that aren’t exact string copies of each
> other. We’ll want to avoid developers either doing a naive string
> comparison (leading to false negatives) or doing their own home-made
> regexes (leading to probable breakage and potentially security holes).
>
>
>
>
>> §2.1¶1: “configured or registered” is an unnecessary distinction, 6749
>> calls it “registered” regardless of how it got there
>>
>
> While I suppose that's true about 6749, I think colloquially 'registered'
> and 'configured' have come to have more meaning to some/many people about
> how the client came to be setup at the AS. So it might be strictly
> unnecessary but I'd prefer to keep the "configured or registered" just to
> help say that it doesn't matter how the AS came to get the expected DN for
> client.
>
>
> That’s a fair assessment, and I’m fine with it as-is in that case.
>
>
>
>
>> §2.1.1¶1: Is it necessary to introduce the registry here instead of just
>> pointing to it? I’m fine with stating that the values are used in both
>> discovery and client registration.
>>
>
> I had a hard time describing things concisely here because of the history
> of how and when the authentication methods registry came to be, it's name,
> and where it's used.  That text in ¶1 is what I was able to come up with
> that I thought adequately explained it. It's admittedly not the most
> elegant prose ever written but it does convey the info and I'm inclined to
> leave it. However, I would be happy to consider alternative text here, if
> you've got something specific to propose.
>
>
> I guess I just don’t think all that history is really needed right here.
> So I’d replace it with:
>
> For the PKI method of mutual TLS client authentication, this specification
>    defines and registers the following authentication method metadata
>    value into the "OAuth Token Endpoint Authentication Methods" registry
>
>    [IANA.OAuth.Parameters <https://tools.ietf.org/html/draft-ietf-oauth-mtls-07#ref-IANA.OAuth.Parameters>].
>
> If you feel it needs a reference, you can potentially put it in intro
> paragraph of the IANA section that sets the values, maybe? (§6.3)
>
> In the end I’m fine if the text stays — it’s not incorrect, I just feel
> it’s superfluous. Same comments apply to the other sections so I’m not
> going to copy them here.
>
>
>
> §A¶2: This paragraph reads a bit overly defensive. I understand the need
>> to position the two drafts in relationship to each other, but the tone here
>> could be adjusted significantly without losing the thrust of the main
>> argument.
>>
>
> The line about Token Binding not having a monopoly on the binding of
> tokens is admittedly a bit tongue-in-cheek and also a nod to the point you
> made the other day about running out of names.
>
> Honestly though, this text wasn't intended to be defensive and, even when
> I read it again, it doesn't come off that way to me. As usual, if you've
> got specific text to propose that you think would be better, I'd be happy
> to consider it. But I don't feel like the current text is particularly
> problematic or in need of change.
>
>
> I took a crack at rewriting the second paragraph (note that I removed the
> first sentence entirely), but in the end it’s up to you how you want to
> present the comparison between the documents:
>
>    Token Binding uses bare keys that are generated on the client, which avoids many of
>    the difficulties of creating, distributing, and managing the certificates used in this specification.
>
>    However, Token Binding requires support across different portions of the application
>
>    stack, including TLS and browser implementations. At the time of this writing,
>
>    there is relatively little support for it in available application
>    development platforms and tooling.  On the other hand, mutual TLS has been around for some time
>    and enjoys widespread support in web servers and development
>    platforms. As a consequence, Mutual TLS for OAuth 2.0 can be built and deployed now
>    using existing platforms and tools. In the future, the two specifications are likely to be
>
>    deployed in parallel for solving similar problems in different environments.
>
>
> — Justin
>
>
>

-- 
_CONFIDENTIALITY NOTICE: This email may contain confidential and privileged 
material for the sole use of the intended recipient(s). Any review, use, 
distribution or disclosure by others is strictly prohibited.  If you have 
received this communication in error, please notify the sender immediately 
by e-mail and delete the message and any file attachments from your 
computer. Thank you._

---

From: Justin Richer <[email-masked]>
Date: Tue, 10 Apr 2018 14:59:03 -0400

Welcome back and I hope the break was enjoyable!

The proposed diff below is great and just the kind of thing that I was after. It’s non-normative but it alerts a naive developer (like me) that doing string.equals() might not do what they intend. 

I’ll keep an eye out for the next revision and be sure to read it through.

Thank you for being so responsive, the document is good and provides important functionality that people are implementing today.

 — Justin

> On Apr 9, 2018, at 7:10 PM, Brian Campbell <[email-masked]> wrote:
> 
> Thanks for the responses and additional suggestions. Also sorry for the slow reply on my end - I was away on a nice long spring break with the family, which was immediately followed by some other travel. 
> 
> I totally get what you are saying about DN comparison and agree with the sentiment. I've just struggled a bit with wording that would be both appropriate for a spec and also helpful to implementers. Having looked at it just a little more I see that RFC 2253 is obsoleted by RFC 4514. But RFC 4514 says that it "does not define a canonical string representation for DNs" and points to RFC 4517 for comparison of DNs. So this change https://github.com/ietf-oauth-mtls/i-d/commit/73e42346c90c3e25ec9248fbc0e52bee76afe7bd <https://github.com/ietf-oauth-mtls/i-d/commit/73e42346c90c3e25ec9248fbc0e52bee76afe7bd>, which just a non normative bit with an informational reference, is what I came up with for a note about DN comparison that is consistent with those RFCs. Suggestions on improvements or alternatives are welcome as always. 
> 
> Looking again at the current text about authentication method metadata values and your proposed alternative text, what I have does seem a bit superfluous. I'm leaning towards using your suggestion for 2.1.1 & 2.2.1. 
> 
> I'll take another pass over Appendix A and consider incorporating some of your text and/or the sentiment. 
> 
> 
> 
>  
> 
> 
> On Wed, Mar 28, 2018 at 3:57 PM, Justin Richer <[email-masked] <mailto:[email-masked]>> wrote:
> Thanks for the responses. I’ve cut out places where we seem to agree here and responded to the rest inline below. 
> 
>>  
>> 
>> §2.1¶1: It would be helpful to have a pointer on methods of comparing DNs. In our implementation we serialize them to strings using a canonical format (RFC2253) and doing a string comparison based on that. There are probably other ways, but it would be good to help developers avoid doing something naive like comparing two different serializations as strings. 
>> 
>> That's really an implementation detail but I can note that some kind of normalization is likely needed in comparing DNs. 
> 
> Might be worth pointing to to RFC4514 in a non-normative example here. The thing is, there are equivalent DNs that aren’t exact string copies of each other. We’ll want to avoid developers either doing a naive string comparison (leading to false negatives) or doing their own home-made regexes (leading to probable breakage and potentially security holes).
> 
>> 
>>  
>> §2.1¶1: “configured or registered” is an unnecessary distinction, 6749 calls it “registered” regardless of how it got there
>> 
>> While I suppose that's true about 6749, I think colloquially 'registered' and 'configured' have come to have more meaning to some/many people about how the client came to be setup at the AS. So it might be strictly unnecessary but I'd prefer to keep the "configured or registered" just to help say that it doesn't matter how the AS came to get the expected DN for client.
> 
>> §2.1.1¶1: Is it necessary to introduce the registry here instead of just pointing to it? I’m fine with stating that the values are used in both discovery and client registration. 
>> 
>> I had a hard time describing things concisely here because of the history of how and when the authentication methods registry came to be, it's name, and where it's used.  That text in ¶1 is what I was able to come up with that I thought adequately explained it. It's admittedly not the most elegant prose ever written but it does convey the info and I'm inclined to leave it. However, I would be happy to consider alternative text here, if you've got something specific to propose.
> 
> I guess I just don’t think all that history is really needed right here. So I’d replace it with:
> 
> If you feel it needs a reference, you can potentially put it in intro paragraph of the IANA section that sets the values, maybe? (§6.3)
> 
> In the end I’m fine if the text stays — it’s not incorrect, I just feel it’s superfluous. Same comments apply to the other sections so I’m not going to copy them here.
> 
>>  
>> 
>> §A¶2: This paragraph reads a bit overly defensive. I understand the need to position the two drafts in relationship to each other, but the tone here could be adjusted significantly without losing the thrust of the main argument.
>> 
>> The line about Token Binding not having a monopoly on the binding of tokens is admittedly a bit tongue-in-cheek and also a nod to the point you made the other day about running out of names. 
>> 
>> Honestly though, this text wasn't intended to be defensive and, even when I read it again, it doesn't come off that way to me. As usual, if you've got specific text to propose that you think would be better, I'd be happy to consider it. But I don't feel like the current text is particularly problematic or in need of change. 
> 
> I took a crack at rewriting the second paragraph (note that I removed the first sentence entirely), but in the end it’s up to you how you want to present the comparison between the documents:
> 
> CONFIDENTIALITY NOTICE: This email may contain confidential and privileged material for the sole use of the intended recipient(s). Any review, use, distribution or disclosure by others is strictly prohibited.  If you have received this communication in error, please notify the sender immediately by e-mail and delete the message and any file attachments from your computer. Thank you.