Re: [PATCH] imap-send: link against libcrypto for HMAC and others

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 6, 2010, 06:06 UTC
Message-ID: <20101206060628.GA30188@burratino>
In-Reply-To: <1291613978.4756.157.camel@yamato.local>

Diego Elio Pettenò wrote:

> Il giorno lun, 06/12/2010 alle 00.28 -0500, Anders Kaseorg ha scritto:

Show 6 quoted lines

>> Also, the Makefile already has a NEEDS_CRYPTO_WITH_SSL flag that’s
>> automatically set on Darwin, Windows, and MinGW.  We shouldn’t have two
>> mechanisms for addressing the same problem; maybe we just need to enable
>> the existing flag on more (or all) platforms?
>
> No, these should be different issues

Are you sure? The change description for v1.6.5-rc1~23 (Makefile: Add NEEDS_CRYPTO_WITH_SSL, 2009-10-08) says:

    The Makefile comment for NEEDS_SSL_WITH_CRYPTO says to define it "if
    you need -lcrypto with -lssl (Darwin)."  However, what it actually
    does is add -lssl when you use -lcrypto and not the other way around.
    However, libcrypto contains a majority of the ERR_* functions from
    OpenSSL (at least on OS X) so we need it both ways.

and grepping that version reveals the same hit I think your patch deals with:

 $ git grep -F -e ERR_ v1.6.5-rc1~23
 v1.6.5-rc1~23:imap-send.c:      fprintf(stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), NULL));

I am guessing the reality is closer to:

    Previously we relied on -lssl pulling in libcrypto on most
    platforms.  That has at least three problems:

     (1) it is not resiliant against future changes in libssl.
         The DT_NEEDED entries in a library are generally
         considered an implementation detail that is allowed to
         change.

     (2) it does not work on all platforms; for example, ld on
         Mac OS X and ld --no-copy-dt-needed-entries on Linux
         do not permit that trick.

     (3) it is tricky.  A simple rule is "explicitly link to
         all libraries whos symbols you use directly".

    To fix this, eliminate the NEEDS_CRYPTO_WITH_SSL knob and
    always treat it as true.  NEEDS_SSL_WITH_CRYPTO can stay
    because there really are differences between platforms
    for that one.

Warning: I could be totally wrong. :)  I have not verified any
of these claims.