Re: [secdir] secdir review of draft-ietf-oauth-device-flow

William Denniss <wdenniss@google.com> Thu, 02 August 2018 00:03 UTC

Return-Path: <wdenniss@google.com>
X-Original-To: secdir@ietfa.amsl.com
Delivered-To: secdir@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E8D3D130EA2 for <secdir@ietfa.amsl.com>; Wed, 1 Aug 2018 17:03:29 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -17.51
X-Spam-Level:
X-Spam-Status: No, score=-17.51 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, ENV_AND_HDR_SPF_MATCH=-0.5, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, T_DKIMWL_WL_MED=-0.01, USER_IN_DEF_DKIM_WL=-7.5, USER_IN_DEF_SPF_WL=-7.5] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=google.com
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 wopl5pmyXwls for <secdir@ietfa.amsl.com>; Wed, 1 Aug 2018 17:03:28 -0700 (PDT)
Received: from mail-ua0-x234.google.com (mail-ua0-x234.google.com [IPv6:2607:f8b0:400c:c08::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B655D130DE3 for <secdir@ietf.org>; Wed, 1 Aug 2018 17:03:25 -0700 (PDT)
Received: by mail-ua0-x234.google.com with SMTP id i4-v6so288863uak.0 for <secdir@ietf.org>; Wed, 01 Aug 2018 17:03:25 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=vZ2pZqhd4hkm5bYv7HFI2O7Y9QCZw9mhKjYMPBnLKIE=; b=GbCg2hrnWbyiD4o8EjQT/zb6rZII6YIi01faSB4AMu0/l+5usYphqvDOkQrF9xbPGl QbJ9nkZyNTvJ72GPZVejaw+Hfebb9ZtfruP0izCfjmjXdwDan3drgJoseIo1F6hf0poH ZQfavbl9LnQ/N6uNS5uix1zxtowP228FbYaTLFP1G5Sojd6uJmcqjxktxh6/WPorzn6n RruIoh0BXc6dkv8xAabsFchCL5+l0PjoQNUJ7z/zU/5YN9ujXqKI2MvXKiNWQ4IiW1rC kBXWz/yqGUXEQkCwF/zl9/sx//0xd0k5cumA9mEOJ1wD9uWiIX2sG/Imn+q36jEnNFXj Ngew==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=vZ2pZqhd4hkm5bYv7HFI2O7Y9QCZw9mhKjYMPBnLKIE=; b=mRwzlS+WYwStxN03lt5XaanGHfrBLeKWh0JINNhSvwHCBClsLA8IF8hj7fYhY0KS1d 07K9UEbk61bQDQSdl6EV3Z7iJiO+k8ZtypUCwikOSKSmjJpYIm721Kwgnz11uve+06nW PMb9e9fOJF/XQunXJYKQMM8ohuNNnLkzpjpQQkyd3OVKYxHWvsnax0+wotrdzaSO3a7D o/dTsvs7qlP32nbrrTwYNkscD2GhNS85f/HBnr2HDv69n6zOZDyjgk7g0SwMzJCFBfPH vSN6h97hqeLoqcMeZKgwtA8N7UDrwZZ1Gkbw27icf8PeUbcuF5STfqaGIy36orSGaXEq opEA==
X-Gm-Message-State: AOUpUlEm6SEnk7fi9UYnhprAFfxBf3c9R8f5cWOejnuIniYdUbWAcvmg e8fglKbt9RcOeFwpWrWR8+rmHHqxnK1mjLcbus0VIg==
X-Google-Smtp-Source: AAOMgpdthd4yc9PpooYHDZV9KaQbIIjqs49rw2W0fZGKMYquDKOtPrUuvVTSnyQVnbV9dxh/UvXyC4lFcgaMDOtEydA=
X-Received: by 2002:ab0:4987:: with SMTP id e7-v6mr380810uad.198.1533168204393; Wed, 01 Aug 2018 17:03:24 -0700 (PDT)
MIME-Version: 1.0
Received: by 2002:ab0:185a:0:0:0:0:0 with HTTP; Wed, 1 Aug 2018 17:03:03 -0700 (PDT)
In-Reply-To: <E17D37F9-070C-4E9D-9955-0E50F09DA89B@gmail.com>
References: <E17D37F9-070C-4E9D-9955-0E50F09DA89B@gmail.com>
From: William Denniss <wdenniss@google.com>
Date: Wed, 1 Aug 2018 17:03:03 -0700
Message-ID: <CAAP42hAqmOZjC1ANjdG=R8QSY1G7as4qR=utRWE-ZD=Yr_FNNQ@mail.gmail.com>
To: Christopher Wood <christopherwood07@gmail.com>
Cc: The IESG <iesg@ietf.org>, secdir@ietf.org, draft-ietf-oauth-device-flow.all@ietf.org
Content-Type: multipart/alternative; boundary="00000000000034915d0572688aa3"
Archived-At: <https://mailarchive.ietf.org/arch/msg/secdir/gOnllbMXJz7tRAzKqWN74O8adp4>
Subject: Re: [secdir] secdir review of draft-ietf-oauth-device-flow
X-BeenThere: secdir@ietf.org
X-Mailman-Version: 2.1.27
Precedence: list
List-Id: Security Area Directorate <secdir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/secdir>, <mailto:secdir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/secdir/>
List-Post: <mailto:secdir@ietf.org>
List-Help: <mailto:secdir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/secdir>, <mailto:secdir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 02 Aug 2018 00:03:30 -0000

Christopher,

Thank you for your valuable feedback. Version 12 incorporates much of your
feedback. Replies inline:

On Tue, Jun 12, 2018 at 5:55 PM, Christopher Wood <
christopherwood07@gmail.com> wrote:

> Hello,
>
> I have reviewed this document as part of the security directorate's
> ongoing effort to review all IETF documents being processed by the
> IESG.  These comments were written primarily for the benefit of the
> security area directors.  Document editors and WG chairs should treat
> these comments just like any other last call comments.
>
>   The summary of my review is: Ready with nits.
>
> Overall, the document is in fine shape. I have a few general comments (not
> quite nits,
> though not quite issues either), listed below:
>
> - Section 3.5, fifth paragraph: Requiring clients to poll at a
> “reasonable” polling interval
> without a suggestion of what is reasonable seems strange. Could you
> suggest a value that’s
> within reason, e.g., every second?
>

We documented a default of 5s in version 12.


> - Section 5.1, first paragraph: It might be useful to point to Section 6.1
> wherein User Code
> generation is discussed. Right now minimum entropy “requirements” are
> listed without further
> details regarding viable mechanisms.
>

The authors are still considering this feedback, along with Benjamin
Kaduk's DISCUSS.


> - Section 5.2, second paragraph: The text claims that an end user would
> end up “on the
> authorization page of the wrong service.” Can you provide more details
> here? What stops
> the malicious MITM from serving an authorization page that’s
> indistinguishable from the
> legitimate service page?



> - Section 5.3, first paragraph: How specifically does the authorization
> service prevent
> devices from lying when providing “information about the device”? Or,
> alternatively, how
> does the authorization service learn this information?
>

These 2 also pending.


> - Section 5.4: Would it be useful to suggest that clients SHOULD use a
> secure (encrypted
> and authenticated) channel when communicating to the user device?
>

This section is actually referring to real-world spying, i.e. someone in
the same room as you who can see the TV. Perhaps we need to make that more
clear?


>
> The remainder of my comments, listed below, are editorial in nature, aimed
> towards improving
> readability of the document.
>
> - Section 1, step (E): This is the first time client polling is mentioned
> without
> discussion of timeouts or server-generated errors. The draft provides such
> details later
> on, so it would be helpful to allude or point to them here.
>

I believe this is covered in detail in the document, this section is
intending to just be a high-level overview.


> - Section 3.3, second paragraph: Please cite TLS upon use (“… in a secure
> TLS-protected
> session.”).
>

Done, thanks!


> - Section 3.3, second paragraph: The text suggests that the server informs
> the user to
> “return to their device.” Perhaps this should be prefaced with a MAY, as
> the client will
> eventually learn that authorization is complete upon polling.
>

MAY was added, thanks!


> - Section 3.3.1, first paragraph: Should it be required that
> “verification_uri_complete”
> is constructed in part from the “verification_uri” and “user_code”? I’m
> not sure this is
> necessary, though the example given is constructed this way. If not
> required, this might be
> worth noting.
>

The working group considered this, in fact originally we were not going to
have verification_uri_complete and it would just be defined as a
composition of those two values. In the end, the work group decided to make
it separate. The authorization server can combine them to create the
complete verification URI, or may use something else.

The text currently states the following which I believe covers this:

"A verification URI that includes the "user_code" (or
other information with the same function as the "user_code"),
designed for non-textual transmission."

- Section 3.5, first paragraph: s/token endpoint/authentication server?
>

This section does actually relate to the token endpoint.


> - Section 5.1, third paragraph: This text is mostly redundant with the
> preceding paragraphs.
> I would remove or merge it with the paragraphs above.
>

I don't agree that it's redundant. Willing to review text if you have a
concrete proposal.


Please let me know if you’ve further questions, comments, or concerns. I
> hope this helps.
>
> Best,
> Chris
>


Best,
William