netdev - Re: [PATCH v2 0/8] Switch BPF's digest to SHA256

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Jan 2017 17:09:42 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Eric Biggers <ebiggers3@...il.com>,
        Tom Herbert <tom@...bertland.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2 0/8] Switch BPF's digest to SHA256

On Tue, Jan 10, 2017 at 03:24:38PM -0800, Andy Lutomirski wrote:
> I can imagine future uses for the new-in-4.10 BPF digest feature that
> would be problematic if malicious users could produce collisions, and
> SHA-1 is no longer consdiered to be collision-free.  Even without
> needing collision resistance, SHA-1 is no longer recommended for new
> applications.  Switch the BPF digest to SHA-256 instead.

Using this reasoning we must immediately change 'git' to use sha256
as well. malicious users are coming!
Seriously, NACK for bpf bits again, since you somehow missed
the reasons I gave earlier, here they are again:
.......
This statement is also bogus. The only reason we added prog_digest is
to improve debuggability and introspection of bpf programs.
As I said in the previous thread "collisions are ok" and we could have
used jhash here to avoid patches like this ever appearing
and wasting everyones time.

sha1 is 20 bytes which is already a bit long to print and copy paste by humans.
whereas 4 byte jhash is a bit too short, since collisions are not that rare
and may lead to incorrect assumptions from the users that develop the programs.
I would prefer something in 6-10 byte range that prevents collisions most of
the time and short to print as hex, but I don't know of anything like this
in the existing kernel and inventing bpf specific hash is not great.
Another requirement for debugging (and prog_digest) that user space
should be able to produce the same hash without asking kernel, so
sha1 fits that as well, since it's well known and easy to put into library.

sha256 doesn't fit either of these requirements. 32-bytes are too long to print
and when we use it as a substitue for the prog name for jited ksym, looking
at long function names will screw up all tools like perf, which we don't
want. sha256 is equally not easy for user space app like iproute2,
so not an acceptable choice from that pov either.
...........

tldr: I see only Cons for sha1->sha256 switch. Not a single Pro, hence
nack for bpf patches 4+
The patches 1-3 are nice and useful on their own.