Re: [OAUTH-WG] WGLC review of OAuth 2.0 Security Best Current Practice by Mike Jones

Benjamin Kaduk <kaduk@mit.edu> Sat, 23 November 2019 06:46 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 15A4112002E for <oauth@ietfa.amsl.com>; Fri, 22 Nov 2019 22:46:22 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.2
X-Spam-Level:
X-Spam-Status: No, score=-4.2 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=unavailable 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 3vjjnGJoKyF5 for <oauth@ietfa.amsl.com>; Fri, 22 Nov 2019 22:46:21 -0800 (PST)
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 0BC62120833 for <oauth@ietf.org>; Fri, 22 Nov 2019 22:46:20 -0800 (PST)
Received: from 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 xAN6kCgq032130 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 23 Nov 2019 01:46:15 -0500
Date: Fri, 22 Nov 2019 22:46:12 -0800
From: Benjamin Kaduk <kaduk@mit.edu>
To: Mike Jones <Michael.Jones=40microsoft.com@dmarc.ietf.org>
Cc: "oauth@ietf.org" <oauth@ietf.org>, "ve7jtb@gmail.com" <ve7jtb@gmail.com>, "'Andrey Labunets'" <isciurus@fb.com>
Message-ID: <20191123064612.GI32847@mit.edu>
References: <BN8PR00MB056369C04536770FF5B90512F54F0@BN8PR00MB0563.namprd00.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <BN8PR00MB056369C04536770FF5B90512F54F0@BN8PR00MB0563.namprd00.prod.outlook.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/oauth/aG17hNZi2oDO0eYuqpMptGpIKDs>
Subject: Re: [OAUTH-WG] WGLC review of OAuth 2.0 Security Best Current Practice by Mike Jones
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: Sat, 23 Nov 2019 06:46:22 -0000

On Wed, Nov 20, 2019 at 03:40:34AM +0000, Mike Jones wrote:
> I did a complete read of draft-ietf-oauth-security-topics-13<https://tools.ietf.org/html/draft-ietf-oauth-security-topics-13>.  My review comments follow, divided into substantive and editorial sections.
> 
> SUBSTANTIVE
> 
[...]
> 
> 4.8.1.1. Metadata - This section suggests the use of a "resource_servers" metadata value.  This isn't defined by RFC 8414 nor do I see it the IANA OAuth Authorization Server Metadata registry at https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#authorization-server-metadata.  Is this something that's been standardized elsewhere?  If so, please add a citation.  If not, please clearly say that this metadata value is not standardized, and is therefore unlikely to be interoperable.

I would go further and say that we should not document "best practices"
that involve non-standardized values.

> 4.8.1.1. Metadata - This section suggests the use a "access_token_resource_server" token response value.  Please likewise clearly say that this parameter isn't a standard.

(ditto)

[...]
> 4.8.1.3. Audience Restricted Access Tokens - Delete "basically".
> 
> Many locations - The draft often uses the word "which" when you mean "that"..  For instance, in 4.9.2, please change "which could" to "that could" and in 4.11 change "which are" to "that are".  There's lots of places to look up the difference in meaning, but a rule of thumb that's usually right is that if you don't have a comma before the "which", it should probably be "that".

The RFC Editor uses a "restrictive" vs. "non-restrictive" split for "that"
and "which" -- see https://www.rfc-editor.org/styleguide/tips/ .

-Ben (no hats)