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

Joseph Touch <touch@strayalpha.com> Sun, 20 December 2020 00:41 UTC

Return-Path: <touch@strayalpha.com>
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 ED5C03A003E; Sat, 19 Dec 2020 16:41:49 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.318
X-Spam-Level:
X-Spam-Status: No, score=-1.318 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_NEUTRAL=0.779, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=strayalpha.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 sDMDw0z5lIOS; Sat, 19 Dec 2020 16:41:48 -0800 (PST)
Received: from server217-4.web-hosting.com (server217-4.web-hosting.com [198.54.116.98]) (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 DC4053A0039; Sat, 19 Dec 2020 16:41:47 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=strayalpha.com; s=default; h=To:References:Message-Id:Cc:Date:In-Reply-To: From:Subject:Mime-Version:Content-Type:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=POX1ZH/3JDwayOb9ZmMC68KwnTIRKLlwFyOmS85hQho=; b=VkNfnpULlsJ58IFn8JjVOU/ic 6rh4ky9g+ckdQbJiAWHkNxs+i9EdkRkb4+QH3gvRSne0FLGmrEcfECc1TwhASt8R7cDYC5yic98F1 e12p9hHSM8cXeD9lpoNJRHrMOJbi7YqU8bpSeQOFV4YTLygDwNEDv6ePp0pvePWHUOwuWcyzEVgmC hWtcoHKfqp9fymrBQ3/gTi03tZ6mpOAOR1t930eXHrmNeNgT95K0eCcZ3CcQKO2ITUsjnfVB+3UgW xZnlYqzOeEMm4lZTlpUpSqoh8wkVTX2M8bqo9Cm0Kkz8WbrBE+5A7u3r+/4sDDXaNYXMN6z6+YUQf AMu/e4Vsw==;
Received: from cpe-172-250-225-198.socal.res.rr.com ([172.250.225.198]:53136 helo=[192.168.1.14]) by server217.web-hosting.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from <touch@strayalpha.com>) id 1kqmn8-000kUL-Fs; Sat, 19 Dec 2020 19:41:47 -0500
Content-Type: multipart/alternative; boundary="Apple-Mail=_420D82D4-04BA-469E-9AAB-076C7A0BC0E5"
Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.20.0.2.21\))
From: Joseph Touch <touch@strayalpha.com>
In-Reply-To: <1B7AE37A-1A16-47C5-95A3-FD131A1F0E20@chopps.org>
Date: Sat, 19 Dec 2020 16:41:41 -0800
Cc: ipsec@ietf.org, tsv-art <tsv-art@ietf.org>, draft-ietf-ipsecme-iptfs.all@ietf.org
Message-Id: <C35712E0-553A-4D1A-9254-E30FD2ACD7D0@strayalpha.com>
References: <160706317241.25013.15326204319913211090@ietfa.amsl.com> <559B100B-4BC2-4E95-AFF8-95BBAE0BDAC8@chopps.org> <724B2213-3B5E-4057-8343-4EE039034417@strayalpha.com> <1B7AE37A-1A16-47C5-95A3-FD131A1F0E20@chopps.org>
To: Christian Hopps <chopps@chopps.org>
X-Mailer: Apple Mail (2.3654.20.0.2.21)
X-OutGoing-Spam-Status: No, score=-0.5
X-AntiAbuse: This header was added to track abuse, please include it with any abuse report
X-AntiAbuse: Primary Hostname - server217.web-hosting.com
X-AntiAbuse: Original Domain - ietf.org
X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12]
X-AntiAbuse: Sender Address Domain - strayalpha.com
X-Get-Message-Sender-Via: server217.web-hosting.com: authenticated_id: touch@strayalpha.com
X-Authenticated-Sender: server217.web-hosting.com: touch@strayalpha.com
X-Source:
X-Source-Args:
X-Source-Dir:
X-From-Rewrite: unmodified, already matched
Archived-At: <https://mailarchive.ietf.org/arch/msg/tsv-art/LkLH3P8XyE34JNnOBqSCK36WhJA>
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: Sun, 20 Dec 2020 00:41:50 -0000


> On Dec 19, 2020, at 1:17 AM, Christian Hopps <chopps@chopps.org> wrote:
> 
> Changes are underway. One comments inline.
> 
>> On Dec 4, 2020, at 11:42 AM, Joseph Touch <touch@strayalpha.com <mailto: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.

Test vectors run only during testing. By putting this test in regular operation, you’re adding overhead to check that the code is working every time it runs.

It’s your call; I was just pointing out that this is very inefficient.

> Regarding complexity, having implemented this I can say there really is no complexity :)

If the LOC > 0, then it’s not “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.

Agreed, it’s simple - but it’s nonzero effort and code. Again, at run-time. For a check that basically keeps saying the code is working, rather than checking an actual packet error.

Again, recommending inefficient code is up to you.

Joe