[OAUTH-WG] Review of oauth-mtls-07

Justin Richer <jricher@mit.edu> Tue, 20 March 2018 17:53 UTC

Return-Path: <jricher@mit.edu>
X-Original-To: oauth@ietfa.amsl.com
Delivered-To: oauth@ietfa.amsl.com
Received: from localhost (localhost []) by ietfa.amsl.com (Postfix) with ESMTP id C7025126CB6 for <oauth@ietfa.amsl.com>; Tue, 20 Mar 2018 10:53:07 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.21
X-Spam-Status: No, score=-4.21 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([]) by localhost (ietfa.amsl.com []) (amavisd-new, port 10024) with ESMTP id O4P1-RcraOtt for <oauth@ietfa.amsl.com>; Tue, 20 Mar 2018 10:53:05 -0700 (PDT)
Received: from dmz-mailsec-scanner-5.mit.edu (dmz-mailsec-scanner-5.mit.edu []) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 99E04126BF7 for <oauth@ietf.org>; Tue, 20 Mar 2018 10:53:04 -0700 (PDT)
X-AuditID: 12074422-80bff700000057b1-4a-5ab14a7dbe7a
Received: from mailhub-auth-2.mit.edu ( []) (using TLS with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP id 96.8B.22449.E7A41BA5; Tue, 20 Mar 2018 13:53:02 -0400 (EDT)
Received: from outgoing.mit.edu (OUTGOING-AUTH-1.MIT.EDU []) by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id w2KHqv1A025436 for <oauth@ietf.org>; Tue, 20 Mar 2018 13:52:59 -0400
Received: from [] (89-197-166-66.virtual1.co.uk [] (may be forged)) (authenticated bits=0) (User authenticated as jricher@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id w2KHqsgF012409 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT) for <oauth@ietf.org>; Tue, 20 Mar 2018 13:52:56 -0400
From: Justin Richer <jricher@mit.edu>
Content-Type: multipart/alternative; boundary="Apple-Mail=_DAA6A88B-9E40-4DFB-B3D4-B839133FFDD7"
Mime-Version: 1.0 (Mac OS X Mail 11.2 \(3445.5.20\))
Message-Id: <86368D0D-EB6D-4803-8AC3-C587405BAA32@mit.edu>
Date: Tue, 20 Mar 2018 17:52:54 +0000
To: "<oauth@ietf.org>" <oauth@ietf.org>
X-Mailer: Apple Mail (2.3445.5.20)
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrMIsWRmVeSWpSXmKPExsUixG6nolvntTHK4P1SU4uTb1+xOTB6LFny kymAMYrLJiU1J7MstUjfLoEr49nbj4wFv7Iq5hzZwtbAeCeqi5GDQ0LARKL5qm4XIxeHkMBi JokN3y6xQDjHGCWu7//DDuG8Z5K4fHsuUxcjJwebgKrE9DUtYDazQJLElfY17CA2L9Ck928f gsWFBRQkpnatYgPZwCtgJfFiEg9ImAWo9fXyxYwgtoiAusSa8z/ByiUElCSmf7/NNoGRZxaS qbOQTIWIa0ssW/iaGcLWlNjfvZwFU1xDovPbRNYFjGyrGGVTcqt0cxMzc4pTk3WLkxPz8lKL dE31cjNL9FJTSjcxgkPPRWkH48R/XocYBTgYlXh4J0hsjBJiTSwrrsw9xCjJwaQkyhuoCBTi S8pPqcxILM6ILyrNSS0+xCjBwawkwpupAJTjTUmsrEotyodJSXOwKInzephoRwkJpCeWpGan phakFsFkZTg4lCR4Ez2BGgWLUtNTK9Iyc0oQ0kwcnCDDeYCG54PU8BYXJOYWZ6ZD5E8x2nN8 6nnQxsyx5dFLIHkATN548bqNWYglLz8vVUqcVw2kTQCkLaM0D24yKK1EHl3m9IpRHOhRYd55 IFU8wJQEN/sV0FomoLXZMzeArC1JREhJNTDy3VcIFf3hobw3rtWsVvB6i3GpS1Uui/06ec+j 0wREFsj8ZQl8vsXZnf/DR8vtWwVecHuVhfexb3x8wnOdVZvfi+8Oi3hP+dw6IfiJz2mlkVAu A2vesf5zil9+LogOdvPMlF0nvuGkoPHG9TeeCVnu+2ejsWveSeN9U6zKF+0/GP9ogmqH0WIl luKMREMt5qLiRACulsveBgMAAA==
Archived-At: <https://mailarchive.ietf.org/arch/msg/oauth/EuXHApEqYz1A6-UJqIvu88QnJVc>
Subject: [OAUTH-WG] Review of oauth-mtls-07
X-BeenThere: oauth@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: OAUTH WG <oauth.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/oauth>, <mailto:oauth-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/oauth/>
List-Post: <mailto:oauth@ietf.org>
List-Help: <mailto:oauth-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/oauth>, <mailto:oauth-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 20 Mar 2018 17:53:08 -0000

As promised in yesterday’s meeting, here’s my review of the oauth-mtls draft. We’ve recently implemented the spec from the AS and RS side for an as-yet-unreleased version of the Authlete service, and overall it’s in really good shape and very implementable as it stands today. Great work, and usable right now!

Comments, nits, and suggestions as follows:

§Abstract: Single sentence is a bit of a run-on that’s hard to follow. Suggested rewrite:

This document describes OAuth client authentication and sender-constrained tokens using Transport Layer Security (TLS) mutual authentication with X.509 certificates. OAuth clients are provided a mechanism for authentication to the authorization sever using mutual TLS, based on either single certificates or public key infrastructure (PKI). OAuth authorization servers are provided a mechanism for binding access tokens to a client’s mutual TLS certificate, and OAuth protected resources are provided a method for ensuring that such an access token presented to it was issued to the client presenting the token.

§1¶1 (and throughout): The document goes back and forth between “mutual TLS authentication” and “TLS mutual authentication”, one should be picked and used consistently throughout. I realize this is spelled out in 1.2 but it might be worth the effort to use one form most of the time.
§1¶3: maybe don’t call it a “basic bearer token” and instead just a “bearer token” to avoid sounding judgmental	
§2¶1: suggest turning parenthetical into a list: “(regardless of whether the client was dynamically registered, statically configured, or otherwise established)”
§2¶3: It seems this paragraph is trying to leave the door open to other MTLS bound client auth methods, but such methods would require the definition of a different auth method parameter value and a new spec, not really an extension of what’s here. Therefore, suggest changing the end of the paragraph into a single compact sentence:

 The authorization server MUST enforce the
   binding a certificate to a specific client as described in either Section 2.1 <https://tools.ietf.org/html/draft-ietf-oauth-mtls-07#section-2.1> or
   Section 2.2 <https://tools.ietf.org/html/draft-ietf-oauth-mtls-07#section-2.2> 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. 
§2.1¶1: “configured or registered” is an unnecessary distinction, 6749 calls it “registered” regardless of how it got there
§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. 
§2.1.2: I’m only just now seeing the reference to RFC4514 here so this reference needs to be in the parent section as well. I was previously under the impression that no format was prescribed. 
§2.2¶1: Might want to say explicitly in here that the cert is in the JWK for the client (instead of lower down), as it would make the description of the JWKS_URI method make more sense upfront. This could also live in the parent section.
§2.2¶1: "certificate chain is not validated” should probably more explicitly point to the *client’s* certificate not being validated to prevent clients from not validating the *server’s* certificate chain.
§2.2¶1: Extraneous comma: "successfully authenticated, if the subject”
§2.2.1: Same comment as §2.1.1
§3.1¶2: As Brian mentioned in another message, this should specify “no padding”.
§4.1¶1: Probably intend “set up” instead of “setup”
§4.1¶4: “separate host name” should be “separate host name or port”
§4.2¶1: Wording is a bit awkward, suggest:

Since the resource server relies on the authorization server to perform client authentication, there is no need for the resource server to validate
   the trust chain of the client's certificate in any of the methods
   defined in this document.  
§4.3¶1: I get what this section is trying to say but it is confusingly laid out. Might be better to say something like “MTLS client auth and sender-constrained MTLS bound tokens can be used independently of each other”. 
§4.3¶1: This advice doesn’t just apply to public clients, so we probably don’t mean “would not authenticate the client” here but instead “would not authenticate the client using mutual TLS”, since the client could authenticate in other methods. Though it is important to point out that public clients can do this :too:, it’s just as important to allow a client to use private_key_jwt or client_secret_basic and still get a constrained token.
§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.