Created attachment 174209 [details]
Openconnect 7.07 fails to build with LibreSSL. The reason is missing checks for LIBRESSL_VERSION_NUMBER.
Openconnect also builds blindly against liblz4.so if it's present, so I've added the option to use it or not.
Builds fine in Poudriere on 10.3.
Has this been given to the openconnect developer for review?
I don't have a huge interest in maintaining local patches if he hasn't accepted it and committed it upstream.
(In reply to Ryan Steinmetz from comment #2)
I've sent it to OC's developers but there's been no response yet.
The patch is simple and quite straightforward. LibreSSL defines OPENSSL_VERSION_NUMBER as 0x20000000L and there were changes in OpenSSL API in versions 1.0.2 and 1.1.0, which is why there is:
#if OPENSSL_VERSION_NUMBER < 0x10100000L
#if OPENSSL_VERSION_NUMBER < 0x10002000L
The patch doesn't change anything for OpenSSL and only fixes the build for LibreSSL.
Adding brnrd@, the LibreSSL master so that he can review my patch :)
Please contact David Woodhouse--he's very active and committed changes to the openconnect tree today.
The triviality of the patch isn't of huge consequence to me. I want the developer(s) of the tool to accept the patches into their tree before I adopt them.
I'm not keen on this patch. It makes things *build* but it doesn't work and it fails even 'make check'.
(Btw, please add 'make check' to your build process by default).
It looks like LibreSSL has removed the support for Cisco DTLS that OpenConnect needs. It's only a trivial variation in the way the Finished MAC is calculated, and the Change Cipher Spec message. And we have unit tests for it now. But it's gone.
So on the whole I'd *very* much prefer that people aren't building against LibreSSL. If OpenSSL isn't available, perhaps build against GnuTLS instead?
Upstream/lead developer has indicated that this patch breaks things. Patch/PR rejected.
(In reply to Ryan Steinmetz from comment #7)
You could at least accept the LZ4 option.
A commit references this bug:
Date: Wed Aug 31 13:22:30 UTC 2016
New revision: 421160
- Add LZ4 OPTION
Submitted by: Piotr Kubaj
Thanks for the response.
`make check` doesn't do anything here, I'm not seeing any tests in the tests dir.
Ryan: Running tests by default isn't proper for ports. I added
> TEST_TARGET= check
to the ports' Makefile but that doesn't run any tests either
(In reply to Bernard Spil from comment #10)
Ah, right. The really interesting test was added in git after the 7.07 release. But when you're submitting patches upstream you should be generating and testing them against git master... :)
(In reply to dwmw2 from comment #11)
I have OpenConnect 7.06 working with LibreSSL 2.4.2 right now. It works just fine, even though there are DTLS-related errors when connecting (but it connects anyway).
Is it possible to disable DTLS tests or even disable DTLS support compile-time?
(In reply to Piotr Kubaj from comment #12)
It doesn't work fine. http://sites.inka.de/bigred/devel/tcp-tcp.html
I'm happy for you that you got it to build, but please don't enable the port to build against LibreSSL and please don't encourage anyone to use it this way without functional DTLS support.
Fix LibreSSL, or build the port against OpenSSL or GnuTLS.
Now that the build fixes are committed ( see http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/d37c3e0527a05c375488850cfd0fdda3b04a83dc and http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/9c36560d3ed0aaf5d40c94fb18873584afe96cb8 ) can we get my patch committed? :)
David will release a new version in the near future and the patches will be included.
Gonna close this.