On 2017/05/24 11:44, Nils Frohberg wrote:
> On Wed, May 24, 2017 at 10:04:12AM +0100, Stuart Henderson wrote:
> > On 2017/05/24 10:43, Nils Frohberg wrote:
> > > On Wed, May 24, 2017 at 09:11:24AM +0100, Stuart Henderson wrote:
> > > > On 2017/05/24 10:03, Nils Frohberg wrote:
> > > > > The patch disables the symbols
> > > > >       ECDSA_METHOD_new
> > > > >       ECDSA_METHOD_free
> > > > >       ECDSA_METHOD_set_flags
> > > > >       ECDSA_METHOD_set_name
> > > > > that get pulled in due to the test
> > > > > #if OPENSSL_VERSION_NUMBER >= 0x10002000L
> > > > 
> > > > Is there any more-targetted check you can make rather than 'if 0',
> > > > are there any related macros that were introduced at the same time as
> > > > the symbols you could use instead?
> > > 
> > > LibreSSL sets OPENSSL_VERSION_NUMBER to 0x20000000L. So we could
> > >   1) change the outter #if from ">= 0x10002000L" to "== 0x10002000L",
> > >      but then we don't include symbols that are actually there
> > >   2) change the "#if 0" to "#if OPENSSL_VERSION_NUMBER == 0x10002000L"
> > >   3) test for "LIBRESSL_VERSION_NUMBER" on the four functions to
> > >      see if we're using LibreSSL
> > > 
> > > I would suggest 2) or 3), but I don't know which fits better. 3)
> > > could be changed to a "<=" test later, in case the functions get
> > > implemented.
> > 
> > I'm wondering if there's something related to the function, rather than a 
> > pure
> > version number check. Sometimes it's not possible but that would be the 
> > first
> > choice if so.
> 
> This commit seems to be the one that adds said functions:
> https://github.com/openssl/openssl/commit/94c2f77a62be7079ab1893ab14b18a30157c4532
> 
> According to this, #define ECDSA_F_ECDSA_METHOD_NEW gets auto-generated.
> We could use this to test for the function. This assumes that if
> the functions are added in the future, a) this #define will also
> be added/generated, and b) all four functions are implemented (the
> other 3 functions don't get #defines).
> 
> > In the absence of that, your updated version is OK, at least when there's a
> > LIBRESSL_VERSION_NUMBER check it's clear what the patch is doing and we'll
> > find it when we grep the ports tree if the functions are added in the 
> > future.
> 
> Here's a new .tar.gz. I also removed the comment since it's rather redundant 
> now.
> Patch:
> 
> --- ECDSA.xs.orig       Fri Jan  2 02:24:13 2015
> +++ ECDSA.xs    Wed May 24 11:36:14 2017
> @@ -80,6 +80,8 @@ ECDSA_set_method(EC_KEY *eckey, const ECDSA_METHOD *me
>  int      
>  ECDSA_size(const EC_KEY *eckey)
>  
> +#ifdef ECDSA_F_ECDSA_METHOD_NEW
> +
>  ECDSA_METHOD *
>  ECDSA_METHOD_new(ECDSA_METHOD *ecdsa_method=0)
>  
> @@ -91,6 +93,8 @@ ECDSA_METHOD_set_flags(ECDSA_METHOD *ecdsa_method, int
>  
>  void 
>  ECDSA_METHOD_set_name(ECDSA_METHOD *ecdsa_method, char *name)
> +
> +#endif
>  
>  void 
>  ERR_load_ECDSA_strings()


Seems a good choice to me, OK sthen@ for anyone to import..

Reply via email to