libressl: static declaration of 'OpenSSL_version_num' #2319

jungle-boogie · Feb 19, 2018

Hello,

Curl is failing to build on openBSD -current:

/home/jungle/bin/curl/lib/vtls/openssl.c:121:22: error: static declaration of 'OpenSSL_version_num' follows non-static declaration                                                     
static unsigned long OpenSSL_version_num(void)                                                                                                                                       
                     ^                                                                                                                                                               
/usr/include/openssl/crypto.h:340:15: note: previous declaration is here                                                                                                             
unsigned long OpenSSL_version_num(void);                                                                                                                                             
              ^                                                                                                                                                                      
1 error generated.                                                                                                                                                                   
*** Error 1 in . (lib/CMakeFiles/libcurl.dir/build.make:2943 'lib/CMakeFiles/libcurl.dir/vtls/openssl.c.o': cd /home/jungle/bin/curl/build/lib...)                                     
*** Error 1 in . (CMakeFiles/Makefile2:1014 'lib/CMakeFiles/libcurl.dir/all')                                                                                                        
*** Error 1 in /home/jungle/bin/curl/build (Makefile:141 'all')

See this openBSD commit to lib/libcrypto/crypto.h:
openbsd/src@3a94b19#diff-f5754718dcf4bb001cacd3a8e689bb01

Simply editing this file:
https://github.com/curl/curl/blob/master/lib/vtls/openssl.c#L121

and removing Static at the beginning of the line causes the build to successfully build.

Do you know the first version of libressl where this is fixed? I think it would be better to change it to #if (LIBRESSL_VERSION_NUMBER < fixedver)

jay · Feb 19, 2018

Do you know the first version of libressl where this is fixed? I think it would be better to change it to #if (LIBRESSL_VERSION_NUMBER < fixedver)

Hi @jay,

I don't know the first version. If you take a look at the openBSD commit I linked, it was only modified 5 days ago. My openBSD -current machine has libressl 2.7.0, if that helps.

jungle-boogie · Feb 19, 2018

Hi @jay,

I don't know the first version. If you take a look at the openBSD commit I linked, it was only modified 5 days ago. My openBSD -current machine has libressl 2.7.0, if that helps.

i see, it's the dev branch not release. It looks like they're transitioning from libressl 2.6.x to 3.0.x but it's unclear to me whether they'll continue with a 2.6.5 in parallel and whether it will have the api.

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 2a6b3cf..f8b3fa8 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -117,7 +117,8 @@
 #define X509_get0_notBefore(x) X509_get_notBefore(x)
 #define X509_get0_notAfter(x) X509_get_notAfter(x)
 #define CONST_EXTS /* nope */
-#ifdef LIBRESSL_VERSION_NUMBER
+#if defined(LIBRESSL_VERSION_NUMBER) && \
+  (LIBRESSL_VERSION_NUMBER <= 0x2060400fL)
 static unsigned long OpenSSL_version_num(void)
 {
   return LIBRESSL_VERSION_NUMBER;

jay · Feb 19, 2018

i see, it's the dev branch not release. It looks like they're transitioning from libressl 2.6.x to 3.0.x but it's unclear to me whether they'll continue with a 2.6.5 in parallel and whether it will have the api.

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 2a6b3cf..f8b3fa8 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -117,7 +117,8 @@
 #define X509_get0_notBefore(x) X509_get_notBefore(x)
 #define X509_get0_notAfter(x) X509_get_notAfter(x)
 #define CONST_EXTS /* nope */
-#ifdef LIBRESSL_VERSION_NUMBER
+#if defined(LIBRESSL_VERSION_NUMBER) && \
+  (LIBRESSL_VERSION_NUMBER <= 0x2060400fL)
 static unsigned long OpenSSL_version_num(void)
 {
   return LIBRESSL_VERSION_NUMBER;

I think we should either get some confirmation of this from the libressl team, or wait for an actual release to see how this will end up. Otherwise I think the fix looks correct.

bagder · Feb 19, 2018

I think we should either get some confirmation of this from the libressl team, or wait for an actual release to see how this will end up. Otherwise I think the fix looks correct.

I think we should either get some confirmation of this from the libressl team

@4a6f656c

jay · Feb 19, 2018

I think we should either get some confirmation of this from the libressl team

@4a6f656c

0x2070000fL would be appropriate if checking for a version, however the only place cURL uses this is to print LibreSSL/$version, so decoding based on LIBRESSL_VERSION_NUMBER is more appropriate here. This is what we're currently doing in ports:

Index: lib/vtls/openssl.c
--- lib/vtls/openssl.c.orig
+++ lib/vtls/openssl.c
@@ -117,12 +117,7 @@
 #define X509_get0_notBefore(x) X509_get_notBefore(x)
 #define X509_get0_notAfter(x) X509_get_notAfter(x)
 #define CONST_EXTS /* nope */
-#ifdef LIBRESSL_VERSION_NUMBER
-static unsigned long OpenSSL_version_num(void)
-{
-  return LIBRESSL_VERSION_NUMBER;
-}
-#else
+#ifndef LIBRESSL_VERSION_NUMBER
 #define OpenSSL_version_num() SSLeay()
 #endif
 #endif
@@ -3527,7 +3522,11 @@ static size_t Curl_ossl_version(char *buffer, size_t s
   unsigned long ssleay_value;
   sub[2]='\0';
   sub[1]='\0';
+#ifdef LIBRESSL_VERSION_NUMBER
+  ssleay_value = LIBRESSL_VERSION_NUMBER;
+#else
   ssleay_value = OpenSSL_version_num();
+#endif
   if(ssleay_value < 0x906000) {
     ssleay_value = SSLEAY_VERSION_NUMBER;
     sub[0]='\0';

Results with this on OpenBSD -current:

$ curl -V
curl 7.58.0 (x86_64-unknown-openbsd6.2) libcurl/7.58.0 LibreSSL/2.7.0 zlib/1.2.3 nghttp2/1.30.0
Release-Date: 2018-01-24
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IPv6 Largefile NTLM NTLM_WB SSL libz HTTP2 UnixSockets HTTPS-proxy

sthen · Feb 19, 2018

0x2070000fL would be appropriate if checking for a version, however the only place cURL uses this is to print LibreSSL/$version, so decoding based on LIBRESSL_VERSION_NUMBER is more appropriate here. This is what we're currently doing in ports:

Index: lib/vtls/openssl.c
--- lib/vtls/openssl.c.orig
+++ lib/vtls/openssl.c
@@ -117,12 +117,7 @@
 #define X509_get0_notBefore(x) X509_get_notBefore(x)
 #define X509_get0_notAfter(x) X509_get_notAfter(x)
 #define CONST_EXTS /* nope */
-#ifdef LIBRESSL_VERSION_NUMBER
-static unsigned long OpenSSL_version_num(void)
-{
-  return LIBRESSL_VERSION_NUMBER;
-}
-#else
+#ifndef LIBRESSL_VERSION_NUMBER
 #define OpenSSL_version_num() SSLeay()
 #endif
 #endif
@@ -3527,7 +3522,11 @@ static size_t Curl_ossl_version(char *buffer, size_t s
   unsigned long ssleay_value;
   sub[2]='\0';
   sub[1]='\0';
+#ifdef LIBRESSL_VERSION_NUMBER
+  ssleay_value = LIBRESSL_VERSION_NUMBER;
+#else
   ssleay_value = OpenSSL_version_num();
+#endif
   if(ssleay_value < 0x906000) {
     ssleay_value = SSLEAY_VERSION_NUMBER;
     sub[0]='\0';

Results with this on OpenBSD -current:

$ curl -V
curl 7.58.0 (x86_64-unknown-openbsd6.2) libcurl/7.58.0 LibreSSL/2.7.0 zlib/1.2.3 nghttp2/1.30.0
Release-Date: 2018-01-24
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IPv6 Largefile NTLM NTLM_WB SSL libz HTTP2 UnixSockets HTTPS-proxy

@jay - the additional API will appear in the 2.7.x release. There will only be a 2.6.5 release if there are security or bug fixes and such a release would not contain the API changes. As sthen notes, conditioning on 0x2070000fL is the correct version to use, if you want to take that route. For now, I'd recommend holding off until we finish the API churn, as there may be other fixes/changes.

4a6f656c · Feb 19, 2018

@jay - the additional API will appear in the 2.7.x release. There will only be a 2.6.5 release if there are security or bug fixes and such a release would not contain the API changes. As sthen notes, conditioning on 0x2070000fL is the correct version to use, if you want to take that route. For now, I'd recommend holding off until we finish the API churn, as there may be other fixes/changes.

thanks @4a6f656c, waiting a bit to see how this settles for real in a coming release seems like the sensible thing to do.

bagder · Feb 20, 2018

thanks @4a6f656c, waiting a bit to see how this settles for real in a coming release seems like the sensible thing to do.

libressl 2.7.0 has been released so it makes sense to revisit this issue now...

bagder · Mar 22, 2018

libressl 2.7.0 has been released so it makes sense to revisit this issue now...

@sthen 's fix is probably the most correct. Internally LibreSSL will print out its OPENSSL_VERSION_NUMBER which will always be 0x20000000L so doing a patch like the following would not get the desired behavior.

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 2a6b3cfac..78a98b0ca 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -118,10 +118,13 @@
 #define X509_get0_notAfter(x) X509_get_notAfter(x)
 #define CONST_EXTS /* nope */
 #ifdef LIBRESSL_VERSION_NUMBER
+/* For LibreSSL before 2.7.0 */
+#if (LIBRESSL_VERSION_NUMBER < 0x2070000fL)
 static unsigned long OpenSSL_version_num(void)
 {
   return LIBRESSL_VERSION_NUMBER;
 }
+#endif
 #else
 #define OpenSSL_version_num() SSLeay()
 #endif

donny-dont · Mar 23, 2018

@sthen 's fix is probably the most correct. Internally LibreSSL will print out its OPENSSL_VERSION_NUMBER which will always be 0x20000000L so doing a patch like the following would not get the desired behavior.

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 2a6b3cfac..78a98b0ca 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -118,10 +118,13 @@
 #define X509_get0_notAfter(x) X509_get_notAfter(x)
 #define CONST_EXTS /* nope */
 #ifdef LIBRESSL_VERSION_NUMBER
+/* For LibreSSL before 2.7.0 */
+#if (LIBRESSL_VERSION_NUMBER < 0x2070000fL)
 static unsigned long OpenSSL_version_num(void)
 {
   return LIBRESSL_VERSION_NUMBER;
 }
+#endif
 #else
 #define OpenSSL_version_num() SSLeay()
 #endif

@sthen 's fix is probably the most correct. Internally LibreSSL will print out its OPENSSL_VERSION_NUMBER which will always be 0x20000000L so doing a patch like the following would not get the desired behavior.

Thank you for that information. Is there any way we can get LIBRESSL_VERSION_NUMBER dynamically at runtime. I am thinking of what happens when, for example, a user's shared LibreSSL is upgraded. How would we get the actual version number to report to the user, is it possible through the API?

jay · Mar 23, 2018

@sthen 's fix is probably the most correct. Internally LibreSSL will print out its OPENSSL_VERSION_NUMBER which will always be 0x20000000L so doing a patch like the following would not get the desired behavior.

Thank you for that information. Is there any way we can get LIBRESSL_VERSION_NUMBER dynamically at runtime. I am thinking of what happens when, for example, a user's shared LibreSSL is upgraded. How would we get the actual version number to report to the user, is it possible through the API?

There's OpenSSL_version

const char *
OpenSSL_version(int t)
{
	switch (t) {
	case OPENSSL_VERSION:
		return OPENSSL_VERSION_TEXT;
	case OPENSSL_BUILT_ON:
		return("built on: date not available");
	case OPENSSL_CFLAGS:
		return("compiler: information not available");
	case OPENSSL_PLATFORM:
		return("platform: information not available");
	case OPENSSL_DIR:
		return "OPENSSLDIR: \"" OPENSSLDIR "\"";
	case OPENSSL_ENGINES_DIR:
		return "ENGINESDIR: N/A";
	}
	return("not available");
}

/* These will change with each release of LibreSSL-portable */
#define LIBRESSL_VERSION_NUMBER	0x2070000fL
#define LIBRESSL_VERSION_TEXT	"LibreSSL 2.7.0"

/* These will never change */
#define OPENSSL_VERSION_NUMBER	0x20000000L
#define OPENSSL_VERSION_TEXT	LIBRESSL_VERSION_TEXT
#define OPENSSL_VERSION_PTEXT	" part of " OPENSSL_VERSION_TEXT

One of the LibreSSL guys might want to chime in here on the best route. Just commenting because I recently hit this issue and am working around it currently.

donny-dont · Mar 23, 2018

There's OpenSSL_version

const char *
OpenSSL_version(int t)
{
	switch (t) {
	case OPENSSL_VERSION:
		return OPENSSL_VERSION_TEXT;
	case OPENSSL_BUILT_ON:
		return("built on: date not available");
	case OPENSSL_CFLAGS:
		return("compiler: information not available");
	case OPENSSL_PLATFORM:
		return("platform: information not available");
	case OPENSSL_DIR:
		return "OPENSSLDIR: \"" OPENSSLDIR "\"";
	case OPENSSL_ENGINES_DIR:
		return "ENGINESDIR: N/A";
	}
	return("not available");
}

/* These will change with each release of LibreSSL-portable */
#define LIBRESSL_VERSION_NUMBER	0x2070000fL
#define LIBRESSL_VERSION_TEXT	"LibreSSL 2.7.0"

/* These will never change */
#define OPENSSL_VERSION_NUMBER	0x20000000L
#define OPENSSL_VERSION_TEXT	LIBRESSL_VERSION_TEXT
#define OPENSSL_VERSION_PTEXT	" part of " OPENSSL_VERSION_TEXT

One of the LibreSSL guys might want to chime in here on the best route. Just commenting because I recently hit this issue and am working around it currently.

ok how about we parse from that

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 2a6b3cf..74b78f5 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -3520,7 +3520,22 @@ static ssize_t ossl_recv(struct connectdata *conn, /* connection data */
 static size_t Curl_ossl_version(char *buffer, size_t size)
 {
 #ifdef OPENSSL_IS_BORINGSSL
-  return snprintf(buffer, size, OSSL_PACKAGE);
+  return snprintf(buffer, size, "%s", OSSL_PACKAGE);
+#elif LIBRESSL_VERSION_NUMBER
+  const char *ver;
+  ver = OpenSSL_version(OPENSSL_VERSION);
+  if(ver && !strncmp(ver, "LibreSSL ", 9) && ver[9]) {
+    char *partial = strdup(&ver[9]); /* eg "2.7.0 beta" */
+    if(partial) {
+      char *p;
+      for(p = partial; *p; ++p) {
+        if(ISSPACE(*p))
+          *p = '_';
+      }
+      return snprintf(buffer, size, "%s/%s", OSSL_PACKAGE, partial);
+    }
+  }
+  return snprintf(buffer, size, "%s", OSSL_PACKAGE);
 #else /* OPENSSL_IS_BORINGSSL */
   char sub[3];
   unsigned long ssleay_value;

put aside for the moment not checking snprintf in band error

jay · Mar 23, 2018

ok how about we parse from that

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 2a6b3cf..74b78f5 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -3520,7 +3520,22 @@ static ssize_t ossl_recv(struct connectdata *conn, /* connection data */
 static size_t Curl_ossl_version(char *buffer, size_t size)
 {
 #ifdef OPENSSL_IS_BORINGSSL
-  return snprintf(buffer, size, OSSL_PACKAGE);
+  return snprintf(buffer, size, "%s", OSSL_PACKAGE);
+#elif LIBRESSL_VERSION_NUMBER
+  const char *ver;
+  ver = OpenSSL_version(OPENSSL_VERSION);
+  if(ver && !strncmp(ver, "LibreSSL ", 9) && ver[9]) {
+    char *partial = strdup(&ver[9]); /* eg "2.7.0 beta" */
+    if(partial) {
+      char *p;
+      for(p = partial; *p; ++p) {
+        if(ISSPACE(*p))
+          *p = '_';
+      }
+      return snprintf(buffer, size, "%s/%s", OSSL_PACKAGE, partial);
+    }
+  }
+  return snprintf(buffer, size, "%s", OSSL_PACKAGE);
 #else /* OPENSSL_IS_BORINGSSL */
   char sub[3];
   unsigned long ssleay_value;

put aside for the moment not checking snprintf in band error

please test the pr at #2425 if you use libressl

jay · Mar 24, 2018

please test the pr at #2425 if you use libressl

Hi,

Thanks for fixing this and sorry I didn't get a chance to specifically test the PR. However, now that it's merged into master, I can confirm the curl built successfully on openbsd -current from today's build with libressl 2.7.2.

$ curl --version
curl 7.60.0-DEV (OpenBSD) libcurl/7.60.0-DEV LibreSSL/2.7.2 zlib/1.2.3
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Largefile NTLM SSL libz UnixSockets HTTPS-proxy

Thank you!

jungle-boogie · Apr 4, 2018

Hi,

Thanks for fixing this and sorry I didn't get a chance to specifically test the PR. However, now that it's merged into master, I can confirm the curl built successfully on openbsd -current from today's build with libressl 2.7.2.

$ curl --version
curl 7.60.0-DEV (OpenBSD) libcurl/7.60.0-DEV LibreSSL/2.7.2 zlib/1.2.3
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Largefile NTLM SSL libz UnixSockets HTTPS-proxy

Thank you!

jay added the build label Feb 19, 2018

jay added the on-hold label Feb 21, 2018

bagder changed the title from openBSD: static declaration of 'OpenSSL_version_num' to libressl: static declaration of 'OpenSSL_version_num' Mar 5, 2018

bagder added SSL/TLS and removed on-hold labels Mar 22, 2018

jay referenced this issue Mar 24, 2018
Closed
openssl: retrieve reported libressl version at runtime #2425

bagder referenced this issue Mar 30, 2018
Closed
build curl 7.59.0 against libressl 2.7.1 fail #2439

bagder closed this in 7c90c93 Apr 4, 2018

janstary referenced this issue Apr 27, 2018
Merged
libressl-devel: upgrade to 2.7.2 #1671
8 of 9 tasks complete

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

curl/curl

Join GitHub today

libressl: static declaration of 'OpenSSL_version_num' #2319

Comments

jungle-boogie commented Feb 19, 2018

This comment has been minimized.

jay commented Feb 19, 2018

jay added the build label Feb 19, 2018

This comment has been minimized.

jungle-boogie commented Feb 19, 2018

This comment has been minimized.

jay commented Feb 19, 2018

This comment has been minimized.

bagder commented Feb 19, 2018

This comment has been minimized.

jay commented Feb 19, 2018

This comment has been minimized.

sthen commented Feb 19, 2018 • edited Edited 1 time sthen edited Feb 19, 2018 (most recent)

This comment has been minimized.

4a6f656c commented Feb 19, 2018

This comment has been minimized.

bagder commented Feb 20, 2018

jay added the on-hold label Feb 21, 2018

bagder changed the title from openBSD: static declaration of 'OpenSSL_version_num' to libressl: static declaration of 'OpenSSL_version_num' Mar 5, 2018

This comment has been minimized.

bagder commented Mar 22, 2018

bagder added SSL/TLS and removed on-hold labels Mar 22, 2018

This comment has been minimized.

donny-dont commented Mar 23, 2018 • edited Edited 1 time donny-dont edited Mar 23, 2018 (most recent)

This comment has been minimized.

jay commented Mar 23, 2018

This comment has been minimized.

donny-dont commented Mar 23, 2018

This comment has been minimized.

jay commented Mar 23, 2018 • edited Edited 2 times jay edited Mar 23, 2018 (most recent) jay edited Mar 23, 2018

jay added a commit to jay/curl that referenced this issue Mar 24, 2018

jay referenced this issue Mar 24, 2018

This comment has been minimized.

jay commented Mar 24, 2018

jay added a commit to jay/curl that referenced this issue Mar 24, 2018

bagder referenced this issue Mar 30, 2018

bagder closed this in 7c90c93 Apr 4, 2018

This comment has been minimized.

jungle-boogie commented Apr 4, 2018

janstary referenced this issue Apr 27, 2018

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

sthen commented Feb 19, 2018 •

edited

Edited 1 time

sthen edited Feb 19, 2018 (most recent)

donny-dont commented Mar 23, 2018 •

edited

Edited 1 time

donny-dont edited Mar 23, 2018 (most recent)

jay commented Mar 23, 2018 •

edited

Edited 2 times

jay edited Mar 23, 2018 (most recent)

jay edited Mar 23, 2018

bagder closed this in `7c90c93` Apr 4, 2018