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

Fail to build 2.10.0 with libressl 3.5.x #8702

Closed
rozhuk-im opened this issue Feb 19, 2023 · 9 comments · Fixed by #8708 or #8709
Closed

Fail to build 2.10.0 with libressl 3.5.x #8702

rozhuk-im opened this issue Feb 19, 2023 · 9 comments · Fixed by #8708 or #8709

Comments

@rozhuk-im
Copy link
Contributor

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Try to build with libressl 3.5.4

Environment (please complete the following information):

  • OS: FreeBSD 13.2/stable
  • Architecture: amd64

Additional context

FAILED: libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o 
/usr/local/bin/ccache /usr/local/libexec/ccache/cc -DCMAKE_BUILD_TYPE=Release -DEXT_PATH=\"/usr/local/lib/freerdp2/extensions\" -DFREERDP_EXPORTS -DHAVE_CONFIG_H -DSWSCALE_FOUND=1 -DWINPR_EXPORTS -DWITH_OPENSSL -DWITH_X11 -DWITH_XKBFILE -D_FILE_OFFSET_BITS=64 -Dfreerdp_EXPORTS -I/tmp/ports/usr/ports/net/freerdp/work/.build -I/tmp/ports/usr/ports/net/freerdp/work/.build/include -I/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0/include -I/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0/winpr/include -I/tmp/ports/usr/ports/net/freerdp/work/.build/winpr/include -O2 -pipe -O3 -pipe -funroll-loops -mretpoline  -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing  -fdebug-prefix-map=/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0=. -Wno-unused-parameter -Wno-unused-macros -Wno-padded -Wno-c11-extensions -Wno-gnu -Wno-unused-command-line-argument -Wno-deprecated-declarations -fno-omit-frame-pointer -DWINPR_DLL -O2 -pipe -O3 -pipe -funroll-loops -mretpoline  -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing  -fdebug-prefix-map=/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0=. -DNDEBUG -fPIC -MD -MT libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o -MF libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o.d -o libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o -c /tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0/libfreerdp/crypto/crypto.c
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:980:8: error: use of undeclared identifier 'NID_sha3_224'
                case NID_sha3_224:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:982:8: error: use of undeclared identifier 'NID_sha3_256'
                case NID_sha3_256:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:984:8: error: use of undeclared identifier 'NID_sha3_384'
                case NID_sha3_384:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:986:8: error: use of undeclared identifier 'NID_sha3_512'
                case NID_sha3_512:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:989:8: error: use of undeclared identifier 'NID_shake128'
                case NID_shake128:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:991:8: error: use of undeclared identifier 'NID_shake256'
                case NID_shake256:
                     ^
6 errors generated.
@akallabeth akallabeth added this to the stable-next milestone Feb 20, 2023
akallabeth added a commit to akallabeth/FreeRDP that referenced this issue Feb 20, 2023
akallabeth added a commit to akallabeth/FreeRDP that referenced this issue Feb 20, 2023
@akallabeth
Copy link
Member

@rozhuk-im could you check that the referenced pull requests fix your issue?

akallabeth added a commit to akallabeth/FreeRDP that referenced this issue Feb 20, 2023
akallabeth added a commit to akallabeth/FreeRDP that referenced this issue Feb 20, 2023
@rozhuk-im
Copy link
Contributor Author

No, it does not fix all errors:

FAILED: libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o 
/usr/local/bin/ccache /usr/local/libexec/ccache/cc -DCMAKE_BUILD_TYPE=Release -DEXT_PATH=\"/usr/local/lib/freerdp2/extensions\" -DFREERDP_EXPORTS -DHAVE_CONFIG_H -DSWSCALE_FOUND=1 -DWINPR_EXPORTS -DWITH_OPENSSL -DWITH_X11 -DWITH_XKBFILE -D_FILE_OFFSET_BITS=64 -Dfreerdp_EXPORTS -I/tmp/ports/usr/ports/net/freerdp/work/.build -I/tmp/ports/usr/ports/net/freerdp/work/.build/include -I/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0/include -I/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0/winpr/include -I/tmp/ports/usr/ports/net/freerdp/work/.build/winpr/include -O2 -pipe -O3 -pipe -funroll-loops -mretpoline  -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing  -fdebug-prefix-map=/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0=. -Wno-unused-parameter -Wno-unused-macros -Wno-padded -Wno-c11-extensions -Wno-gnu -Wno-unused-command-line-argument -Wno-deprecated-declarations -fno-omit-frame-pointer -DWINPR_DLL -O2 -pipe -O3 -pipe -funroll-loops -mretpoline  -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing  -fdebug-prefix-map=/tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0=. -DNDEBUG -fPIC -MD -MT libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o -MF libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o.d -o libfreerdp/CMakeFiles/freerdp.dir/crypto/crypto.c.o -c /tmp/ports/usr/ports/net/freerdp/work/freerdp-2.10.0/libfreerdp/crypto/crypto.c
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:980:8: error: use of undeclared identifier 'NID_sha3_224'
                case NID_sha3_224:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:982:8: error: use of undeclared identifier 'NID_sha3_256'
                case NID_sha3_256:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:984:8: error: use of undeclared identifier 'NID_sha3_384'
                case NID_sha3_384:
                     ^
../freerdp-2.10.0/libfreerdp/crypto/crypto.c:986:8: error: use of undeclared identifier 'NID_sha3_512'
                case NID_sha3_512:
                     ^
4 errors generated.

but this does:

@@ -976,7 +976,7 @@ WINPR_MD_TYPE crypto_cert_get_signature_alg(X509* xcert)
 			return WINPR_MD_SHA512;
 		case NID_ripemd160:
 			return WINPR_MD_RIPEMD160;
-#if (OPENSSL_VERSION_NUMBER >= 0x1010101fL) || defined(LIBRESSL_VERSION_NUMBER)
+#if defined(NID_sha3_224)
 		case NID_sha3_224:
 			return WINPR_MD_SHA3_224;
 		case NID_sha3_256:
@@ -986,10 +986,12 @@ WINPR_MD_TYPE crypto_cert_get_signature_alg(X509* xcert)
 		case NID_sha3_512:
 			return WINPR_MD_SHA3_512;
 #endif
+#if defined(NID_shake128)
 		case NID_shake128:
 			return WINPR_MD_SHAKE128;
 		case NID_shake256:
 			return WINPR_MD_SHAKE256;
+#endif
 		case NID_undef:
 		default:
 			return WINPR_MD_NONE;

OpenSSL and LibreSSL uses #define for NID_* so it work better than playing around with lib version and name.

@akallabeth
Copy link
Member

@rozhuk-im sorry, did you test the correct branch? #if (OPENSSL_VERSION_NUMBER >= 0x1010101fL) || defined(LIBRESSL_VERSION_NUMBER) has been replaced by #if (OPENSSL_VERSION_NUMBER >= 0x1010101fL) in the fix branch.
if libressl still has this bug then they export invalid openssl compatibility versions...

@akallabeth
Copy link
Member

@rozhuk-im ok, tried a test install of libressl, how do they version their stuff?
they export OPENSSL_VERSION 0 (which looks ok as they are only compatible with the 0.98x API) but set OPENSSL_VERSION_NUMBER 0x20000000L which is completely incompatible with the versioning from OpenSSL

@rozhuk-im
Copy link
Contributor Author

Have no idea how they make version :)
#if defined(NID_sha3_224) + #if defined(NID_shake128) works without version digging.

@akallabeth
Copy link
Member

@rozhuk-im and can be considered a dirty hack. so how do they version their stuff, hash algorigthms are usually introduced with a version (or deprecated), so just ignoring this is no good practice.

@ffontaine
Copy link
Contributor

sha-3 is not supported by any version of libressl so

#if (OPENSSL_VERSION_NUMBER >= 0x1010101fL) || defined(LIBRESSL_VERSION_NUMBER)

could be replaced by:

#if (OPENSSL_VERSION_NUMBER >= 0x1010101fL) && !defined(LIBRESSL_VERSION_NUMBER)

NID_shake128 and NID_shake256 are also not supported in any version of libressl so they could be protected by #if !defined(LIBRESSL_VERSION_NUMBER)

akallabeth added a commit to akallabeth/FreeRDP that referenced this issue Feb 22, 2023
akallabeth added a commit to akallabeth/FreeRDP that referenced this issue Feb 22, 2023
@akallabeth
Copy link
Member

@ffontaine thank you. do you know by chance how they interpret OPENSSL_VERSION_NUMBER compatibility?

@akallabeth akallabeth linked a pull request Feb 22, 2023 that will close this issue
@ffontaine
Copy link
Contributor

I'm not directly involved in libressl but it seems that OPENSSL_VERSION_NUMBER is never updated. Here is an extract of libressl/portable#573:

Question 2: OPENSSL_VERSION_NUMBER seems to be always set to 0x20000000L with LibreSSL. Would that not break client code that uses this macro to support multiple OpenSSL versions, such as in my example above?

LibreSSL bumped OPENSSL_VERSION_NUMBER early on - unfortunately there is a lot of code that checks version rather than actual feature/function available and without doing this, many of the new features would not be detected/used. LIBRESSL_VERSION_NUMBER exists if you really need to use it.

mfleisz pushed a commit that referenced this issue Feb 22, 2023
@akallabeth akallabeth linked a pull request Feb 22, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants