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

Reply via email to