Fix build with LibreSSL 2.7 by Sp1l · Pull Request #2447 · curl/curl

Sp1l · 2018-04-02T17:09:11Z

LibreSSL 2.7 implements (most of) OpenSSL 1.1 API

See also: https://bugs.freebsd.org/226845
Signed-off-by: Bernard Spil brnrd@FreeBSD.org

- LibreSSL 2.7 implements (most of) OpenSSL 1.1 API Signed-off-by: Bernard Spil <brnrd@FreeBSD.org>

bagder · 2018-04-02T20:59:17Z

The macOS libressl build fails. Presumably because of it being an older verison(?)

vtls/openssl.c:2805:35: error: passing 'const X509_ALGOR *' (aka 'const struct X509_algor_st *') to parameter of type 'X509_ALGOR *' (aka 'struct X509_algor_st *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
        X509_signature_print(mem, palg, a);
                                  ^~~~
/usr/local/Cellar/libressl/2.7.2/include/openssl/x509.h:670:46: note: passing argument to parameter 'alg' here
int X509_signature_print(BIO *bp,X509_ALGOR *alg, ASN1_STRING *sig);
                                             ^
1 error generated.

Sp1l · 2018-04-04T08:05:43Z

There's many of these differences between 1.0 and 1.1 API where they were regular and are now const. Could it be one of the build flags that you pass in the MacOS build? I get these with clang builds as well but they're not fatal.

bagder · 2018-04-04T08:10:56Z

Yes that's it. We're using very picky compiler warnings and -werror for most builds, including the libressl build.

I would prefer to fix the warnings rather than to let them through since letting warnings through tend to make us not notice when "legitimate" warnings are introduced down the line.

bagder · 2018-04-04T08:58:16Z

Perhaps like this:

From 59d4a6ee30f9a0fd813833bdae88fbcf06eaaea4 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Wed, 4 Apr 2018 10:55:56 +0200
Subject: [PATCH] openssl: provide defines for argument typecasts to build
 warning-free

... as OpenSSL >= 1.1.0 and libressl >= 2.7.0 use different argument types.
---
 lib/vtls/openssl.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index bbb8ec766..92eb952ad 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -110,10 +110,20 @@
 #define HAVE_X509_GET0_EXTENSIONS 1 /* added in 1.1.0 -pre1 */
 #define HAVE_OPAQUE_EVP_PKEY 1 /* since 1.1.0 -pre3 */
 #define HAVE_OPAQUE_RSA_DSA_DH 1 /* since 1.1.0 -pre5 */
 #define CONST_EXTS const
 #define HAVE_ERR_REMOVE_THREAD_STATE_DEPRECATED 1
+
+/* funny typecast defines due to differences in APIs */
+#ifdef LIBRESSL_VERSION_NUMBER
+#define ARG2_X509_signature_print (X509_ALGOR *)
+#define ARG2_X509_get0_signature (const X509_ALGOR **)
+#else
+#define ARG2_X509_signature_print
+#define ARG2_X509_get0_signature
+#endif
+
 #else
 /* For OpenSSL before 1.1.0 */
 #define ASN1_STRING_get0_data(x) ASN1_STRING_data(x)
 #define X509_get0_notBefore(x) X509_get_notBefore(x)
 #define X509_get0_notAfter(x) X509_get_notAfter(x)
@@ -2799,12 +2809,12 @@ static CURLcode get_cert_chain(struct connectdata *conn,
 #if defined(HAVE_X509_GET0_SIGNATURE) && defined(HAVE_X509_GET0_EXTENSIONS)
     {
       const X509_ALGOR *palg = NULL;
       ASN1_STRING *a = ASN1_STRING_new();
       if(a) {
-        X509_get0_signature(&psig, &palg, x);
-        X509_signature_print(mem, palg, a);
+        X509_get0_signature(&psig, ARG2_X509_get0_signature &palg, x);
+        X509_signature_print(mem, ARG2_X509_signature_print palg, a);
         ASN1_STRING_free(a);
 
         if(palg) {
           i2a_ASN1_OBJECT(mem, palg->algorithm);
           push_certinfo("Public Key Algorithm", i);
-- 
2.16.3

bagder · 2018-04-04T09:04:55Z

@Sp1l if you give me a 👍 on that, I'll merge ...

bagder · 2018-04-04T09:17:55Z

The libressl API is (inconsistently) requiring one of the arguments to be const and the other to not be const, which made me opt for the typecasts and not change the variable declaration done just above the two function calls.

Sp1l · 2018-04-04T10:03:28Z

I think this is fixable, but may require changes in other parts of the code. Let me check and get back on this...

Sp1l · 2018-04-04T10:30:35Z

I have seen an example somewhere that does something like

#if LIBRESSL_VERSION_NUMBER >= 0x20700000L
#define APIISCONST const
#else 
#define APIISCONST const
#endif

Then your code would be something like

@@ -2799,12 +2809,12 @@ static CURLcode get_cert_chain(struct connectdata *conn,
 #if defined(HAVE_X509_GET0_SIGNATURE) && defined(HAVE_X509_GET0_EXTENSIONS)
     {
       const X509_ALGOR *palg = NULL;
       ASN1_STRING *a = ASN1_STRING_new();
       if(a) {
-        X509_get0_signature(&psig, &palg, x);
-        X509_signature_print(mem, palg, a);
+        X509_get0_signature(&psig, APIISCONST &palg, x);
+        X509_signature_print(mem, APIISCONST palg, a);
         ASN1_STRING_free(a);
 
         if(palg) {
           i2a_ASN1_OBJECT(mem, palg->algorithm);
           push_certinfo("Public Key Algorithm", i);

and reusable anywhere for other methods.

Sp1l · 2018-04-04T10:33:57Z

Looks like OpenSSL 1.1.1 is going down the same path?
openssl/openssl@8adc1cb

bagder · 2018-04-04T11:23:51Z

The problem is X509_signature_print, which in the libressl version doesn't take a const argument but it does in OpenSSL. (the second define I added turned out to not be necessary)

bagder · 2018-04-04T11:28:26Z

I merged this now since I want to get the CI builds green again. If we want to further polish the libressl conditions, that's fine too.

Sp1l · 2018-04-04T15:39:31Z

Thanks bagder!
Haven't you run into the same issue with OpenSSL 1.1.1-pre4?
https://github.com/openssl/openssl/blob/master/include/openssl/x509.h#L364 also defines

int X509_signature_print(BIO *bp, const X509_ALGOR *alg,
const ASN1_STRING *sig);

I'll keep my eyes peeled for where I've found other projects solving this.

bagder · 2018-04-05T06:29:12Z

Things that are not the same. Contents from include/openssl/x509.h in three projects:

libressl 2.7.2 (dev master)

int X509_signature_print(BIO *bp,X509_ALGOR *alg, ASN1_STRING *sig);

openssl 1.1.1-pre (dev master)

int X509_signature_print(BIO *bp, const X509_ALGOR *alg, const ASN1_STRING *sig);

boringssl (dev master)

int X509_signature_print(BIO *bp, const X509_ALGOR *alg, const ASN1_STRING *sig);

Fix build with LibreSSL 2.7

1385ff2

- LibreSSL 2.7 implements (most of) OpenSSL 1.1 API Signed-off-by: Bernard Spil <brnrd@FreeBSD.org>

bagder added build TLS labels Apr 2, 2018

This was referenced Apr 2, 2018

openssl: fix build with libressl 2.7 #2448

Closed

Revert "openssl: Don't add verify locations when verifypeer==0" #2451

Closed

bagder closed this in 7c90c93 Apr 4, 2018

lock bot locked as resolved and limited conversation to collaborators Jul 4, 2018

Uh oh!

Conversation

Sp1l commented Apr 2, 2018

Uh oh!

bagder commented Apr 2, 2018

Uh oh!

Sp1l commented Apr 4, 2018

Uh oh!

bagder commented Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bagder commented Apr 4, 2018

Uh oh!

bagder commented Apr 4, 2018

Uh oh!

bagder commented Apr 4, 2018

Uh oh!

Sp1l commented Apr 4, 2018

Uh oh!

Sp1l commented Apr 4, 2018

Uh oh!

Sp1l commented Apr 4, 2018

Uh oh!

bagder commented Apr 4, 2018

Uh oh!

bagder commented Apr 4, 2018

Uh oh!

Sp1l commented Apr 4, 2018

Uh oh!

bagder commented Apr 5, 2018

libressl 2.7.2 (dev master)

openssl 1.1.1-pre (dev master)

boringssl (dev master)

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

bagder commented Apr 4, 2018 •

edited

Loading