Re: [Tsv-art] Tsvart early review of draft-irtf-pearg-numeric-ids-generation-02

Fernando Gont <fgont@si6networks.com> Tue, 08 September 2020 14:48 UTC

Return-Path: <fgont@si6networks.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 321D23A0D64; Tue, 8 Sep 2020 07:48:44 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.845
X-Spam-Level:
X-Spam-Status: No, score=-2.845 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, NICE_REPLY_A=-0.948, SPF_HELO_NONE=0.001, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=unavailable 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 sJ8SjQ5GfU8i; Tue, 8 Sep 2020 07:48:41 -0700 (PDT)
Received: from fgont.go6lab.si (fgont.go6lab.si [91.239.96.14]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 375133A0D69; Tue, 8 Sep 2020 07:48:40 -0700 (PDT)
Received: from [10.0.0.134] (unknown [186.19.8.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by fgont.go6lab.si (Postfix) with ESMTPSA id 0B4972839E5; Tue, 8 Sep 2020 14:48:36 +0000 (UTC)
From: Fernando Gont <fgont@si6networks.com>
To: Michael Tüxen <tuexen@fh-muenster.de>, tsv-art@ietf.org
Cc: pearg@irtf.org, draft-irtf-pearg-numeric-ids-generation.all@ietf.org
References: <159680292803.8931.4890868238678597521@ietfa.amsl.com> <44fcae41-96d8-d0e4-5b8e-cd4419a516a4@si6networks.com>
Message-ID: <fd0c67e0-72cb-61d6-ea63-6bf7ca3e6b6f@si6networks.com>
Date: Tue, 08 Sep 2020 11:20:46 -0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1
MIME-Version: 1.0
In-Reply-To: <44fcae41-96d8-d0e4-5b8e-cd4419a516a4@si6networks.com>
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/tsv-art/_9bhRdkBFT3W4Wv67WntxGhL7-I>
Subject: Re: [Tsv-art] Tsvart early review of draft-irtf-pearg-numeric-ids-generation-02
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: Tue, 08 Sep 2020 14:48:44 -0000

Hi, Michael,

FWIW, we have posted a rev that hopefully addresses your feedback.

The latest rev is at: 
https://tools.ietf.org/html/draft-irtf-pearg-numeric-ids-generation-03

And the diff from the previous version here: 
https://tools.ietf.org/rfcdiff?url2=draft-irtf-pearg-numeric-ids-generation-03.txt
https://tools.ietf.org/rfcdiff?url2=draft-irtf-pearg-numeric-ids-generation-03.txt

Please do let us know if you think there are ny further changes that 
should be applied.

Thanks a lot!

Regards,
Fernando




On 27/8/20 09:10, Fernando Gont wrote:
> Hi, Michael,
> 
> Thanks a lot for your feedback! In-line...
> 
> On 7/8/20 09:22, Michael Tüxen via Datatracker wrote:
>> Reviewer: Michael Tüxen
>> Review result: Ready with Issues
>>
>> The document is well written, provides algorithms which could be used to
>> address identified problems. One  could add some text covering TCP 
>> timestamps.
> 
> You mean e.g. to spell out which of the proposed algorithms one might 
> use for TCP timestamps?
> 
> 
>> Section 1 states:
>> "Recent history indicates that when new protocols are standardized or
>> new protocol implementations are produced, the security and privacy
>> properties of the associated identifiers tend to be overlooked,..."
>> How does this related to recent/current activities like SCTP/DTLS or 
>> QUIC?
> 
> SCTP (RFC4960) is similar to TCP, in this respect. OTOH, I have only 
> skimmed through the DTLS (RFC6347), and it seems that it initially sets 
> sequence numbers to 0. -- while these are meant to be protected, I'm 
> curious if they could have done with monotonically increasing sequence 
> numbers ala 6528, or with a random origin.
> 
> 
> 
> 
> 
>> The Algorithms in 7.1.1 and 7.1.2 can be substantially simplified when
>> check_suitable_id() always returns true. Why are not the simplified 
>> algorithms
>> shown?
> 
> Are you referring to the case where an implementation need not check the 
> generated ID against other existing I-Ds?
> 
> 
> (FWIW, in order for check_suitable_id()to always return "true", it means 
> that there are essentially no requirements on the ID -- just pick any 
> random number... at which point, there's not really much of an algorithm 
> ( Section 7.1.1 and Section 7.1.2 cover "recovery" strategies for the 
> cases where the algorithm finds collisions.
> 
> 
> 
>> The algorithm in 7.3 (and later) uses a function F and it is stated 
>> that F must
>> be a cryptographically-secure hash function. Couldn't you also use 
>> something
>> like SipHash.
> 
> Siphash *has* been employed for generating IDs.. although it seems that 
> the 64-bits versions are considered too weak. see e.g. 
> https://lore.kernel.org/patchwork/patch/1083206/
> 
> 
> 
>> When reading the algorithm in 7.4.1, I had the impression that it 
>> should also
>> work with id_inc > 1 (by just changing that parameter). If that is 
>> true, I
>> guess you would need to change "if (next_id == max_id)" to "if (max_id -
>> next_id < id_inc) {" and also "next_id = min_id;" to "next_id = min_id +
>> (id_inc - (max_id - next_id + 1));".  If you don't want to be that 
>> generic, you
>> might want to remove id_inc and just use 1.
> 
> You raise a very good point. I'm curious whether it might make sense to 
> include the most generic version (as you suggest), and then also show 
> the simplified version when id_inc=1  ? Thoughts?
> 
> 
> Also, we may want to note that, while using increments of "1" reduces 
> the reuse frequency, in principle id_inc does not even need to be 
> constant... -- in this case, since the failure severity is "hard 
> errors", one probably ones to keep id_inc small, to reduce the ID resuse 
> frequency.
> 
> 
> 
>> The algorithm in 7.4.1 also calls "store_counter(CONTEXT, next_id)" when
>> returning ERROR. Why?
> 
> At least with the current increment values, if the algorithm iterates 
> over count = max_id - min_id + 1; values (i.e., between the range min_id 
> and max_id, the value to be stored would be the same one as originally 
> found, so it's actually a no-op. I will remove it.
> 
> 
>> The algorithms given before 9.4 are given in C code. The Algorithm in 9.4
>> breaks with this rule. Is this intended?
> 
> Are you referring to this line:
>             linear(CONTEXT_2)= linear(CONTEXT_2) + increment();
> ?
> 
> If so, I'll try to find a replacements that's C-compatible and that's 
> still clear...
> 
> Thanks!
> 
> Regards,


-- 
Fernando Gont
SI6 Networks
e-mail: fgont@si6networks.com
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492