Re: [Tsv-art] [IPsec] Tsvart early review of draft-ietf-ipsecme-iptfs-03

Christian Hopps <chopps@chopps.org> Sat, 19 December 2020 09:17 UTC

Return-Path: <chopps@chopps.org>
X-Original-To: tsv-art@ietfa.amsl.com
Delivered-To: tsv-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 519ED3A0FAE; Sat, 19 Dec 2020 01:17:53 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.896
X-Spam-Level:
X-Spam-Status: No, score=-1.896 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, 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 SzwdYK9zJM3i; Sat, 19 Dec 2020 01:17:50 -0800 (PST)
Received: from smtp.chopps.org (smtp.chopps.org [54.88.81.56]) by ietfa.amsl.com (Postfix) with ESMTP id 8DE3C3A0FB0; Sat, 19 Dec 2020 01:17:50 -0800 (PST)
Received: from [192.168.2.5] (047-050-069-038.biz.spectrum.com [47.50.69.38]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by smtp.chopps.org (Postfix) with ESMTPSA id 5BCEB6092B; Sat, 19 Dec 2020 09:17:49 +0000 (UTC)
From: Christian Hopps <chopps@chopps.org>
Message-Id: <1B7AE37A-1A16-47C5-95A3-FD131A1F0E20@chopps.org>
Content-Type: multipart/signed; boundary="Apple-Mail=_97F9B0BA-FC19-48F0-8383-4A607082D918"; protocol="application/pgp-signature"; micalg="pgp-sha512"
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Date: Sat, 19 Dec 2020 04:17:48 -0500
In-Reply-To: <724B2213-3B5E-4057-8343-4EE039034417@strayalpha.com>
Cc: Christian Hopps <chopps@chopps.org>, ipsec@ietf.org, tsv-art <tsv-art@ietf.org>, draft-ietf-ipsecme-iptfs.all@ietf.org
To: Joseph Touch <touch@strayalpha.com>
References: <160706317241.25013.15326204319913211090@ietfa.amsl.com> <559B100B-4BC2-4E95-AFF8-95BBAE0BDAC8@chopps.org> <724B2213-3B5E-4057-8343-4EE039034417@strayalpha.com>
X-Mailer: Apple Mail (2.3608.120.23.2.4)
Archived-At: <https://mailarchive.ietf.org/arch/msg/tsv-art/kXRb77v_y1eY-asJtV5scYW8_BA>
Subject: Re: [Tsv-art] [IPsec] Tsvart early review of draft-ietf-ipsecme-iptfs-03
X-BeenThere: tsv-art@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Transport Area Review Team <tsv-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tsv-art>, <mailto:tsv-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tsv-art/>
List-Post: <mailto:tsv-art@ietf.org>
List-Help: <mailto:tsv-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tsv-art>, <mailto:tsv-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 19 Dec 2020 09:17:53 -0000

Changes are underway. One comments inline.

> On Dec 4, 2020, at 11:42 AM, Joseph Touch <touch@strayalpha.com> wrote:
> 
> Hi, Christian,

[...]

>>> There is no clear utility in having the blockoffset point past the end of the
>>> current packet. It serves – at best – as only a partially useful check on the
>>> next packet. I.e., if the two blockoffsets disagree, presumably a packet is
>>> lost – but if they agree, it cannot be concluded that a packet is not lost. It
>>> is sufficient that it points to the end of the tunnel packet.
>> 
>> No harm either though, right? Having implemented this, I can tell you that it does help detect bugs in ones code while in development. :)
> 
> It’s useful as a check, but it’s important to explain what the check means.
> 
> There are four cases (BO = blockoffsets, seq = ESP sequence number)
> 
>                               ESP - no gap         ESP - gap
> ---------------------------------------------------------------------
> BO - aligned          OK                         BO-FP-err
> BO - misaligned    SN-FP-err              Typ-err
> 
> Typ-err is the typical error when a packet is lost, because that should result in both an ESP seqno gap and blockoffset misalignment.
> 
> SN-FP-err is when there’s no ESP seq no gap but the blockoffsets are misaligned. This should only ever happen if the seqno rolls around and you missed it, which you should have some other mechanism to prevent (i.e., drop old packets when they’re half the sequence space old - which is easy to predict because you know the tunnel packet generation rate).
> 
> BO-FP-err is when there’s an ESP seq no gap but the blockoffsets are aligned. This is just a false positive (thus “FP” in my notation). S
> 
> In any of the error cases, you do not reassemble.
> 
> So checking blockoffset alignment only helps you know whether your sequence number check and timeout discard is working correctly.
> 
> Protocol implementation correctness should be checked by test vectors, not during normal code operation IMO. At best, it’s unnecessary complexity. At worst, it gives you a false sense of wanting to reassemble when you shouldn’t.

Conversely, test-vectors are harder to write w/o this and must be external, whereas this is a simple sanity check that can be added inline (perhaps conditionally) to code which has been shown to catch bugs. This will help implementations be more robust.

Regarding complexity, having implemented this I can say there really is no complexity :) You set the block offset to the remaining inner packet length to be sent (or 0 if no packet fragmentation is in progress). This value is sitting right there in the code so you can either plug it in or use some marker value (you suggest the length to the end of the payload). On receive the code is identical: does it exceed the length of the payload or not.

Thanks,
Chris.