Subject: [OAUTH-WG] Adam Roach's No Objection on draft-ietf-oauth-mtls-16: (with COMMENT)

From: Adam Roach via Datatracker <[email-masked]>
Date: Mon, 19 Aug 2019 19:02:39 -0700

Adam Roach has entered the following ballot position for
draft-ietf-oauth-mtls-16: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-oauth-mtls/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks for the work that everyone did on this document. I have one suggestion
for clarification, followed by a handful of editorial nits.

---------------------------------------------------------------------------

§2.1.2:

>  tls_client_auth_san_ip
>     A string representation of an IP address in either dotted decimal
>     notation (for IPv4) or colon-delimited hexadecimal (for IPv6, as
>     defined in [RFC4291] section 2.2) that is expected to be present
>     as an iPAddress SAN entry in the certificate, which the OAuth
>     client will use in mutual TLS authentication.

This probably needs some text that clarifies expectations around comparison
and/or normalization. For example, if the iPAddress value in the cert is
"20 01 0d b8 00 00 00 00 00 00 00 00 c0 00 02 ca" (and a mask of all F's), one
should presume that this would match both tls_client_auth_san_ip values
"2001:db8:0:0:0:0:c000:2ca" and "2001:DB8::192.0.2.202", right? If no, then
this document needs to talk about normalization of address representation.

---------------------------------------------------------------------------

§1:

>  Layering on the abstract flow above, this document standardizes
>  enhanced security options for OAuth 2.0 utilizing client certificate
>  based mutual TLS.

Nit: "client-certificate-based"

>  OAuth 2.0 defines a shared secret method of client authentication but

Nit: "shared-secret"

---------------------------------------------------------------------------
§1:

>  This document describes an additional
>  mechanism of client authentication utilizing mutual TLS certificate-
>  based authentication

Nit: "mutual-TLS"

>  Mutual TLS certificate-bound access tokens ensure that only the party

Nit: "Mutual-TLS"

>  Mutual TLS certificate-bound access tokens and mutual TLS client

Nit: "Mutual-TLS... mutual-TLS"

>  Additional client metadata parameters are introduced by this document
>  in support of certificate-bound access tokens and mutual TLS client
>  authentication.

Nit: "mutual-TLS"

The remainder of the document has several other uses of the phrase "mutual
TLS" as an adjective; they should be similarly hyphenated. I will not call
them out individually. (Non-adjectival uses should not contain hyphens, so
this isn't a simple find-and-replace operation.)

---------------------------------------------------------------------------

§5:

>  Authorization servers supporting both clients using mutual TLS and
>  conventional clients MAY chose to isolate the server side mutual TLS
>  behaviour to only clients intending to do mutual TLS, thus avoiding

Nit: "behavior" (or adjust other spellings in the document to be consistently
British).

---

From: Brian Campbell <[email-masked]>
Date: Wed, 21 Aug 2019 16:07:38 -0600

Thanks Adam, I attempt to provide not-too-terrible replies to your comments
inline below.

Can you help me with some text here? I'm a bit out of my realm, to be
honest. I naively just want it to say that the tls_client_auth_san_ip value
and iPAddress value in the cert need to be the same IP address. But I do
realize "same" isn't totally clear as your comment and others have pointed
out. But I don't know enough to know how to write it in a way that's
suitable for this kind of thing.

Alternatively, as I mentioned in my response to Ben (down a ways in
https://mailarchive.ietf.org/arch/msg/oauth/TMaNYXJgZ3-kz4GNsIL4O-9ncgQ),
I've never personally been able to think of a situation where
tls_client_auth_san_ip is actually needed.  So maybe removing it all
together is an option as well?

Mr. Kaduk beat you to this one. Will fix.

>  OAuth 2.0 defines a shared secret method of client authentication but

Will fix.

Understood. In theory anyway.  I'll take a pass thought the whole document
and endeavor to find and hyphenate all the places that "mutual TLS" is used
as an adjective.

 Ugh. I'm surprised my jingoistic spellcheck didn't flag that one. Will
fix.

-- 
_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: Brian Campbell <[email-masked]>
Date: Thu, 22 Aug 2019 16:48:53 -0600

Regarding the comment on tls_client_auth_san_ip, some other reviewers have
suggested using the using the IPv6 Address Text Representation described in
RFC 5952 rather than the one from in RFC 4291. Which I think makes sense to
do. Maybe also with a note that the comparison of the addresses should done
using binary as suggested in https://tools.ietf.org/html/rfc5952#section-8

What do you think?

On Wed, Aug 21, 2019 at 4:07 PM Brian Campbell <[email-masked]>
wrote:

> Thanks Adam, I attempt to provide not-too-terrible replies to your
> comments inline below.
>
> On Mon, Aug 19, 2019 at 8:02 PM Adam Roach via Datatracker <
> [email-masked]> wrote:
>

---

From: Adam Roach <[email-masked]>
Date: Thu, 22 Aug 2019 18:02:41 -0500

On 8/22/19 5:48 PM, Brian Campbell wrote:
> Regarding the comment on tls_client_auth_san_ip, some other reviewers 
> have suggested using the using the IPv6 Address Text Representation 
> described in RFC 5952 rather than the one from in RFC 4291. Which I 
> think makes sense to do. Maybe also with a note that the comparison of 
> the addresses should done using binary as suggested in 
> https://tools.ietf.org/html/rfc5952#section-8
>
> What do you think?

That sounds like a fine approach. Thanks for digging that reference up. :)

/a

---

From: Brian Campbell <[email-masked]>
Date: Fri, 23 Aug 2019 16:34:52 -0600

Thanks again Adam for the review, suggestions, and grammar lesson. I went
ahead and pushed a -17, which I think addresses everything discussed
herein.

https://tools.ietf.org/html/draft-ietf-oauth-mtls-17