Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1556123

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

Show all headers | View raw

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.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v2 0/8] Switch BPF's digest to SHA256 Andy Lutomirski <luto@kernel.org> - 2017-01-11 00:30 +0100
  [PATCH v2 1/8] crypto/sha256: Factor out the parts of base API that don't use shash_desc Andy Lutomirski <luto@kernel.org> - 2017-01-11 00:30 +0100
  [PATCH v2 6/8] bpf: Rename fdinfo's prog_digest to prog_sha256 Andy Lutomirski <luto@kernel.org> - 2017-01-11 00:30 +0100
  [PATCH v2 8/8] crypto/testmgr: Allocate only the required output size for hash tests Andy Lutomirski <luto@kernel.org> - 2017-01-11 00:30 +0100
    RE: [PATCH v2 8/8] crypto/testmgr: Allocate only the required  output size for hash tests David Laight <David.Laight@ACULAB.COM> - 2017-01-11 16:20 +0100
      Re: [PATCH v2 8/8] crypto/testmgr: Allocate only the required output  size for hash tests Andy Lutomirski <luto@amacapital.net> - 2017-01-11 19:20 +0100
    Re: [PATCH v2 8/8] crypto/testmgr: Allocate only the required output  size for hash tests Herbert Xu <herbert@gondor.apana.org.au> - 2017-01-12 08:50 +0100
      Re: [PATCH v2 8/8] crypto/testmgr: Allocate only the required output  size for hash tests Andy Lutomirski <luto@amacapital.net> - 2017-01-12 09:00 +0100
    Re: [PATCH v2 8/8] crypto/testmgr: Allocate only the required output  size for hash tests Herbert Xu <herbert@gondor.apana.org.au> - 2017-01-12 17:50 +0100
  [PATCH v2 3/8] crypto/sha256: Build the SHA256 core separately from the crypto module Andy Lutomirski <luto@kernel.org> - 2017-01-11 00:30 +0100
  [PATCH v2 4/8] bpf: Use SHA256 instead of SHA1 for bpf digests Andy Lutomirski <luto@kernel.org> - 2017-01-11 00:30 +0100
  [PATCH v2 2/8] crypto/sha256: Export a sha256_{init,update,final}_direct() API Andy Lutomirski <luto@kernel.org> - 2017-01-11 00:30 +0100
  Re: [PATCH v2 0/8] Switch BPF's digest to SHA256 Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2017-01-11 02:20 +0100

csiph-web