Re: Facing error in git-imap-send while compiling Git

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Nirmal,

On Fri, 31 Jan 2020, Nirmal Khedkar wrote:

> I'll admit I'm quite perplexed with this OpenSSL problem that I'm
> facing. Here's what I've done along with their results:
> 1. eroen and Jack Bates' suggestions
> (https://lore.kernel.org/git/66967e0e-8bd9-f4b6-d2d4-ccce9004f42e@nottheoilrig.com/)
> 2. I've also implemented Johannes' suggestions, and I'm still facing
> the same problem.

Could you repeat the symptoms? I forgot the details since you posted your
previous email.

> Here's the final diff:
> ---
> diff --git a/imap-send.c b/imap-send.c
> index 6c54d8c29d..3248bc2123 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -41,7 +41,9 @@ typedef void *SSL;
>  /* We don't have curl, so continue to use the historical implementation */
>  #define USE_CURL_DEFAULT 0
>  #endif
> -
> +#ifndef SSL_library_init
> +       #define SSL_library_init();
> +#endif
>  static int verbosity;
>  static int use_curl = USE_CURL_DEFAULT;
>
> @@ -59,6 +61,13 @@ static struct option imap_send_options[] = {
>  #define DRV_BOX_BAD     -2
>  #define DRV_STORE_BAD   -3
>
> +
> +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> +       #define OPENSSL_sk_num(x) sk_GENERAL_NAME_num(x)
> +       #define OPENSSL_sk_value(x,y) sk_GENERAL_NAME_value((x),(y))
> +       #define OPENSSL_sk_pop_free(x,y) sk_GENERAL_NAME_pop_free((x),(y))
> +#endif
> +
>  __attribute__((format (printf, 1, 2)))
>  static void imap_info(const char *, ...);
>  __attribute__((format (printf, 1, 2)))
> @@ -275,21 +284,30 @@ static int verify_hostname(X509 *cert, const
> char *hostname)
>
>  static int ssl_socket_connect(struct imap_socket *sock, int
> use_tls_only, int verify)
>  {
> -#if (OPENSSL_VERSION_NUMBER >= 0x10000000L)
> -       const SSL_METHOD *meth;
> -#else
> -       SSL_METHOD *meth;
> -#endif
> -       SSL_CTX *ctx;
> -       int ret;
> -       X509 *cert;
> -
> -       SSL_library_init();
> -       SSL_load_error_strings();
> +       #if (OPENSSL_VERSION_NUMBER >= 0x10000000L)
> +               const SSL_METHOD *meth;
> +       #else
> +               SSL_METHOD *meth;
> +       #endif
> +               SSL_CTX *ctx;
> +               int ret;
> +               X509 *cert;
> +
> +       #if OPENSSL_VERSION_NUMBER >= 0x10100000L ||
> defined(LIBRESSL_VERSION_NUMBER)
> +               OPENSSL_init_ssl(0, NULL);
> +               meth = TLS_method();
> +       #else
> +               SSL_library_init();
> +               SSL_load_error_strings();
> +               meth = SSLv23_method();
> +       #endif
>
> -       meth = SSLv23_method();
>         if (!meth) {
> -               ssl_socket_perror("SSLv23_method");
> +       #if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
> +                       ssl_socket_perror("TLS_method");
> +       #else
> +                       ssl_socket_perror("SSLv23_method");
> +       #endif
>                 return -1;
>         }
>
>
> ---

That diff looks pretty okay to me.

> Also, on a different note: I'm actually really interested in applying
> to Git for GSoC, and I should be doing Git microprojects right now to
> properly cement my chance of doing GSoC with Git. Many aspiring GSoC
> applicants already been asking, enquiring and maybe even working about
> Git microprojects, as evident from the mailing list.

For the record, I am not even sure whether Git will participate in GSoC
this year; I am not aware of any activity in that direction.

Having said that, the purpose of a Git microproject is to get acquainted
with the development process of Git (at least in my mind). It is not so
much fixing some issue in Git, but more like learning how to interact with
the Git mailing list, in particular how to communicate effectively with
the developers/reviewers on this list.

In that light, if I were a possible mentor (which I am not, at least not
in this year's GSoC) I would not insist on a microproject. Or more like: I
would accept your work on getting this vexing OpenSSL v1.1.1 issue sorted
out as a microproject in its own right.

> So while I'm not saying that I'm in deep trouble and all this OpenSSL
> v1.1.1 issue fixing is completely useless (I'm learning quite a lot
> along the way and able to understand the project structure), but
> saying that I'm not worried about my GSoC prospects of working in this
> organization would honestly be false :) . I love git, I would love
> contributing to Git, but I'd love to do a GSoC Summer with Git much
> more than the rest.
>
> Please let me know where am I going wrong. If there's any other system
> packages that I can download so that I can focus on other Git issues
> and this one simultaneously, please let me know. Here are my system
> specifications (let me know if you need anything more specific):
> ---
> OS: Ubuntu 18.04
> Linux Kernel: 5.3
> OpenSSL Version: 1.1.1d
> ---
>
> Apologies for the long email,

This is not even close to being the longest email sent to this list, so
don't worry!

Ciao,
Johannes

> Thank You,
> Nirmal Khedkar
> (https://nirmalhk7.github.io)
>
>
> On Thu, Jan 23, 2020 at 12:50 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > >> From my limited knowledge of OpenSSL libraries, I think the error has
> > >> more to do with 'SSL_library_init()' , which appears like a
> > >> constructor to the OpenSSL library. I found these emails regarding
> > >> "if" cases around this function. Please check out these patches:
> > >> 1. Rosen Penev:
> > >> https://lore.kernel.org/git/20181227023548.396-1-rosenp@gmail.com/
> > >
> > > I remember that one. And I agreed with Junio that the documentation
> > > suggests that the call is _optional_, while the patch suggests that it
> > > would be _incorrect_ instead.
> > >
> > > And looking at
> > > https://www.openssl.org/docs/man1.1.1/man3/SSL_library_init.html suggests
> > > to me that it is still supported.
> > >
> > > Having said that, if I look at the headers installed for `libssl-dev`
> > > version `1.1.1-1ubuntu2.1~18.04.5` in my Ubuntu installation, I see that
> > > `/usr/include/openssl/ssl.h` defines that symbol as:
> > >
> > >       #  define SSL_library_init() OPENSSL_init_ssl(0, NULL)
> > >
> > > but _only_:
> > >
> > >       # if OPENSSL_API_COMPAT < 0x10100000L
> > >
> > > So maybe that disagrees with the documentation that says that
> > > SSL_library_init() is optional?
> > >
> > > The curious thing is that `OPENSSL_API_COMPAT` is not even defined
> > > anywhere. So maybe it _is_ the right thing to also `#define
> > > SSL_library_init() (void)` in the diff you listed above?
> > >
> > > _Maybe_ guarded within `#ifndef SSL_library_init ... #endif` guards?
> > >
> > >> 2. eroen: https://lore.kernel.org/git/20170112104219.563497-1-git-scm@occam.eroen.eu/
> > >
> > > That sounds like a good suggestion, too.
> > >
> > >> Are the fixes made in these patches relevant here. Please let me know
> > >> if I'm going wrong.
> > >
> > > Yes, both threads are relevant, and if you can reconcile them into a patch
> > > that makes Git compile with OpenSSL v1.1.1, I will try my best to review
> > > them (Cc: me, just in case).
> >
> > I agree with the above reasoning and the suggestion given by Bates in
> > https://lore.kernel.org/git/66967e0e-8bd9-f4b6-d2d4-ccce9004f42e@nottheoilrig.com/
> > sounds like a reasonable one.
> >
> > Thanks for digging and double-checking these two previous efforts,
> > and giving another round of thoughts on them.
> >
> >
> >
> >
>