From: Andy Lutomirski <luto@amacapital.net>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Andy Lutomirski <luto@kernel.org>,
Netdev <netdev@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
"Jason A. Donenfeld" <Jason@zx2c4.com>,
Hannes Frederic Sowa <hannes@stressinduktion.org>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Eric Dumazet <edumazet@google.com>,
Eric Biggers <ebiggers3@gmail.com>,
Tom Herbert <tom@herbertland.com>,
"David S. Miller" <davem@davemloft.net>,
Alexei Starovoitov <ast@kernel.org>
Subject: Re: [PATCH v2 7/8] net: Rename TCA*BPF_DIGEST to ..._SHA256
Date: Wed, 11 Jan 2017 10:19:49 -0800 [thread overview]
Message-ID: <CALCETrVhuszdsfayLrBBkSzJ+A3m+hJjdZQWDj9FOg+2UB_ZBw@mail.gmail.com> (raw)
In-Reply-To: <5875F65A.4010904@iogearbox.net>
On Wed, Jan 11, 2017 at 1:09 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Hi Andy,
>
> On 01/11/2017 04:11 AM, Andy Lutomirski wrote:
>>
>> On Tue, Jan 10, 2017 at 4:50 PM, Daniel Borkmann <daniel@iogearbox.net>
>> wrote:
>>>
>>> On 01/11/2017 12:24 AM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> This makes it easier to add another digest algorithm down the road if
>>>> needed. It also serves to force any programs that might have been
>>>> written against a kernel that had the old field name to notice the
>>>> change and make any necessary changes.
>>>>
>>>> This shouldn't violate any stable API policies, as no released kernel
>>>> has ever had TCA*BPF_DIGEST.
>>>
>>>
>>> Imho, this and patch 6/8 is not really needed. Should there ever
>>> another digest alg be used (doubt it), then you'd need a new nl
>>> attribute and fdinfo line anyway to keep existing stuff intact.
>>> Nobody made the claim that you can just change this underneath
>>> and not respecting abi for existing applications when I read from
>>> above that such apps now will get "forced" to notice a change.
>>
>>
>> Fair enough. I was more concerned about prerelease iproute2 versions,
>> but maybe that's a nonissue. I'll drop these two patches.
>
>
> Ok. Sleeping over this a bit, how about a general rename into
> "prog_tag" for fdinfo and TCA_BPF_TAG resp. TCA_ACT_BPF_TAG for
> the netlink attributes, fwiw, it might reduce any assumptions on
> this being made? If this would be preferable, I could cook that
> patch against -net for renaming it?
That would be fine with me.
I think there are two reasonable approaches to computing the actual tag.
1. Use a standard, modern cryptographic hash. SHA-256, SHA-512,
Blake2b, whatever. SHA-1 is a bad choice in part because it's partly
broken and in part because the implementation in lib/ is a real mess
to use (as you noticed while writing the code).
2. Use whatever algorithm you like but make the tag so short that it's
obviously not collision-free. 48 or 64 bits is probably reasonable.
The intermediate versions are just asking for trouble. Alexei wants
to make the tag shorter, but I admit I still don't understand why he
prefers that over using a better crypto hash and letting user code
truncate the tag if it wants.
next prev parent reply other threads:[~2017-01-11 18:19 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-10 23:24 [PATCH v2 0/8] Switch BPF's digest to SHA256 Andy Lutomirski
2017-01-10 23:24 ` [PATCH v2 1/8] crypto/sha256: Factor out the parts of base API that don't use shash_desc Andy Lutomirski
2017-01-10 23:24 ` [PATCH v2 2/8] crypto/sha256: Export a sha256_{init,update,final}_direct() API Andy Lutomirski
2017-01-10 23:24 ` [PATCH v2 3/8] crypto/sha256: Build the SHA256 core separately from the crypto module Andy Lutomirski
2017-01-10 23:24 ` [PATCH v2 4/8] bpf: Use SHA256 instead of SHA1 for bpf digests Andy Lutomirski
2017-01-10 23:24 ` [PATCH v2 5/8] bpf: Avoid copying the entire BPF program when hashing it Andy Lutomirski
2017-01-10 23:24 ` [PATCH v2 6/8] bpf: Rename fdinfo's prog_digest to prog_sha256 Andy Lutomirski
2017-01-10 23:24 ` [PATCH v2 7/8] net: Rename TCA*BPF_DIGEST to ..._SHA256 Andy Lutomirski
2017-01-11 0:50 ` Daniel Borkmann
2017-01-11 3:11 ` Andy Lutomirski
2017-01-11 9:09 ` Daniel Borkmann
2017-01-11 18:19 ` Andy Lutomirski [this message]
2017-01-13 23:08 ` Daniel Borkmann
2017-01-10 23:24 ` [PATCH v2 8/8] crypto/testmgr: Allocate only the required output size for hash tests Andy Lutomirski
2017-01-11 15:13 ` David Laight
2017-01-11 18:10 ` Andy Lutomirski
2017-01-12 7:47 ` Herbert Xu
2017-01-12 7:52 ` Andy Lutomirski
2017-01-12 16:44 ` Herbert Xu
2017-01-11 1:09 ` [PATCH v2 0/8] Switch BPF's digest to SHA256 Alexei Starovoitov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALCETrVhuszdsfayLrBBkSzJ+A3m+hJjdZQWDj9FOg+2UB_ZBw@mail.gmail.com \
--to=luto@amacapital.net \
--cc=Jason@zx2c4.com \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=ebiggers3@gmail.com \
--cc=edumazet@google.com \
--cc=hannes@stressinduktion.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=tom@herbertland.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.