Re: [netmod] Benjamin Kaduk's Discuss on draft-ietf-netmod-artwork-folding-09: (with DISCUSS and COMMENT)

Erik Auerswald <auerswal@unix-ag.uni-kl.de> Sun, 15 September 2019 11:31 UTC

Return-Path: <auerswal@unix-ag.uni-kl.de>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id CCF27120026; Sun, 15 Sep 2019 04:31:26 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.196
X-Spam-Level:
X-Spam-Status: No, score=-4.196 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, MAY_BE_FORGED=0.001, 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 lDmqME6mIzI5; Sun, 15 Sep 2019 04:31:24 -0700 (PDT)
Received: from mailgw1.uni-kl.de (mailgw1.uni-kl.de [IPv6:2001:638:208:120::220]) (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 6682E12000F; Sun, 15 Sep 2019 04:31:24 -0700 (PDT)
Received: from [172.20.10.2] (x2e720334.dyn.telefonica.de [46.114.3.52] (may be forged)) (authenticated bits=0) by mailgw1.uni-kl.de (8.14.4/8.14.4/Debian-8+deb8u2) with ESMTP id x8FBVAws156849 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 15 Sep 2019 13:31:20 +0200
To: Benjamin Kaduk <kaduk@mit.edu>, Kent Watsen <kent+ietf@watsen.net>
Cc: "netmod-chairs@ietf.org" <netmod-chairs@ietf.org>, draft-ietf-netmod-artwork-folding@ietf.org, The IESG <iesg@ietf.org>, "netmod@ietf.org" <netmod@ietf.org>
References: <156766366671.22774.7481795788724573201.idtracker@ietfa.amsl.com> <0100016d0372debf-16e6e132-b334-41b3-ad9c-953fd9314963-000000@email.amazonses.com> <20190911000337.GQ18198@kduck.mit.edu>
From: Erik Auerswald <auerswal@unix-ag.uni-kl.de>
Message-ID: <2df2e7e9-0580-ec1f-07a8-dbad7b72db70@unix-ag.uni-kl.de>
Date: Sun, 15 Sep 2019 13:31:09 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190911000337.GQ18198@kduck.mit.edu>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/FXzgWS3lPr2REhhCTKU5sqBjqUw>
Subject: Re: [netmod] Benjamin Kaduk's Discuss on draft-ietf-netmod-artwork-folding-09: (with DISCUSS and COMMENT)
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 15 Sep 2019 11:31:27 -0000

Hi Ben,

comments regarding just the security considerations below.

On 11.09.19 02:03, Benjamin Kaduk wrote:
> On Thu, Sep 05, 2019 at 10:02:03PM +0000, Kent Watsen wrote:
>>> On Sep 5, 2019, at 2:07 AM, Benjamin Kaduk via Datatracker <noreply@ietf.org> wrote:
>>> [...]
>>> Section 10
>>>
>>> We should warn against implementations scanning past the end of a
>>> buffer (containing the entire contents of a file) when checking what's
>>> in the beginning of the next line -- if a file ends with a backslash
>>> and "end of line" but no further content, we could perform an out of
>>> bounds access if the code assumes it is safe to check for the next
>>> line's initial content.

In a way that is like requiring any IS-IS draft to warn not to blindly
trust the received length value.

I do understand your concern.  All input needs to be handled safely.

For example, the "heartbleed" vulnerability was caused by blindly
trusting a length value received from the network.

But validating input and checking buffer boundaries are general
programming concepts and I do not think it useful to single out one (or
a few) and treat them specially in one I-D, but not others. This could
create the impression that checking buffer boundaries is important here,
but not in those other places.  And mentioning just the buffer
boundaries ignores the short read that is quite likely to occur at end
of file (the garbage left in the buffer after the last byte read must be
ignored as well, or content integrity is violated, which might change
the meaning of the I-D or add a vulnerability to the code inside the
I-D[1]).

>> Both Sections 7.2.2 and 8.2.2 describe conditions to determine when
>> unfolding occurs. AFIACT, in both cases, the unfolding algorithm stays
>> within the bounds of those conditions.
> 
> These procedures are fine if you're operating in a context where you
> interact with the text corpus via "get next line" operations.  But I don't
> think we have limited ourselves to such contexts; consider the case where I
> (foolishly) write text-processing code in C, and read(2) the text in
> question into a memory buffer.  I'm on my own for linebreak detection, and
> if I start peeking past escape characters, it's not so hard to imagine that
> I could fail to check for "end of buffer" and trigger undefined behavior.

This is true whenever something similar to C and read(2) is used to fill
a fixed size buffer with some bytes.  This can create many problems,
e.g., splitting UTF-8 characters, or reading less than the buffer size
(short read, not only possible on EOF).  All of those need to be handled
correctly.  This holds all the time, not just when implementing the
folding algorithms from this I-D.

Reading more than one byte at a time is an optimization only, it is not
needed for correct function.

This optimization is dangerous, because it adds significant complexity. 
Despite this it is used all the time without handling all the corner
cases, so again, I do understand your concerns.

>> For instance, given the input sequence [ '\' '\n' EOF] , the 7.2.2
>> algorithm would replace it with [ EOF ] and the 8.2.2 algorithm wouldn't
>> even attempt to unfold it since the condition of the next line containing
>> a second '\' character isn't met.
>>
>> Is this Security Consideration needed?
> 
> Well, it's a nonblocking comment.  So if the above description seems
> totally implausible to you, I can accept it not being included in the
> document.

As I see it, the gist of your comment is "implementing the algorithm
incorrectly might result in security vulnerabilities, depending on the
implementation details."  But is this a helpful security consideration?
I do not think so.

Thanks,
Erik

[1] Knowing the buffer size (e.g., after probing) could enable to create
     input data that creates an XML injection which could result in an
     attack on some tool to process I-Ds reachable via Internet.  This is
     kind of a contrived example, but not completely implausible.