Created attachment 225961 [details]
fix missing PATH_MAX
This "unbreaks" LibreSSL as it appears OpenSSL silently pulls in the required header.
thanks for the contribution. It seems to fix the compilation but down the road but we'd need to validate pkcs11-helper to use the same SSL library (else we get OpenSSL and LibreSSL mixed in some cases).
Beyond that, I wonder if this is any good. I am loathe to accept it:
* libressl still does not appear to have full OpenSSL-compatible TLS v1.3 support, as of 3.3.3.
* I think we ought to get openvpn-devel fixed first, too (it has self-test failures beyond PATH_MAX). Else we might find out we were wasting time on a temporary solution and would have to withdraw libressl support later. It seems clearer and easier to tell people "use OpenVPN with OpenSSL but not LibreSSL" to avoid confusing people - and we don't have OpenVPN upstream commitments to LibreSSL either.
=> I propose that if you can convince the upstream OpenVPN community that you have someone (possibly yourself) who commits to maintaining the LibreSSL compatibility in OpenVPN upstream, this all might stand a better chance of integration.
So, what do you propose now?
OpenVPN upstream representative here :-)
LibreSSL is not a primary supported SSL library. As in "we do not intentionally break it, but if something breaks due to unrelated patches, this might go unnoticed due to lack of systematic testing on all supported platforms".
We do test on OpenBSD (currently 6.8) with the system LibreSSL, and that works just fine (including plugin-auth-pam). So most "big" incompatibilities are spotted quickly...
$ src/openvpn/openvpn --version
OpenVPN 2.5.3 [git:r25/ecaf88f8a4e75856] x86_64-unknown-openbsd6.8 [SSL (OpenSSL)] [LZO] [LZ4] [MH/RECVDA] [AEAD] built on Jun 22 2021
library versions: LibreSSL 3.2.2, LZO 2.10
This all said, your patch to auth-pam.c makes sense - having something crypto-unrelated which just happens to build due to indirect header chains should be fixed.
Due to the way our process works: can you send the patch to email@example.com, please? We require everything to be reviewed on the public list, even if somewhat straightforward.
I'm not sure about all implications of the Makefile-patch for the FreeBSD-port - as Matthias said, this might cause problems with pkcs11-helper. So I'm not voicing support or refusal here.
Thanks for the responses. Patch sent to mailing list but it was rejected by the list software. Not sure if I need to subscribe first and/or the attachment was the issue?
I enabled PKCS11 and TEST option for OPNsense and it compiles fine and passes tests under LibreSSL. Checking with ldd neither openvpn binary nor libpkcs11-helper.so link to base OpenSSL so at least from this point of view I cannot see any obvious issues with it.
The maintainers for pkcs11-helper only mention LibreSSL build support back in 2018 and there are no open or closed issues regarding LibreSSL:
It seems to me rather straightforward and if you both agree we will be more vigilant in the future to chase LibreSSL interoperability issues in OPNsense and try to work on them with upstream directly?
That being said there were only 1-2 cases over the last 5 years or so.
A commit in branch main references this bug:
Author: Matthias Andree <mandree@FreeBSD.org>
AuthorDate: 2021-06-22 19:25:44 +0000
Commit: Matthias Andree <mandree@FreeBSD.org>
CommitDate: 2021-06-22 19:25:44 +0000
security/openvpn: fix missing include for PATH_MAX
While here, add a warning banner about libressl support status,
and clean up a leftover INSTALL_DATA workaround no longer needed.
Patch suggested and
Reported by: Franco Fichtner <firstname.lastname@example.org>
security/openvpn/Makefile | 10 +++++++---
.../openvpn/files/patch-src_plugins_auth-pam_auth-pam.c (new) | 10 ++++++++++
2 files changed, 17 insertions(+), 3 deletions(-)
So, I've added the patch on the assumption that upstream might consider the patch going forward. Regarding pkcs11-helper, since we link to the lib, its build needs to be linked against the exact same OpenSSL library that OpenVPN would use. We have a post-build target to sanity check this, and break the build on mismatch (detected by checking if we have multiple libcrypto or libssl libraries in the executable).