Re: [Gen-art] FW: Gen-ART LC review of draft-ietf-imapapnd-appendlimit-extension-07

Barry Leiba <barryleiba@computer.org> Mon, 28 December 2015 03:17 UTC

Return-Path: <barryleiba@gmail.com>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 477951A87CE; Sun, 27 Dec 2015 19:17:47 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 1.022
X-Spam-Level: *
X-Spam-Status: No, score=1.022 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FM_FORGED_GMAIL=0.622, FREEMAIL_FROM=0.001, MANGLED_LIST=2.3, SPF_PASS=-0.001] autolearn=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 Imk5NqiPcgCJ; Sun, 27 Dec 2015 19:17:45 -0800 (PST)
Received: from mail-vk0-x232.google.com (mail-vk0-x232.google.com [IPv6:2607:f8b0:400c:c05::232]) (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 2D6851A87CD; Sun, 27 Dec 2015 19:17:45 -0800 (PST)
Received: by mail-vk0-x232.google.com with SMTP id a123so15427000vkh.1; Sun, 27 Dec 2015 19:17:45 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=rOkYPMynOJicTMUXEgcj7GN5SMTsYDVMcieleUhAFkM=; b=bLEInZAmcG2L5YnGMtPuhR2cv+/92DUL2OToiep9q3dHdTzy4aLSYMp2RweOipuaFF ilvaH7Oa6zKpnEp8oPLpG/X2una5+HXdEIaf8444k1z4LkN2bN+K7h3tvs55swIE27Wk LzlMhUsnumTMGj16dTW+C0uIlMoB5Pa9oeihuGh9e2s0y47Tys5+j0zQUBqCrw6O1PaE gtN/AyDvxY2nX3+KipO9MpXd9pJdi1Pmd38mzZ0TBrJHAvQOaj5YhpyOqQvL9ycrHTQN Fr6FUKDZjB3qLmAmuZGuHmYQIToRDxacOfNuwbKDQAcHk4mo0gwA+vo69eGvcOJ2trV4 CVrA==
MIME-Version: 1.0
X-Received: by 10.31.15.4 with SMTP id 4mr30390556vkp.10.1451272664284; Sun, 27 Dec 2015 19:17:44 -0800 (PST)
Sender: barryleiba@gmail.com
Received: by 10.31.182.211 with HTTP; Sun, 27 Dec 2015 19:17:44 -0800 (PST)
In-Reply-To: <028001d140e9$565a4850$030ed8f0$@akayla.com>
References: <027701d140e4$c1337070$439a5150$@akayla.com> <028001d140e9$565a4850$030ed8f0$@akayla.com>
Date: Sun, 27 Dec 2015 22:17:44 -0500
X-Google-Sender-Auth: VjgIhLhIGc_end1Z6ad07uYm5I0
Message-ID: <CALaySJK1_TG7Gzgc4YrqLWfTm_uYQOK0LorcD5NFyL+RoJ0i7w@mail.gmail.com>
From: Barry Leiba <barryleiba@computer.org>
To: Peter Yee <peter@akayla.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <http://mailarchive.ietf.org/arch/msg/gen-art/7M0qDrqzqHkdJVpKG4OFARHb7Fk>
Cc: draft-ietf-imapapnd-appendlimit-extension.all@ietf.org, General Area Review Team <gen-art@ietf.org>, "imapext@ietf.org" <imapext@ietf.org>
Subject: Re: [Gen-art] FW: Gen-ART LC review of draft-ietf-imapapnd-appendlimit-extension-07
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 28 Dec 2015 03:17:47 -0000

Hi, Peter, and thanks for the review.  I'm adding the working group
mailing list here, so they have a record of this.  My comments below.

> I am the assigned Gen-ART reviewer for this draft.  The General Area Review
> Team (Gen-ART) reviews all IETF documents being processed by the IESG for
> the IETF Chair.  Please treat these comments just like any other last call
> comment.  For background on Gen-ART, please see the FAQ at
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
>
> Document: draft-ietf-imapapnd-appendlimit-extension-07
> Reviewer: Peter Yee
> Review Date: December 27, 2015
> IETF LC End Date: January 1, 2016
> IESG Telechat date: TBD
>
> Summary: This draft is basically ready for publication as a standards track
> RFC, but has nits that should be fixed before publication. [Ready with nits]
>
> The draft describes an extension to IMAP4v1 that allows a server to signal a
> maximum message upload size limit.
>
> Most of nits noted are linguistic, although there's a minor, repeated
> mistake in the ABNF that's critical to fix.
>
> Comments/Questions:
>
> Section 1, 2nd paragraph, 2nd sentence: the claim that this extension allows
> a server to avoid processing overly large messages (or attachments) is only
> true if a client implements and honors the extension.  A malicious client
> could still upload large messages and cause the server to process the
> message up to the point where it exceeds the server's limit.  While these
> overly large uploads would not be saved to disk, the server would still have
> to process them up to a point in order to determine that they should be
> discarded and a TOOBIG response returned.  Other mechanisms would be needed
> to fend off malicious clients that persist in such uploads.

Indeed; as with any IMAP extension, this helps only compliant
client-server combinations.  I think the Security Considerations
adequately covers the issue of a malicious client.

> Page 6, Section 6, 2nd full sentence: In light of the last paragraph of
> section 5 indicating that the number is a fixed maximum value, how would
> submitting a little too large message work?  Why is the server being lenient
> here?

The point is to warn against that sort of leniency.  One might think
that if you set a limit of, say, 2 MB, that you might act harshly to a
client that tries to post 6 MB, but not be so strict when a client
posts 2.1 MB.  The text warns that such leniency might be used to
enable an attack.

> Nits:

I noted in my AD review (posted to the imapext mailing list) that the
document is in need of a significant review for English corrections.
In discussion of that, we decided that the RFC Editor will have to
handle that: the authors have done a great job on this, but are not
native English writers, and we don't have natives at the ready to
help.

Authors, Peter has provided, below, a good list to help you with this.
If you can incorporate the changes he suggests, it'll help a lot, and
the RFC Editor can deal with any that remain.

One that I need to call out that's buried in Peter's list, the one he
mentions about ABNF (and this one is my fault, as I provided the
current version of ABNF and made the typo myself):

> Page 5, Section 5, ABNF: change "/=" to "=/" for the definitions of
> "capability", "status-att", and "status-att-val".

Yes.  I think I make that goof regularly.  Sigh.  Thanks for catching it.

Barry


> Page 1, Abstract, 1st sentence: change "mail" to "message".  Delete "of".
>
> Page 2, Section 1, 1st paragraph, 1st sentence: change "mail" to "message".
>
> Page 2, Section 1, 1st paragraph, 4th sentence: change "mail" to "message".
> Change "attachment" to "attachments".

Actually, make it "messages".

> Page 2, Section 1, 2nd paragraph, 1st sentence: insert "a" before "maximum".
> Insert "the" before "email".
>
> Page 2, Section 1, 2nd paragraph, 2nd sentence: change "server side" to
> "server-side".
>
> Page 3, Section 2, 1st paragraph, 1st sentence: insert "the" before the
> first "APPENDLIMIT".  Insert "the" before "authenticated".
>
> Page 3, Section 2, 1st paragraph, last sentence: insert "An" at the
> beginning of the sentence.
>
> Page 3, Section 2, 1st paragraph after (a), 1st sentence: delete "the"
> before "mailboxes".
>
> Page 3, Section 2, 1st paragraph after (a), 2nd sentence: insert "the"
> before "same".
>
> Page 3, Section 2, 3rd paragraph after (b), 1st sentence: insert "an" before
> "APPENDLIMIT".  Insert "a" before "STATUS".
>
> Page 3, Section 2, 3rd paragraph after (b), 2nd sentence: change "New" to "A
> new".  Change "mailbox specific" to "mailbox-specific".
>
> Page 3, Section 2, 3rd paragraph after (b), 3rd sentence: insert "to" before
> "section".  Insert "the" before "response".
>
> Page 3, Section 2, last paragraph, 1st sentence: insert "An" at the
> beginning of the sentence.  Delete "kind of".
>
> Page 3, Section 2, last paragraph, 2nd sentence: insert "a" before "client".
> Insert "the" before "advertised".
>
> Page 3, Section 3, heading: change "Mailbox specific" to "Mailbox-specific".
>
> Page 3, Section 3, 1st paragraph: insert "the" before "CAPABILITY".
>
> Page 4, Section 3.1, 1st paragraph, 1st sentence: insert "a" before
> "STATUS".
>
> Page 4, Section 3.1, 1st paragraph, 2nd sentence: insert "An" before "IMAP".
> Insert "a" before "STATUS".  Insert "an" before "APPENDLIMIT".  Change
> "mailbox specific" to "mailbox-specific".
>
> Page 4, Section 3.1, 1st paragraph, 3rd sentence: delete the comma.
>
> Page 4, Section 3.2, 1st paragraph, 2nd sentence: delete the comma.

Actually, this can go wither way, and because of the lengths of the
clauses here, I think the comma should stay.  We can let the RFC
Editor weigh in on it with their final edits.

> Page 5, Section 4, 1st paragraph, 1st sentence: insert "a" before "client".
> Change "mail" to "message".  Change "to" to "for" before "that".  Insert
> "the" before "server".
>
> Page 5, Section 4, 1st paragraph, 2nd sentence: insert "to" before
> "[RFC4469]".  Change "(4) to "4".
>
> Page 5, Section 4, 2nd paragraph, 1st sentence: change "Client" to "A
> client".  Insert "the" before "maximum".
>
> Page 5, Section 4, 2nd paragraph, 2nd sentence: insert "to" before
> "section".
>
> Page 5, Section 5, ABNF: change "/=" to "=/" for the definitions of
> "capability", "status-att", and "status-att-val".
>
> Page 6, Section 8: append a comma after "Long".
>