Hi Manu, On Tue, Mar 07, 2017 at 03:09:41PM +0100, Emmanuel Hocdet wrote: > >> I choosed xxh64 because it is very quick, the repartion is good and the > >> collision risk is low. Obviously sha1 is better because the collision > >> risk is very low, but is very slow. So I prefer xxh64. > > > > Yep and also in the end we only keep 32 or 64 bit of the resulting hash, > > we're not doing crypto here. The typical use case is to have a reasonably > > good indication whether two very large cipher lists are similar or not > > without storing them. > > For that we need to avoid collision, it's the propriety of cryptographic hash. > xxhash collision risk is low but absolutely not negligible. False positif > is a wound and the real usage of this fingerprint is out of control.
No, the only output of this is to have a reduced output of a combination of ciphers. It's not even related to security, it's just meant to say "looks like this one". Collisions here are a feature since we want a *short* ID for a cipher combination. I was even advocating for 32 bits but we figured it was easy to truncate the resulting string. I mean, we already use xxh64 for the pattern caches and various other things, and here the information it provides is even less important than a truncated user agent. > For the speed, we are in a ssl connection environnement, i doubt that > this is a significant argument. It's not even about speed or anything else, it's about keeping things simple where needed. I personally am not interested in more than *16* bits for this stuff but calling xxh64 gives them and more for free. Anything larger makes it painful to read and compare in a log table for example. In fact you just want to count the statistical occurrences of each combination for reporting purposes. > For the code: > ssl->msg_callback_arg is considering as ssl internal. It should be a very good > think to avoid internal structures/undocumented call usage to try to control > the > beast or limit painful compatibilities. > In this case SSL_set_ex_data and SSL_get_ex_data will do the job. Ah yes definitely, thanks for reporting this. I guess that openssl 1.1 will complain with a warning because of this. > I will try to fix the patch, it breaks my compile environment. OK thanks! Willy

