From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752744AbcLMXgh (ORCPT ); Tue, 13 Dec 2016 18:36:37 -0500 Received: from frisell.zx2c4.com ([192.95.5.64]:45495 "EHLO frisell.zx2c4.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751995AbcLMXgf (ORCPT ); Tue, 13 Dec 2016 18:36:35 -0500 MIME-Version: 1.0 In-Reply-To: References: From: "Jason A. Donenfeld" Date: Wed, 14 Dec 2016 00:36:22 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3] siphash: add cryptographically secure hashtable function To: Linus Torvalds Cc: Andi Kleen , "kernel-hardening@lists.openwall.com" , LKML , Linux Crypto Mailing List , George Spelvin , Scott Bauer , Andy Lutomirski , Greg KH , Eric Biggers , Jean-Philippe Aumasson , "Daniel J . Bernstein" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Linus, On Tue, Dec 13, 2016 at 8:25 PM, Linus Torvalds wrote: > Yeah,. the TCP sequence number md5_transform() cases are likely the > best example of something where siphash might be good. That tends to > be really just a couple words of data (the address and port info) plus > the net_secret[] hash. I think they currently simply just fill in the > fixed-sized 64-byte md5-round area. > > I wonder it's worth it to have a special spihash version that does > that same "fixed 64-byte area" thing. What happens in MD5 the hash function is that it first initializes its initial 128-bit hash to a magic constant, and then reads 64 bytes at a time from the input and calls md5_transform on that, which each time manipulates that 128-bit value from its starting value. At the end of the input, some special padding is applied for small final blocks, some finalization, and then the resultant hash is whatever that 128-bit value is at the end of the process. What the tcp stack does with secure_tcp_sequence_number function in net/core/secure_seq.c, and a variety of other places, is to just supply that 128-bit initial value not with the magic constant, but instead with saddr||daddr||sport||dport||net_secret[15] and then calls md5_transform on the 64-byte long term secret random value (net_secret). From the resultant 128-bit value, they take the first 32-bits. In addition to being rather heavy weight, this strikes me as cryptographically a bit dubious too. But that's where your "fixed 64-byte area" notion comes from. Siphash makes things a lot more simple than that. Since siphash is a PRF and not a mere hash function, it takes an explicit secret key parameter, which would be net_secret, some input data, which would be saddr||daddr||sport||dport, and then spits out a 64-bit number, 32 bits of which would be used as the sequence number. seq_num = seq_scale(siphash24(saddr||daddr||sport||dport, net_secret)); A lot simpler, faster, and actually secure. > But please talk to the netwotrking people. Maybe that's the proper way > to get this merged? I had hoped to do it the lazy way, and just have it just wind up in lib/. But I suppose you and Greg are of course right, and I should submit this with a real usage. So I'll do that, and resubmit in another thread as a series to LKML and netdev. Thanks for your feedback! Jason