Re: [OAUTH-WG] Benjamin Kaduk's No Objection on draft-ietf-oauth-jwsreq-32: (with COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Thu, 08 April 2021 23:15 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: oauth@ietfa.amsl.com
Delivered-To: oauth@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id CA0B93A2104; Thu, 8 Apr 2021 16:15:12 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.197
X-Spam-Level:
X-Spam-Status: No, score=-4.197 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bLcgw3QG4Axb; Thu, 8 Apr 2021 16:15:08 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (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 22D3C3A2103; Thu, 8 Apr 2021 16:15:07 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 138NF0Ir027088 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 8 Apr 2021 19:15:05 -0400
Date: Thu, 8 Apr 2021 16:15:00 -0700
From: Benjamin Kaduk <kaduk@mit.edu>
To: Mike Jones <Michael.Jones@microsoft.com>
Cc: "iesg@ietf.org" <iesg@ietf.org>, "oauth@ietf.org" <oauth@ietf.org>, "oauth-chairs@ietf.org" <oauth-chairs@ietf.org>, "draft-ietf-oauth-jwsreq@ietf.org" <draft-ietf-oauth-jwsreq@ietf.org>
Message-ID: <20210408231500.GC79563@kduck.mit.edu>
References: <DM5PR00MB04216A0EE09F7F86805B1365F5749@DM5PR00MB0421.namprd00.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM5PR00MB04216A0EE09F7F86805B1365F5749@DM5PR00MB0421.namprd00.prod.outlook.com>
Archived-At: <https://mailarchive.ietf.org/arch/msg/oauth/JdZIExuaLU2hzsNYbwNoesK-cw8>
Subject: Re: [OAUTH-WG] Benjamin Kaduk's No Objection on draft-ietf-oauth-jwsreq-32: (with COMMENT)
X-BeenThere: oauth@ietf.org
X-Mailman-Version: 2.1.29
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: Thu, 08 Apr 2021 23:15:13 -0000

Hi Mike,

Also inline...

On Thu, Apr 08, 2021 at 04:45:15AM +0000, Mike Jones wrote:
> Thanks for your review, Ben.  We've published https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-33 to address your and other IESG comments.
> 
> Responses are inline below, prefixed by "Mike>".
> 
> -----Original Message-----
> From: OAuth <oauth-bounces@ietf.org> On Behalf Of Benjamin Kaduk via Datatracker
> Sent: Tuesday, April 6, 2021 11:39 AM
> To: The IESG <iesg@ietf.org>
> Cc: oauth@ietf.org; oauth-chairs@ietf.org; draft-ietf-oauth-jwsreq@ietf.org
> Subject: Benjamin Kaduk's No Objection on draft-ietf-oauth-jwsreq-32: (with COMMENT)
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-oauth-jwsreq-32: 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-jwsreq/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thank you for the new security considerations text in Sections 10.7 and
> 10.8 since I last reviewed; it does a great job capturing my comments (and the current deployed reality).
> 
> Thanks also to Watson Ladd for the latest secdir review and the authors for their responses to it.
> 
> Mike> Glad to hear it!
> 
> Section 6.2
> 
>    The Authorization Server MUST validate the signature of the JSON Web
>    Signature [RFC7515] signed Request Object.  The signature MUST be
>    validated using a key associated with the client and the algorithm
>    specified in the "alg" Header Parameter.  [...]
> 
> The last version I reviewed had some language tying the algorithm used for verification back to a registration (and I commented that perhaps a registration might not always exist).  This language seems to set the recipient up to blindly use the "alg" header parameter, even if it's "none", and we should probably have some hedging language here (I see we do have language elsewhere that bans "none" specifically)...
> 
>                                              If a "kid" Header Parameter
>    is present, the key identified MUST be the key used, and MUST be a
>    key associated with the client.  Algorithm verification MUST be
>    performed, as specified in Sections 3.1 and 3.2 of [RFC8725].
> 
> ... and maybe that would just take the form of swapping the order of these two sentences, now that I keep reading.
> 
> Mike> Order swapped, per your suggestion.
> 
> Section 10.2
> 
> Do any of the procedures listed for steps (c) and/or (d) need to be performed in step (e) as well?  (In particular, the requirements on the returned URI seem like they would still apply, and possibly the need for client authentication.
> 
> Mike> Having re-read (c), (d), and (e), I don't think so.  The returned URI in (e) can be an opaque identifier that is not a URL, so the URL checking logic wouldn't apply.

Okay.  Thanks for taking a second look.

> Section 10.3
> 
> Nat's response at
> https://mailarchive.ietf.org/arch/msg/oauth/hB_ON_BR0fDf3NSDFcFo5MwbZ2Y/ to my previous review suggested that there might be value in future work that specifies the linkage across these endpoints to try to address the cross-phase attacks discussed in [FETT].  However, the paragraph that I had commented on (that mentions "an extension specification") has since been removed, and I failed to track down why just from a quick mailarchive search.  There may well have been a good reason for removing it (and the reference to [FETT]), so please help refresh my memory if that's the case.
> 
> Mike> It was removed as suggested by Watson Ladd in the discussion that resulted from his SecDir review.

Ah!  Thanks for refreshing my memory.

> Section 10.4
> 
>    The introduction of "request_uri" introduces several attack
>    possibilities.  Consult the security considerations in Section 7 of
>    RFC3986 [RFC3986] for more information regarding risks associated
>    with URIs.
> 
> My previous comments had mentioned that because of time skew the dereferenced request URI might actually have the contents of a different request than the one intended, and Nat's response at https://mailarchive.ietf.org/arch/msg/oauth/hB_ON_BR0fDf3NSDFcFo5MwbZ2Y/
> pointed out that OIDC actually does use a nonce that would prevent such an occurrence.  Hopefully Nat did get a chance to think about whether there was anything else that we should mention in this document, on this topic.  ("There isn't anything else to mention here" is a fine answer; I just wanted to close the loop on that thread, since I didn't find one in the mail archive.)
> 
> Mike> OpenID Connect requests using some response_type values include a nonce and those using others don't.  Thinking about it some more, I don't think there's any particular risks about using these URLs that are sufficiently different than those of other URLs that they're worth mentioning here.

Okay, as before, thanks for thinking through it again.

-Ben

> Section 11.1
> 
> nit: s/TFP/trusted third-party service/ (multiple instances), since we stopped using "Trust Framework Provider" in the main body.
> 
> Mike> Thanks for the catch - corrected!
> 
> Sction 14.1
> 
> Not your fault, but BCP 195 now includes both RFC 7525 and RFC 8996 -- thank you for referencing it as BCP 195!  I expect the RFC Editor will catch the new reference if you don't want to figure out how to notate it properly.
> 
> Mike> Thanks for the heads-up.  I'll plan to ask the RFC Editor what the right way to do this is.
> 
> _______________________________________________
> OAuth mailing list
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth
> 
> 				Thanks again!
> 				-- Mike
>