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

Fernando Gont <fgont@si6networks.com> Thu, 27 August 2020 12:10 UTC

Return-Path: <fgont@si6networks.com>
X-Original-To: pearg@ietfa.amsl.com
Delivered-To: pearg@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 1DB383A0C4E for <pearg@ietfa.amsl.com>; Thu, 27 Aug 2020 05:10:26 -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=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 VrquS6bCu5Ja for <pearg@ietfa.amsl.com>; Thu, 27 Aug 2020 05:10:24 -0700 (PDT)
Received: from fgont.go6lab.si (fgont.go6lab.si [IPv6:2001:67c:27e4::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 2F1023A0C4C for <pearg@irtf.org>; Thu, 27 Aug 2020 05:10:24 -0700 (PDT)
Received: from [IPv6:2800:810:464:965:ec24:6f8a:1aff:81d] (unknown [IPv6:2800:810:464:965:ec24:6f8a:1aff:81d]) (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 2BCB12836FA; Thu, 27 Aug 2020 12:10:20 +0000 (UTC)
From: Fernando Gont <fgont@si6networks.com>
To: =?UTF-8?Q?Michael_T=c3=bcxen?= <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>
Message-ID: <44fcae41-96d8-d0e4-5b8e-cd4419a516a4@si6networks.com>
Date: Thu, 27 Aug 2020 09:10:08 -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: <159680292803.8931.4890868238678597521@ietfa.amsl.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/pearg/yo81QZJ4XG42l9akb7hUVWxFm8k>
Subject: Re: [Pearg] Tsvart early review of draft-irtf-pearg-numeric-ids-generation-02
X-BeenThere: pearg@irtf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Privacy Enhancements and Assessment Proposed RG <pearg.irtf.org>
List-Unsubscribe: <https://www.irtf.org/mailman/options/pearg>, <mailto:pearg-request@irtf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/pearg/>
List-Post: <mailto:pearg@irtf.org>
List-Help: <mailto:pearg-request@irtf.org?subject=help>
List-Subscribe: <https://www.irtf.org/mailman/listinfo/pearg>, <mailto:pearg-request@irtf.org?subject=subscribe>
X-List-Received-Date: Thu, 27 Aug 2020 12:10:26 -0000

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