{"thread":{"id":"50110","subject":"[PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs","startedAt":"2018-12-27T02:35:54Z","lastAt":"2018-12-28T20:13:47Z","messageCount":4,"participants":["Rosen Penev","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"365829","messageId":"20181227023548.396-1-rosenp@gmail.com","threadId":"50110","inReplyTo":null,"subject":"[PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs","fromName":"Rosen Penev","fromEmail":"rosenp@gmail.com","sentAt":"2018-12-27T02:35:48Z","receivedAt":"2018-12-27T02:35:54Z","isPatch":true,"sender":{"key":"rosenp@gmail.com","avatar":"https://gravatar.com/avatar/33a030031a58410cc9dd6b414805d443c63099a64cbe92fd432cfee8e8db9755?d=mp&s=160"},"body":"Initialization in OpenSSL has been deprecated in version 1.1. This makes\ncompilation fail when deprecated APIs for OpenSSL are compile-time\ndisabled.\n\nSigned-off-by: Rosen Penev <rosenp@gmail.com>\n---\n imap-send.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex b4eb886e2..21f741c8c 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve\n \tint ret;\n \tX509 *cert;\n \n+#if (OPENSSL_VERSION_NUMBER < 0x10000000L)\n \tSSL_library_init();\n \tSSL_load_error_strings();\n+#endif\n \n \tmeth = SSLv23_method();\n \tif (!meth) {\n-- \n2.20.1\n\n"},{"id":"365831","messageId":"xmqqwonvpjiy.fsf@gitster-ct.c.googlers.com","threadId":"50110","inReplyTo":"20181227023548.396-1-rosenp@gmail.com","subject":"Re: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-27T06:32:05Z","receivedAt":"2018-12-27T06:32:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rosen Penev <rosenp@gmail.com> writes:\n\n> Initialization in OpenSSL has been deprecated in version 1.1.\n\nhttps://www.openssl.org/docs/man1.0.2/ssl/SSL_library_init.html says \n\n\tSSL_library_init() must be called before any other action takes\n\tplace.\n\nhttps://www.openssl.org/docs/man1.1.0/ssl/SSL_library_init.html says\nthe same.\n\nWhich makes it necessary for us to defend the following claim\n\n> This makes\n> compilation fail when deprecated APIs for OpenSSL are compile-time\n> disabled.\n\nas a valid problem description more rigorously.  To me, the cursory\nweb-serfing I did above makes me suspect that an OpenSSL\nimplementation with such a compile-time disabling _is_ buggy, as it\nforbids the API users to call an API function they are told to call\nbefore doing anything else.\n\n> Signed-off-by: Rosen Penev <rosenp@gmail.com>\n> ---\n>  imap-send.c | 2 ++\n>  1 file changed, 2 insertions(+)\n>\n> diff --git a/imap-send.c b/imap-send.c\n> index b4eb886e2..21f741c8c 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve\n>  \tint ret;\n>  \tX509 *cert;\n>  \n> +#if (OPENSSL_VERSION_NUMBER < 0x10000000L)\n\nhttps://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_VERSION_NUMBER.html\n\nsays that OPENSSL_VERSION_NUMBER is of form 0xMNNFFPPS where M is\nmajor, NN is minor, FF is fix, PP is patch and S is status, and\ngives an example that 0x00906023 stands for 0.9.6.b beta 3 (M=0,\nNN=09, FF=06, PP=02 and S=3).  So \"< 0x10000000L\" means \"anything\nwith M smaller than 1\".  IOW, we would no longer call _init() for\ne.g. \"version 1.0.0 beta 0\".  That contradicts with the first claim\nof the proposed log message (\"deprecated in 1.1\" implying that it is\nnot yet deprecated in say 1.0.2).\n\n\n\n>  \tSSL_library_init();\n>  \tSSL_load_error_strings();\n> +#endif\n>  \n>  \tmeth = SSLv23_method();\n>  \tif (!meth) {\n"},{"id":"365873","messageId":"CAKxU2N9egn6MbJeWUWFsyYpnwOCj4=mckmkJJtVJGhmQUt36aw@mail.gmail.com","threadId":"50110","inReplyTo":"xmqqwonvpjiy.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs","fromName":"Rosen Penev","fromEmail":"rosenp@gmail.com","sentAt":"2018-12-27T17:55:37Z","receivedAt":"2018-12-27T17:55:50Z","isPatch":true,"sender":{"key":"rosenp@gmail.com","avatar":"https://gravatar.com/avatar/33a030031a58410cc9dd6b414805d443c63099a64cbe92fd432cfee8e8db9755?d=mp&s=160"},"body":"On Wed, Dec 26, 2018 at 10:32 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Rosen Penev <rosenp@gmail.com> writes:\n>\n> > Initialization in OpenSSL has been deprecated in version 1.1.\n>\n> https://www.openssl.org/docs/man1.0.2/ssl/SSL_library_init.html says\n>\n>         SSL_library_init() must be called before any other action takes\n>         place.\n>\n> https://www.openssl.org/docs/man1.1.0/ssl/SSL_library_init.html says\n> the same.\nLater on in the document it mentions that it is deprecated.\n>\n> Which makes it necessary for us to defend the following claim\n>\n> > This makes\n> > compilation fail when deprecated APIs for OpenSSL are compile-time\n> > disabled.\n>\n> as a valid problem description more rigorously.  To me, the cursory\n> web-serfing I did above makes me suspect that an OpenSSL\n> implementation with such a compile-time disabling _is_ buggy, as it\n> forbids the API users to call an API function they are told to call\n> before doing anything else.\nI agree the man page is misleading. The changelog for 1.1.0 is very\nclear though:\n\nAdded support for auto-initialisation and de-initialisation of the library.\n     OpenSSL no longer requires explicit init or deinit routines to be called,\n     except in certain circumstances. See the OPENSSL_init_crypto() and\n     OPENSSL_init_ssl() man pages for further information.\n     [Matt Caswell]\n\nhttps://www.openssl.org/news/changelog.html#x12\n\n>\n> > Signed-off-by: Rosen Penev <rosenp@gmail.com>\n> > ---\n> >  imap-send.c | 2 ++\n> >  1 file changed, 2 insertions(+)\n> >\n> > diff --git a/imap-send.c b/imap-send.c\n> > index b4eb886e2..21f741c8c 100644\n> > --- a/imap-send.c\n> > +++ b/imap-send.c\n> > @@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve\n> >       int ret;\n> >       X509 *cert;\n> >\n> > +#if (OPENSSL_VERSION_NUMBER < 0x10000000L)\n>\n> https://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_VERSION_NUMBER.html\n>\n> says that OPENSSL_VERSION_NUMBER is of form 0xMNNFFPPS where M is\n> major, NN is minor, FF is fix, PP is patch and S is status, and\n> gives an example that 0x00906023 stands for 0.9.6.b beta 3 (M=0,\n> NN=09, FF=06, PP=02 and S=3).  So \"< 0x10000000L\" means \"anything\n> with M smaller than 1\".  IOW, we would no longer call _init() for\n> e.g. \"version 1.0.0 beta 0\".  That contradicts with the first claim\n> of the proposed log message (\"deprecated in 1.1\" implying that it is\n> not yet deprecated in say 1.0.2).\nThis is a mistake. I will send a v2 to fix.\n\nOh I see what I did wrong. I mistakenly copied the above\nOPENSSL_VERSION_NUMBER check without looking carefully at the number.\n>\n>\n>\n> >       SSL_library_init();\n> >       SSL_load_error_strings();\n> > +#endif\n> >\n> >       meth = SSLv23_method();\n> >       if (!meth) {\n"},{"id":"365925","messageId":"xmqqftuhmmtl.fsf@gitster-ct.c.googlers.com","threadId":"50110","inReplyTo":"CAKxU2N9egn6MbJeWUWFsyYpnwOCj4=mckmkJJtVJGhmQUt36aw@mail.gmail.com","subject":"Re: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-28T19:20:00Z","receivedAt":"2018-12-28T20:13:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rosen Penev <rosenp@gmail.com> writes:\n\n> On Wed, Dec 26, 2018 at 10:32 PM Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Rosen Penev <rosenp@gmail.com> writes:\n>>\n>> > Initialization in OpenSSL has been deprecated in version 1.1.\n>>\n>> https://www.openssl.org/docs/man1.0.2/ssl/SSL_library_init.html says\n>>\n>>         SSL_library_init() must be called before any other action takes\n>>         place.\n>>\n>> https://www.openssl.org/docs/man1.1.0/ssl/SSL_library_init.html says\n>> the same.\n> Later on in the document it mentions that it is deprecated.\n>>\n>> Which makes it necessary for us to defend the following claim\n>>\n>> > This makes\n>> > compilation fail when deprecated APIs for OpenSSL are compile-time\n>> > disabled.\n>>\n>> as a valid problem description more rigorously.  To me, the cursory\n>> web-serfing I did above makes me suspect that an OpenSSL\n>> implementation with such a compile-time disabling _is_ buggy, as it\n>> forbids the API users to call an API function they are told to call\n>> before doing anything else.\n> I agree the man page is misleading. The changelog for 1.1.0 is very\n> clear though:\n>\n> Added support for auto-initialisation and de-initialisation of the library.\n>      OpenSSL no longer requires explicit init or deinit routines to be called,\n>      except in certain circumstances. See the OPENSSL_init_crypto() and\n>      OPENSSL_init_ssl() man pages for further information.\n>      [Matt Caswell]\n\nAll it says is explicit calls to SSL_library_init() is now optional.\nIt does not say it is a compile-time-error worthy offense to make\nexplicit init/deinit calls.  Which still means that an OpenSSL\nimplementation with such a compile-time disabling _is_ buggy, as it\nforbids the API users to call an API function they are told to call\nbefore doing anything else.\n\n>> > diff --git a/imap-send.c b/imap-send.c\n>> > index b4eb886e2..21f741c8c 100644\n>> > --- a/imap-send.c\n>> > +++ b/imap-send.c\n>> > @@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve\n>> >       int ret;\n>> >       X509 *cert;\n>> >\n>> > +#if (OPENSSL_VERSION_NUMBER < 0x10000000L)\n>>\n>> https://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_VERSION_NUMBER.html\n>>\n>> says that OPENSSL_VERSION_NUMBER is of form 0xMNNFFPPS where M is\n>> major, NN is minor, FF is fix, PP is patch and S is status, and\n>> gives an example that 0x00906023 stands for 0.9.6.b beta 3 (M=0,\n>> NN=09, FF=06, PP=02 and S=3).  So \"< 0x10000000L\" means \"anything\n>> with M smaller than 1\".  IOW, we would no longer call _init() for\n>> e.g. \"version 1.0.0 beta 0\".  That contradicts with the first claim\n>> of the proposed log message (\"deprecated in 1.1\" implying that it is\n>> not yet deprecated in say 1.0.2).\n> This is a mistake. I will send a v2 to fix.\n>\n> Oh I see what I did wrong. I mistakenly copied the above\n> OPENSSL_VERSION_NUMBER check without looking carefully at the number.\n\nYup, please be careful next time.\n\n>>\n>>\n>>\n>> >       SSL_library_init();\n>> >       SSL_load_error_strings();\n>> > +#endif\n>> >\n>> >       meth = SSLv23_method();\n>> >       if (!meth) {\n"}]}