threads / patch / 50110

patchimap-send: Fix compilation without deprecated OpenSSL APIs

Subject: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs

## tl;dr

4 messages between Dec 27, 2018 and Dec 28, 2018. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Rosen Penev· Dec 27, 2018, 02:35 UTC · lore

Initialization in OpenSSL has been deprecated in version 1.1. This makes compilation fail when deprecated APIs for OpenSSL are compile-time disabled.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 imap-send.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to imap-send.c +2 −0
diff --git a/imap-send.c b/imap-send.c
index b4eb886e2..21f741c8c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	int ret;
 	X509 *cert;
 
+#if (OPENSSL_VERSION_NUMBER < 0x10000000L)
 	SSL_library_init();
 	SSL_load_error_strings();
+#endif
 
 	meth = SSLv23_method();
 	if (!meth) {

-- 
2.20.1
Junio C Hamano· Dec 27, 2018, 06:32 UTC · re: Rosen Penev · lore

Re: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs

Rosen Penev <rosenp@gmail.com> writes:
> Initialization in OpenSSL has been deprecated in version 1.1.
https://www.openssl.org/docs/man1.0.2/ssl/SSL_library_init.html says 
	SSL_library_init() must be called before any other action takes
	place.

https://www.openssl.org/docs/man1.1.0/ssl/SSL_library_init.html says the same.

Which makes it necessary for us to defend the following claim
> This makes
> compilation fail when deprecated APIs for OpenSSL are compile-time
> disabled.

as a valid problem description more rigorously. To me, the cursory web-serfing I did above makes me suspect that an OpenSSL implementation with such a compile-time disabling _is_ buggy, as it forbids the API users to call an API function they are told to call before doing anything else.

Show 14 quoted lines
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  imap-send.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/imap-send.c b/imap-send.c
> index b4eb886e2..21f741c8c 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
>  	int ret;
>  	X509 *cert;
>  
> +#if (OPENSSL_VERSION_NUMBER < 0x10000000L)
https://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_VERSION_NUMBER.html

says that OPENSSL_VERSION_NUMBER is of form 0xMNNFFPPS where M is major, NN is minor, FF is fix, PP is patch and S is status, and gives an example that 0x00906023 stands for 0.9.6.b beta 3 (M=0, NN=09, FF=06, PP=02 and S=3). So "< 0x10000000L" means "anything with M smaller than 1". IOW, we would no longer call _init() for e.g. "version 1.0.0 beta 0". That contradicts with the first claim of the proposed log message ("deprecated in 1.1" implying that it is not yet deprecated in say 1.0.2).

Show 6 quoted lines
>  	SSL_library_init();
>  	SSL_load_error_strings();
> +#endif
>  
>  	meth = SSLv23_method();
>  	if (!meth) {
Rosen Penev· Dec 27, 2018, 17:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs

On Wed, Dec 26, 2018 at 10:32 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
>
> Rosen Penev <rosenp@gmail.com> writes:
>
> > Initialization in OpenSSL has been deprecated in version 1.1.
>
> https://www.openssl.org/docs/man1.0.2/ssl/SSL_library_init.html says
>
>         SSL_library_init() must be called before any other action takes
>         place.
>
> https://www.openssl.org/docs/man1.1.0/ssl/SSL_library_init.html says
> the same.
Later on in the document it mentions that it is deprecated.
Show 12 quoted lines
>
> Which makes it necessary for us to defend the following claim
>
> > This makes
> > compilation fail when deprecated APIs for OpenSSL are compile-time
> > disabled.
>
> as a valid problem description more rigorously.  To me, the cursory
> web-serfing I did above makes me suspect that an OpenSSL
> implementation with such a compile-time disabling _is_ buggy, as it
> forbids the API users to call an API function they are told to call
> before doing anything else.

I agree the man page is misleading. The changelog for 1.1.0 is very clear though:

Added support for auto-initialisation and de-initialisation of the library.
     OpenSSL no longer requires explicit init or deinit routines to be called,
     except in certain circumstances. See the OPENSSL_init_crypto() and
     OPENSSL_init_ssl() man pages for further information.
     [Matt Caswell]
https://www.openssl.org/news/changelog.html#x12
Show 26 quoted lines
>
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  imap-send.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/imap-send.c b/imap-send.c
> > index b4eb886e2..21f741c8c 100644
> > --- a/imap-send.c
> > +++ b/imap-send.c
> > @@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
> >       int ret;
> >       X509 *cert;
> >
> > +#if (OPENSSL_VERSION_NUMBER < 0x10000000L)
>
> https://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_VERSION_NUMBER.html
>
> says that OPENSSL_VERSION_NUMBER is of form 0xMNNFFPPS where M is
> major, NN is minor, FF is fix, PP is patch and S is status, and
> gives an example that 0x00906023 stands for 0.9.6.b beta 3 (M=0,
> NN=09, FF=06, PP=02 and S=3).  So "< 0x10000000L" means "anything
> with M smaller than 1".  IOW, we would no longer call _init() for
> e.g. "version 1.0.0 beta 0".  That contradicts with the first claim
> of the proposed log message ("deprecated in 1.1" implying that it is
> not yet deprecated in say 1.0.2).
This is a mistake. I will send a v2 to fix.

Oh I see what I did wrong. I mistakenly copied the above OPENSSL_VERSION_NUMBER check without looking carefully at the number.

Show 9 quoted lines
>
>
>
> >       SSL_library_init();
> >       SSL_load_error_strings();
> > +#endif
> >
> >       meth = SSLv23_method();
> >       if (!meth) {
Junio C Hamano· Dec 28, 2018, 19:20 UTC · re: Rosen Penev · lore

Re: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs

Rosen Penev <rosenp@gmail.com> writes:
Show 34 quoted lines
> On Wed, Dec 26, 2018 at 10:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Rosen Penev <rosenp@gmail.com> writes:
>>
>> > Initialization in OpenSSL has been deprecated in version 1.1.
>>
>> https://www.openssl.org/docs/man1.0.2/ssl/SSL_library_init.html says
>>
>>         SSL_library_init() must be called before any other action takes
>>         place.
>>
>> https://www.openssl.org/docs/man1.1.0/ssl/SSL_library_init.html says
>> the same.
> Later on in the document it mentions that it is deprecated.
>>
>> Which makes it necessary for us to defend the following claim
>>
>> > This makes
>> > compilation fail when deprecated APIs for OpenSSL are compile-time
>> > disabled.
>>
>> as a valid problem description more rigorously.  To me, the cursory
>> web-serfing I did above makes me suspect that an OpenSSL
>> implementation with such a compile-time disabling _is_ buggy, as it
>> forbids the API users to call an API function they are told to call
>> before doing anything else.
> I agree the man page is misleading. The changelog for 1.1.0 is very
> clear though:
>
> Added support for auto-initialisation and de-initialisation of the library.
>      OpenSSL no longer requires explicit init or deinit routines to be called,
>      except in certain circumstances. See the OPENSSL_init_crypto() and
>      OPENSSL_init_ssl() man pages for further information.
>      [Matt Caswell]

All it says is explicit calls to SSL_library_init() is now optional. It does not say it is a compile-time-error worthy offense to make explicit init/deinit calls. Which still means that an OpenSSL implementation with such a compile-time disabling _is_ buggy, as it forbids the API users to call an API function they are told to call before doing anything else.

Show 24 quoted lines
>> > diff --git a/imap-send.c b/imap-send.c
>> > index b4eb886e2..21f741c8c 100644
>> > --- a/imap-send.c
>> > +++ b/imap-send.c
>> > @@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
>> >       int ret;
>> >       X509 *cert;
>> >
>> > +#if (OPENSSL_VERSION_NUMBER < 0x10000000L)
>>
>> https://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_VERSION_NUMBER.html
>>
>> says that OPENSSL_VERSION_NUMBER is of form 0xMNNFFPPS where M is
>> major, NN is minor, FF is fix, PP is patch and S is status, and
>> gives an example that 0x00906023 stands for 0.9.6.b beta 3 (M=0,
>> NN=09, FF=06, PP=02 and S=3).  So "< 0x10000000L" means "anything
>> with M smaller than 1".  IOW, we would no longer call _init() for
>> e.g. "version 1.0.0 beta 0".  That contradicts with the first claim
>> of the proposed log message ("deprecated in 1.1" implying that it is
>> not yet deprecated in say 1.0.2).
> This is a mistake. I will send a v2 to fix.
>
> Oh I see what I did wrong. I mistakenly copied the above
> OPENSSL_VERSION_NUMBER check without looking carefully at the number.
Yup, please be careful next time.
Show 9 quoted lines
>>
>>
>>
>> >       SSL_library_init();
>> >       SSL_load_error_strings();
>> > +#endif
>> >
>> >       meth = SSLv23_method();
>> >       if (!meth) {

← back to recent threads