Date: Mon, 21 Feb 2011 12:01:56 +0100 From: VANHULLEBUS Yvan <vanhu@FreeBSD.org> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r218794 - in head: . sys/netipsec Message-ID: <20110221110156.GA15358@zeninc.net> In-Reply-To: <20110221092143.GA1766@garage.freebsd.pl> References: <201102180940.p1I9eD29050530@svn.freebsd.org> <20110219073412.GC2016@garage.freebsd.pl> <20110221084025.GA14934@zeninc.net> <20110221092143.GA1766@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Feb 21, 2011 at 10:21:43AM +0100, Pawel Jakub Dawidek wrote: > On Mon, Feb 21, 2011 at 09:40:25AM +0100, VANHULLEBUS Yvan wrote: [RFC4868 and MFC] > You can't talk to two such peers with sysctl or without anyway. I assume > that if someone already has tunnels configured and they work, they work, > because the other end uses 96 bits hashes. Once he upgrades there is no > way to get old behaviour back quickly. > > You are changing on-the-wire protocol in the middle of stable branch. Am > I alone in thinking that this is bad idea? That's a good question. Of other people also think it's a bad idea, I can just forget the MFC. But the same problem will happen when we'll release 9.0. Of course, this is easier to explain, as this will be a new branch. > I'm not saying using larger hash is wrong. Quite the opposite. > I actually implemented this for a customer, but never got around merging > it to FreeBSD, because of the on-the-wire protocol change. Well... I'm not "changing the protocol".... I.... just fixed a bug to be RFC compliant..... Yes, for people who may actually use HMAC_SHA2, the result will be the same: broken tunnels. > Imagine a situation where someone does upgrade from 8.2 to 8.3 one of > his IPsec machines. Tunnels stop to work. How can he tell what went > wrong? We don't even log hash mismatches, we only bump some counter. > This is not user-friendly. Situations like that make people angry and > make them want to use FreeBSD a little bit less. The same issue actually does exist: imagine a situation where someone tries to set up an IPsec tunnel (still using HMAC_SHA2) between a FreeBSD (any stable version) and .... a MacOSX, a recent OpenBSD, a recent Linux, or some commercial appliance which provides RFC compliant HMAC_SHA2 support. It won't work, FreeBSD won't log anything but packet rejected due to bad hashes (peer will probably do the same). Afaik, OpenBSD and Linux just switched from round-96 bits draft to RFC4868, without any way to go back to round-96 bits, and it seems there have been no major issues with that. I guess this is because quite no one actually uses HMAC_SHA2 for IPsec phase2. > > Feel free to send me your benchmarks results if you do such an > > implementation... > > I'm sorry, but is really lame way to deal with criticism. Between other people at my work and other FreeBSD people in private discussions, this must be the 6th time I'm having such discussions, so having to do it once again a monday morning made me a bit upset. But of course, you couldn't know that, sorry for the sarcastic reply. > > > it would be best to log a warning that the > > > other side is using old implementation and it (the other side) should be > > > either upgraded or this sysctl should be changed locally to enable old > > > behaviour. > > > > As said upper, afaik, you just can't detect such things without > > wasting LOTS of CPU cycles.... > > Well, checking two hashes isn't really so time consuming. Actually, simply switching from NO_AUTH to HMAC_SHA1 makes loosing between 10 and 20% perfs in our benchs (we loose less on products with hardware SHA1). Of course, this is the whole authentication process, but I strongly guess hash computation is an important part of that job. > The hash is > only truncated, so if you calculate a larger one you also have smaller > one to compare. Yes, if you code looks like that: hash=SHAx_compute(data); truncate_hash(hash, size); memcmp(hash, orighash); IPsec stack is implemented using callbacks to crypto framework, hashes sizes and rounds are sent as parameters to crypto framework functions, which may also be used elsewhere, I'm really not sure we can easilly implement the dual check you're talking about. > Even better, in common case the larger one will simply > match and you have no additional overhead whatsoever. Having said that, > I don't think this is the right solution. We want larger hash to be more > secure and falling back to smaller hash defeats this goal and there is > also the problem when we are initiator. > Hmm, although on hash mismatch we could try comparing smaller hash and > if it matches discard the packet, but log a warning that the other end > is using unsupported, 96 bits hash. Doing that on the first ever IPsec packet sounds good. But doing that on any received packets sounds like wasting lots of CPU cycles, as we don't know how much packets broken peer will send (and how much packets a man in the middle may send, which will eats twice as CPU for hash check as usual). So we would have to keep some flag to remind that this peer is broken, which would require a change in a kernel structure.... between 8.2 and 8.3 ? [....] > > The only thing you may do is to include again the old code and put > > some #ifdef RFC4868_SUPPORT / #else / #endif in kernel code, or set up > > a global sysctl (which will be quite intrusive for such a small > > patchset) so people will be able to decide if they want their whole > > IPsec stack to be RFC4868 or draft-round96 compliant.... > > Small patchset in what sense? In terms of diff size, yes, but because > you change on-the-wire protocol I think the impact is not small at all. Of course, I was talking in terms of diff size. > > But once again, this will NOT solve issues with multiple peers, and > > once again, the "simple" solution when you can't upgrade both peers is > > to switch back to HMAC_SHA1, which is actually really strong enough, > > and which is NOT affected by this patch. > > The "multiple peers issue" is no a valid argument, as I said, because we > can't handle those anyway in any case. I'm happy that we tell users to > switch to HMAC/SHA1, but we currently don't do that. No warning is > logged, for example. Telling that in UPDATING is the best way I found..... well, in fact, the only way I found.... > All in all, I don't really feel convinced. If nobody supports my > concerns, I'll let go sysctl addition, but I'd still strongly recommend > not to MFC this change to stable/8. If the MFC seems to be a problem, I can just forgot that and just keep the fixed code in HEAD, but the question will be there again when we'll release 9.0, and that day, we may have more people using HMAC_SHA2 because they've been told that "SHA1 is not secure anymore" (even if we're takling here about HMAC_SHA1, for packets which have a validity of a few seconds....). Yvan.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110221110156.GA15358>