Hi, On Thursday, January 21 2021 at 22:34:27 +0100, Tim Düsterhus wrote: > Bertrand, > > Note: I was the contributor that added the secure_memcmp converter. > > Am 21.01.21 um 22:16 schrieb Bertrand Jacquin: > > diff --git a/doc/configuration.txt b/doc/configuration.txt > > index 899bdf553a85..f25da9c1bfa6 100644 > > --- a/doc/configuration.txt > > +++ b/doc/configuration.txt > > @@ -15996,7 +15996,11 @@ secure_memcmp(<var>) > > performed in constant time. > > > > Please note that this converter is only available when haproxy has been > > - compiled with USE_OPENSSL. > > + compiled with USE_OPENSSL. Requires at least OpenSSL 1.0.2. > > + > > + See also the strcmp converter if you need to compare two binary > > + strings without concern related to constant time or if OpenSSL is not > > + enabled. > > The strcmp converter is not binary safe. It uses strncmp internally.
It is not indeed, what do you think of improving current related strcmp documentation and example to add an hex conversion to achieve the same goal? This would be pretty slow, I'd be happy if you have something more efficient to offer for this use case. Also, CRYPTO_memcmp() is relatively simple and could be rewritten in openssl compat as an inline function too. > > Example : > > > > diff --git a/src/sample.c b/src/sample.c > > index bf2de2a2522d..bb12789b551f 100644 > > --- a/src/sample.c > > +++ b/src/sample.c > > @@ -3100,12 +3100,14 @@ static int sample_conv_strcmp(const struct arg > > *arg_p, struct sample *smp, void > > return 1; > > } > > > > -#ifdef USE_OPENSSL > > +#if defined(USE_OPENSSL) && HA_OPENSSL_VERSION_NUMBER > 0x1000200fL > > We strive to use feature detection instead of version number comparisons > for SSL. Is it possible to use feature detection here? Adding Ilya to CC. Feature detection is definitely the way to go, however it is not available for CRYPTO_memcmp() specifically. You can find the initial definition of this function in https://git.openssl.org/?p=openssl.git;a=commit;h=2ee798880a246d648ecddadc5b91367bee4a5d98 as part of OpenSSL 1.0.1d release (I'll adjust that as well). > > /* Compares bytestring with a variable containing a bytestring. Return > > value > > * is `true` if both bytestrings are bytewise identical and `false` > > otherwise. > > * > > - * Comparison will be performed in constant time if both bytestrings are of > > - * the same length. If the lengths differ execution time will not be > > constant. > > + * Comparison will be performed in constant time if the library support > > + * constant time memcmp (starting with OpenSSL 1.0.2) and if both > > + * bytestrings are of the same length. Otherwise execution time will not > > + * be constant. > > I am not sure whether this wording change is useful, as the definition > of the function already is guarded by the #if. As such > sample_conv_secure_memcmp guarantees the constant time comparison > (independent of the library support). It just might be that the function > might not exist. You are correct, that's a left over from initial change :) I'll send a new version with this adjusted once the first point is addressed. Cheers, Bertrand -- Bertrand
