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

Christopher Wood <christopherwood07@gmail.com> Wed, 06 March 2019 23:04 UTC

Return-Path: <christopherwood07@gmail.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 0B7E7130DC4; Wed, 6 Mar 2019 15:04:02 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.75
X-Spam-Level:
X-Spam-Status: No, score=-1.75 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.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 N_NZXbMH9P0U; Wed, 6 Mar 2019 15:04:00 -0800 (PST)
Received: from mail-yw1-xc32.google.com (mail-yw1-xc32.google.com [IPv6:2607:f8b0:4864:20::c32]) (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 CD78B124B0C; Wed, 6 Mar 2019 15:04:00 -0800 (PST)
Received: by mail-yw1-xc32.google.com with SMTP id r188so11652422ywb.12; Wed, 06 Mar 2019 15:04:00 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=2o8ZDDhhjWCM9hveehfUm4sVX9c2xNenVhDL3/RUAQU=; b=t2Cqn/ZM081EGzAWhTkzaYf43ejkRj/0C4adqZChFD0P8NkBEdPCHbsO0rNjiXY4m8 yzseWxlCXDN0tX6BckGYuO4LoB2x6kMqFr2Vf7LFaIvoAwxfzqGtYPseA2S2acU0xIl6 K4GQN5qgo0S+2YPywLwgqSGhw9Hk9WbwUb4hkpNu0wv2tM2+Hjfd8oSil34KVZxhYRJV MJEO0qwcbillyU4J3LewN/ganWPbA2+SpNeDOW35ZsUVaDAYapIyfKtGDUuWUHDUEK2S 2g8BA7b9AwiYfHZiM8VGwVAcJ1L4M1nvarIF0UC6FO3sra4OEABBvh8wLBR9Ta0+DW26 ERig==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=2o8ZDDhhjWCM9hveehfUm4sVX9c2xNenVhDL3/RUAQU=; b=crM54Rw818fKkUkN8/+f8UDweyOvzS9Olv97cLbF7bVhukVOYfrneiRt4sOWWf08Z+ Gvm7+Pr+45yJcD5KdbPQpPI+PM3hiyJKu/eRyLsbHQOx++XYzmIzBS0eYOaFfB1MROh+ LodFbvAb3y1Jnlzu7v0UXhnoSEEDZQdjhDC9dBdV5mozXZk85JHzfrVyw5Ig9zQmjeqy /YsKoRdL/BH9URkbaUCLF9OAxHqcHqKyAoZi5uzf91lNUy7I9dTBTH7CGzpmvRBK97dR PwLR74+eZthTuPXb5c3skW2D0KElHAzND5C0k8QTnUOAMthtxUn/Hiu8yOFls3FT2xLg Gbig==
X-Gm-Message-State: APjAAAWnUq0lxSBBTOANlqVumt//VoSwQ5nbAfPsAVeDyW8vBjfL9EF0 qxohjU/VJV08OB7vbyrfMRwExJMAxlM4y6/ZCdKMo/zr
X-Google-Smtp-Source: APXvYqweQRhmr3So+YJ/WI7Zunhrw1XZBgM1c9LdAseAQipkYnTMm7pVyoFxr8IFtmgNTClNkkoZzMWUorBG/G+/Ijo=
X-Received: by 2002:a0d:edc3:: with SMTP id w186mr7665413ywe.301.1551913439808; Wed, 06 Mar 2019 15:03:59 -0800 (PST)
MIME-Version: 1.0
References: <E17D37F9-070C-4E9D-9955-0E50F09DA89B@gmail.com> <CAAP42hAqmOZjC1ANjdG=R8QSY1G7as4qR=utRWE-ZD=Yr_FNNQ@mail.gmail.com> <CAO8oSXmY93HovXRETmhmcdkurRN9ovrtziQQDfVDNva=-fhT9Q@mail.gmail.com> <CAAP42hCMf3Zn5qU+K9Ee2KW+uxuPveSdxNEc=cO2z8iByQ3iSQ@mail.gmail.com>
In-Reply-To: <CAAP42hCMf3Zn5qU+K9Ee2KW+uxuPveSdxNEc=cO2z8iByQ3iSQ@mail.gmail.com>
From: Christopher Wood <christopherwood07@gmail.com>
Date: Wed, 6 Mar 2019 15:03:49 -0800
Message-ID: <CAO8oSXm+T5rCzm645j2STm5VTyshMj5=3NLebMdVptagT5A_Nw@mail.gmail.com>
To: William Denniss <wdenniss@google.com>
Cc: The IESG <iesg@ietf.org>, draft-ietf-oauth-device-flow.all@ietf.org, secdir <secdir@ietf.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://mailarchive.ietf.org/arch/msg/secdir/hixdKHglfbVg4R9BLTzoJkjK7JU>
Subject: Re: [secdir] secdir review of draft-ietf-oauth-device-flow
X-BeenThere: secdir@ietf.org
X-Mailman-Version: 2.1.29
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: Wed, 06 Mar 2019 23:04:02 -0000

On Tue, Jan 15, 2019 at 1:43 PM William Denniss <wdenniss@google.com> wrote:
>
>
>
> On Sun, Aug 26, 2018 at 7:30 PM Christopher Wood <christopherwood07@gmail.com> wrote:
>>
>> Hi William,
>>
>> Please see inline below.
>>
>> On Wed, Aug 1, 2018 at 5:03 PM William Denniss <wdenniss@google.com> wrote:
>>>
>>>
>>> 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?
>>
>>
>> That might help. It seems to be only a matter of clarity, not correctness.
>
>
> Thank you for the suggestion, a clarification has been added and will be published in -14.
>
> As currently drafted it now reads:
>
>             While the device is pending authorization, it may be possible for a
>             malicious user to physically spy on the device user interface
>             (by viewing the screen on which it's displayed, for example) and hijack the
>             session by completing the authorization faster than the user that
>             initiated it.
>
>>>
>>>
>>>>
>>>>
>>>> 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.
>>
>>
>> Ah, okay. I misunderstood. Thanks!
>>
>>>
>>>>
>>>> - 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.
>>
>>
>> My only recommendation is to remove the paragraph. I think it’s already covered by the preceding paragraphs. That said, I don’t think this is necessary. It’s merely a suggestion.
>>
>
> Note that this paragraph was significantly reworked in -13 and has a longer discussion on entropy now.
>
>>>
>>>
>>>
>>>> Please let me know if you’ve further questions, comments, or concerns. I hope this helps.
>>
>>
>> It does indeed. Thanks for your changes!
>>
>> Best,
>> Chris
>
>
> Thanks again for your review!

My pleasure, William!

Best,
Chris