Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virtuoso-opensource: build against openssl 1.1.0 #583

Closed
wants to merge 1 commit into from
Closed

virtuoso-opensource: build against openssl 1.1.0 #583

wants to merge 1 commit into from

Conversation

sebastianas
Copy link

Patch has been created against 6.1.6 to address
https://bugs.debian.org/828594
and then forwarded ported.

Signed-off-by: Sebastian Andrzej Siewior sebastian@breakpoint.cc

@sebastianas
Copy link
Author

ping

@openlink
Copy link
Owner

Thank you for your contribution.

Development is looking into this, as we need to make sure that this patch for OpenSSL 1.1.0 does not affect portability of the code using older versions of OpenSSL.

@sebastianas
Copy link
Author

ping

Patch has been created against 6.1.6 to address
	https://bugs.debian.org/828594
and then forwarded ported.

Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
@sebastianas
Copy link
Author

refreshed against current develop/7 branch

@jakubklimek
Copy link

Actually, I tried but canno build this on Debian 9 and OpenSSL 1.1.0f:

http.c: In function ‘bif_https_renegotiate’:
http.c:9898:10: error: dereferencing pointer to incomplete type ‘SSL {aka struct ssl_st}’
       ssl->state = SSL_ST_ACCEPT;
          ^~
http.c: In function ‘bif_ftp_log’:
http.c:10822:14: warning: variable ‘day’ set but not used [-Wunused-but-set-variabl ]
   int month, day, year;
              ^~~
http.c: In function ‘ws_serve_client_connection’:
http.c:10893:7: warning: variable ‘rc’ set but not used [-Wunused-but-set-variable]
   int rc = LTE_OK;
       ^~
http.c: In function ‘soap_mime_tree_ctx’:
http.c:11992:12: warning: variable ‘attrs’ set but not used [-Wunused-but-set-variable]
   caddr_t *attrs = NULL, *parts = NULL;
            ^~~~~
Makefile:1582: recipe for target 'libwi_la-http.lo' failed
make[3]: *** [libwi_la-http.lo] Error 1
make[3]: Leaving directory '/home/klimek/virtuoso-opensource/libsrc/Wi'
Makefile:2970: recipe for target 'install' failed
make[2]: *** [install] Error 2
make[2]: Leaving directory '/home/klimek/virtuoso-opensource/libsrc/Wi'
Makefile:477: recipe for target 'install-recursive' failed
make[1]: *** [install-recursive] Error 1
make[1]: Leaving directory '/home/klimek/virtuoso-opensource/libsrc'
Makefile:633: recipe for target 'install-recursive' failed
make: *** [install-recursive] Error 1

@sebastianas
Copy link
Author

sebastianas commented Oct 20, 2017 via email

@jakubklimek
Copy link

@sebastianas I do not have much knowledge of the source code. I just tried compiling it as I am unable to compile Virtuoso normally since I upgraded to OpenSSL 1.1.0 and found out it is still not possible

@sipi
Copy link

sipi commented Apr 2, 2019

No integration of a patch concerning SSL since more than 2 years…
vulnerabilities fixed in 1.1.0
:/

@TallTed
Copy link
Collaborator

TallTed commented Apr 2, 2019

@sipi - Patches to OpenSSL 1.0.2r (the latest 1.0.x to date, against which you should be able to build, as documented (last updated explicitly for 1.0.2p)) have addressed at least most of those vulnerabilities fixed in 1.1.0.

Unfortunately, the OpenSSL pages do not make it easy to clearly see what (if any) vulnerabilities which are listed as addressed in 1.1.x remain in (if they impacted at all) 1.0.2r.

If you have specific information or complaint about such, please provide more detail, so we can appropriately raise priority of the 1.1.0 compatibility effort.

@sipi
Copy link

sipi commented Apr 3, 2019

@TallTed ok, thanks for this answer. It reassures me.
I've no more information about.

kencu added a commit to macports/macports-ports that referenced this pull request Sep 11, 2019
@fxcoudert
Copy link

fxcoudert commented Nov 9, 2019

OpenSSL 1.0 will reach end-of-life on 2019-12-31, i.e. less than two months. At this point, it will not receive even security updates anymore, and will be removed from distributions such as Homebrew.

The latest release of virtuoso is not compatible with OpenSSL 1.1. Unless a new release, compatible with OpenSSL 1.1, is shipped before that date, virtuoso will be removed from Homebrew distribution.

Out of 380 packages that depend on OpenSSL in Homebrew, virtuoso is among the 5 remaining ones that have not migrated to the newer OpenSSL.

@fxcoudert
Copy link

fxcoudert commented Nov 18, 2019

There is need for an additional patch to top-level configure.ac to remove this code:

AC_MSG_CHECKING([OpenSSL version])
AC_TRY_COMPILE([
#include <openssl/opensslv.h>
],[
#ifdef LIBRESSL_VERSION_NUMBER
/* LibreSSL defines OPENSSL_VERSION_NUMBER 0x20000000L but uses a compatible API to OpenSSL v1.0.x */
#elif OPENSSL_VERSION_NUMBER >= 0x1010000fL
#error OpenSSL version too new
#endif
    ],[
      AC_MSG_RESULT([< 1.1.0])
    ],[
      AC_MSG_ERROR([OpenSSL version 1.1.0 or greater is currently not supported.])
    ])

Even with this pull request, and the patch above, compiling against OpenSSL 1.1.0f I get the following error during compilation:

Dkernel.c:5166:8: error: use of undeclared identifier 'SSL_PROTOCOL_TLSV1_3'
        opt = SSL_PROTOCOL_TLSV1_3;
              ^

And later, this error

http.c:9952:10: error: incomplete definition of type 'struct ssl_st'
      ssl->state = SSL_ST_ACCEPT;
      ~~~^

which is fixed by a7d1535


Later, one more issue:

xmlenc.c:2318:23: error: incomplete definition of type 'struct evp_pkey_st'
      if (pkey && pkey->type == EVP_PKEY_RSA)
                  ~~~~^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:93:16: note: forward declaration of 'struct evp_pkey_st'
typedef struct evp_pkey_st EVP_PKEY;
               ^
xmlenc.c:2319:10: error: incomplete definition of type 'struct evp_pkey_st'
        p = pkey->pkey.rsa;
            ~~~~^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:93:16: note: forward declaration of 'struct evp_pkey_st'
typedef struct evp_pkey_st EVP_PKEY;
               ^
xmlenc.c:2322:25: error: incomplete definition of type 'struct evp_pkey_st'
      if (pkkey && pkkey->type == EVP_PKEY_RSA)
                   ~~~~~^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:93:16: note: forward declaration of 'struct evp_pkey_st'
typedef struct evp_pkey_st EVP_PKEY;
               ^
xmlenc.c:2323:11: error: incomplete definition of type 'struct evp_pkey_st'
        r = pkkey->pkey.rsa;
            ~~~~~^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:93:16: note: forward declaration of 'struct evp_pkey_st'
typedef struct evp_pkey_st EVP_PKEY;
               ^
xmlenc.c:7077:22: error: variable has incomplete type 'X509_STORE_CTX' (aka 'struct x509_store_ctx_st')
      X509_STORE_CTX store_ctx;
                     ^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:128:16: note: forward declaration of 'struct x509_store_ctx_st'
typedef struct x509_store_ctx_st X509_STORE_CTX;
               ^
xmlenc.c:7652:19: error: incomplete definition of type 'struct x509_store_st'
  certs = CA_certs->objs;
          ~~~~~~~~^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:127:16: note: forward declaration of 'struct x509_store_st'
typedef struct x509_store_st X509_STORE;
               ^
xmlenc.c:7657:14: error: incomplete definition of type 'struct x509_object_st'
      if (obj->type == X509_LU_X509)
          ~~~^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:130:16: note: forward declaration of 'struct x509_object_st'
typedef struct x509_object_st X509_OBJECT;
               ^
xmlenc.c:7659:17: error: incomplete definition of type 'struct x509_object_st'
          X509 *x = obj->data.x509;
                    ~~~^
/usr/local/opt/openssl@1.1/include/openssl/ossl_typ.h:130:16: note: forward declaration of 'struct x509_object_st'
typedef struct x509_object_st X509_OBJECT;
               ^

Copy link

@Bo98 Bo98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debian has the following experimental patch which they say was forwarded here.

https://sources.debian.org/data/main/v/virtuoso-opensource/7.2.5.1+dfsg-2/debian/patches/ssl1.1.patch

There's differences between that and this pull request however, which I've noted in this informatory review, excluding whitespace differences. Not all are important - I've just noted all differences.

There is also some additional areas that were patched in the Debian patch. These are:

--- a/libsrc/Wi/xmlenc.c
+++ b/libsrc/Wi/xmlenc.c
@@ -2280,12 +2314,12 @@ bif_xenc_key_rsa_read (caddr_t * qst, ca
     {
       in = BIO_new_mem_buf (key_base64, len);
       pkey = d2i_PUBKEY_bio (in, NULL);
-      if (pkey && pkey->type == EVP_PKEY_RSA)
-	p = pkey->pkey.rsa;
+      if (pkey && EVP_PKEY_id(pkey) == EVP_PKEY_RSA)
+	p = EVP_PKEY_get0_RSA(pkey);
       BIO_reset (in);
       pkkey = d2i_PrivateKey_bio (in, NULL);
-      if (pkkey && pkkey->type == EVP_PKEY_RSA)
-	r = pkkey->pkey.rsa;
+      if (pkkey && EVP_PKEY_id(pkkey) == EVP_PKEY_RSA)
+	r = EVP_PKEY_get0_RSA(pkkey);
       BIO_free (in);
     }
   else
@@ -7032,19 +7073,19 @@ bif_xenc_pkcs12_export (caddr_t * qst, c
   if (export_chain)
     {
       int i;
-      X509_STORE_CTX store_ctx;
-      X509_STORE_CTX_init (&store_ctx, CA_certs, x, NULL);
-      if (X509_verify_cert (&store_ctx) > 0)
-	chain = X509_STORE_CTX_get1_chain (&store_ctx);
+      X509_STORE_CTX *ctx;
+      X509_STORE_CTX_init (ctx, CA_certs, x, NULL);
+      if (X509_verify_cert (ctx) > 0)
+	chain = X509_STORE_CTX_get1_chain (ctx);
       else
 	{
 	  const char *err_str;
-	  err_str = X509_verify_cert_error_string (store_ctx.error);
+	  err_str = X509_verify_cert_error_string (X509_STORE_CTX_get_error(ctx));
 	  *err_ret = srv_make_new_error ("22023", "XENCX", "X509 error: %s", err_str);
-	  X509_STORE_CTX_cleanup (&store_ctx);
+	  X509_STORE_CTX_cleanup (ctx);
 	  goto err;
 	}
-      X509_STORE_CTX_cleanup (&store_ctx);
+      X509_STORE_CTX_cleanup (ctx);
       if (chain)
 	{
 	  certs = sk_X509_new_null ();
@@ -7595,14 +7648,14 @@ bif_xenc_x509_ca_certs_list (caddr_t * q
   sec_check_dba ((QI*)qst, me);
   in = BIO_new (BIO_s_mem ());
   mutex_enter (xenc_keys_mtx);
-  certs = CA_certs->objs;
+  certs = X509_STORE_get0_objects(CA_certs);
   len = sk_X509_OBJECT_num (certs);
   for (i = 0; i < len; i++)
     {
       X509_OBJECT * obj = sk_X509_OBJECT_value (certs, i);
-      if (obj->type == X509_LU_X509)
+      if (X509_OBJECT_get_type(obj) == X509_LU_X509)
 	{
-	  X509 *x = obj->data.x509;
+	  X509 *x = X509_OBJECT_get0_X509(obj);
 	  caddr_t itm;
 	  int blen;
 	  BIO_reset (in);
--- a/configure.ac
+++ b/configure.ac
@@ -881,17 +881,6 @@ AC_TRY_COMPILE([
     ])
 
 AC_MSG_CHECKING([OpenSSL version])
-AC_TRY_COMPILE([
-#include <openssl/opensslv.h>
-],[
-#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
-#error OpenSSL version too new
-#endif
-    ],[
-      AC_MSG_RESULT([< 1.1.0])
-    ],[
-      AC_MSG_ERROR([OpenSSL version 1.1.0 or greater is currently not supported.])
-    ])
 
 AC_MSG_CHECKING([usability of the OpenSSL header files and library in ${openssl_dir}])
 AC_TRY_RUN([

(configure.ac patch needs adapting to apply to develop/7 but the intent is obvious)

Note that I've not tested the above for compatibility with 1.0.2 but that shouldn't really be a concern for much longer.

If you are applying this on 7.2.5.1 you also need a7d1535.

@@ -770,7 +772,7 @@ bif_smime_verify (caddr_t * qst, caddr_t * err_ret, state_slot_t ** args)
if (DV_TYPE_OF (msg) == DV_STRING_SESSION)
{
in_bio = strses_to_bio ((dk_session_t *) msg);
to_free = ((BUF_MEM *) in_bio->ptr)->data;
to_free = ((BUF_MEM *) BIO_get_data(out_bio))->data;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
to_free = ((BUF_MEM *) BIO_get_data(out_bio))->data;
to_free = ((BUF_MEM *) BIO_get_data(in_bio))->data;

@@ -780,7 +782,7 @@ bif_smime_verify (caddr_t * qst, caddr_t * err_ret, state_slot_t ** args)
p7 = SMIME_read_PKCS7 (in_bio, &data_bio);
if (to_free)
{
((BUF_MEM *) in_bio->ptr)->data = to_free;
((BUF_MEM *) BIO_get_data(out_bio))->data = to_free;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
((BUF_MEM *) BIO_get_data(out_bio))->data = to_free;
((BUF_MEM *) BIO_get_data(in_bio))->data = to_free;

n = sig->length;
s = sig->data;
n = ASN1_STRING_length(sig);
s = ASN1_STRING_get0_data(sig);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debian did not make these changes, but it doesn't really matter. Both should work.

RSA *rsa = NULL;
RSA *rsa;
BIGNUM *bn;
EVP_PKEY *pk = NULL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see - this isn't needed. This line was excluded from the Debian patch.

static inline const unsigned char *ASN1_STRING_get0_data(const ASN1_STRING *x)
{
return x->data;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my comment earlier, debian did not add ASN1_STRING_get0_data as it wasn't required. It doesn't strictly matter though.

static inline void *BIO_get_data(BIO *a)
{
return a->ptr;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informatory note: Debian did not have this but I think it ought have. That said, 1.0 compatibility was probably not their concern.

raspbian-autopush pushed a commit to raspbian-packages/virtuoso-opensource that referenced this pull request Feb 19, 2021
Bug-Debian: https://bugs.debian.org/828594
Forwarded: openlink/virtuoso-opensource#583
Last-Update: Mo 11. Feb 13:50:17 CET 2019 (by Andreas Tille <tille@debian.org>)


Gbp-Pq: Name ssl1.1.patch
@TallTed
Copy link
Collaborator

TallTed commented Jul 28, 2021

@sebastianas -- I think this PR has been obviated by relatively recent commits to develop/7 and stable/7 branches.

Please confirm and/or close if you agree.

raspbian-autopush pushed a commit to raspbian-packages/virtuoso-opensource that referenced this pull request Aug 20, 2021
Bug-Debian: https://bugs.debian.org/828594
Forwarded: openlink/virtuoso-opensource#583
Last-Update: Mo 11. Feb 13:50:17 CET 2019 (by Andreas Tille <tille@debian.org>)


Gbp-Pq: Name ssl1.1.patch
raspbian-autopush pushed a commit to raspbian-packages/virtuoso-opensource that referenced this pull request Aug 20, 2021
Bug-Debian: https://bugs.debian.org/828594
Forwarded: openlink/virtuoso-opensource#583
Last-Update: Mo 11. Feb 13:50:17 CET 2019 (by Andreas Tille <tille@debian.org>)


Gbp-Pq: Name ssl1.1.patch
raspbian-autopush pushed a commit to raspbian-packages/virtuoso-opensource that referenced this pull request Oct 26, 2021
Bug-Debian: https://bugs.debian.org/828594
Forwarded: openlink/virtuoso-opensource#583
Last-Update: Mo 11. Feb 13:50:17 CET 2019 (by Andreas Tille <tille@debian.org>)


Gbp-Pq: Name ssl1.1.patch
raspbian-autopush pushed a commit to raspbian-packages/virtuoso-opensource that referenced this pull request Feb 25, 2023
Bug-Debian: https://bugs.debian.org/828594
Forwarded: openlink/virtuoso-opensource#583
Last-Update: Mo 11. Feb 13:50:17 CET 2019 (by Andreas Tille <tille@debian.org>)


Gbp-Pq: Name ssl1.1.patch
@pkleef
Copy link
Collaborator

pkleef commented May 12, 2023

Virtuoso versions 7.2.6 and newer have been tested against the following versions of OpenSSL/LibreSSL:

  • OpenSSL 0.9.8 (deprecated)
  • OpenSSL 1.0.0 (deprecated)
  • OpenSSL 1.0.1 (deprecated)
  • OpenSSL 1.0.2 (still supported on RedHat Enterprise Linux 7.x and other distributions)
  • OpenSSL 1.1.0 (out of support)
  • OpenSSL 1.1.1 (still supported until 11th September 2023)
  • OpenSSL 3.0.2 (current Long Term Support version)

as well as

  • LibreSSL 2.x (probably out of support)
  • LibreSSL 3.x

Note that while we do support older versions of OpenSSL from a historic point of view, we recommend you use the latest version available on your platform.

We thank everyone that suggested patches or commented in this thread.

@pkleef pkleef closed this May 12, 2023
raspbian-autopush pushed a commit to raspbian-packages/virtuoso-opensource that referenced this pull request Feb 27, 2024
Bug-Debian: https://bugs.debian.org/828594
Forwarded: openlink/virtuoso-opensource#583
Last-Update: Mo 11. Feb 13:50:17 CET 2019 (by Andreas Tille <tille@debian.org>)


Gbp-Pq: Name ssl1.1.patch
raspbian-autopush pushed a commit to raspbian-packages/virtuoso-opensource that referenced this pull request Feb 27, 2024
Bug-Debian: https://bugs.debian.org/828594
Forwarded: openlink/virtuoso-opensource#583
Last-Update: Mo 11. Feb 13:50:17 CET 2019 (by Andreas Tille <tille@debian.org>)


Gbp-Pq: Name ssl1.1.patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants