Bug 257403 - net/freeradius3: fails to build with libressl
Summary: net/freeradius3: fails to build with libressl
Status: Closed Works As Intended
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Ryan Steinmetz
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-25 08:20 UTC by Felix Palmen
Modified: 2021-07-31 12:33 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (zi)


Attachments
fix building with libressl (2.55 KB, patch)
2021-07-25 08:20 UTC, Felix Palmen
no flags Details | Diff
fix building and dynamic linking with libressl (3.42 KB, patch)
2021-07-25 22:13 UTC, Felix Palmen
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Felix Palmen freebsd_committer freebsd_triage 2021-07-25 08:20:23 UTC
Created attachment 226672 [details]
fix building with libressl

Building with libressl fails with the following errors:

---

src/main/cb.c:68:28: error: use of undeclared identifier 'TLS_ST_SR_CLNT_HELLO'
                        if (SSL_get_state(s) == TLS_ST_SR_CLNT_HELLO) {
                                                ^
src/main/cb.c:178:29: error: use of undeclared identifier 'SSL3_RT_INNER_CONTENT_TYPE'
        } else if (content_type == SSL3_RT_INNER_CONTENT_TYPE && buf[0] == SSL3_RT_APPLICATION_DATA) {
                                   ^
1 warning and 2 errors generated.

---

ld: error: undefined symbol: SSL_CTX_set_max_early_data
>>> referenced by tls.c
>>>               build/objs/src/main/tls.o:(tls_init_ctx)
ld: error: undefined symbol: SSL_CTX_set_max_early_data
>>> referenced by tls.c
>>>               build/objs/src/main/tls.o:(tls_init_ctx)

ld: error: undefined symbol: FIPS_mode
>>> referenced by tls.c
>>>               build/objs/src/main/tls.o:(load_dh_params)
LINK build/bin/radattr

ld: error: undefined symbol: FIPS_mode
>>> referenced by tls.c
>>>               build/objs/src/main/tls.o:(load_dh_params)

---

Attached patch fixes the build.
Comment 1 Felix Palmen freebsd_committer freebsd_triage 2021-07-25 22:13:30 UTC
Created attachment 226692 [details]
fix building and dynamic linking with libressl

Updated patch to also fix this runtime error when loading the pap module:

Error: /usr/local/etc/raddb/mods-enabled/pap[13]: Failed to link to module 'rlm_pap': /usr/local/lib/freeradius-3.0.23/rlm_pap.so: Undefined symbol "EVP_sha3_512"
Comment 2 Ryan Steinmetz freebsd_committer freebsd_triage 2021-07-26 13:31:27 UTC
Please push this upstream to the freeradius developers:
https://github.com/FreeRADIUS/freeradius-server

I do not want to sign up to maintain custom patches for this.

Thanks.
Comment 3 Felix Palmen freebsd_committer freebsd_triage 2021-07-27 08:32:24 UTC
I really don't think "works as intended" is suitable here. Libressl is documented to work with the ports tree, so IMHO, if a port fails with it, it should either be fixed or at least have an IGNORE stating it doesn't work upfront.

Upstream would be nice, but they refused such patches in the past, insisting libressl checks should check for specific libressl versions (which makes no sense if no version of libressl supports the features in question and lead to really silly discussions). So I'm pretty sure upstream isn't an option here.

Of course, adding an IGNORE entry would be an option if you don't want to maintain such patches in the port. Not the best one, but it would avoid build-time errors.
Comment 4 Alan DeKok 2021-07-29 16:27:45 UTC
We're happy to accept patches.  There is no requirement for macros which check for particular versions of libressl.  It's fine to just check for the existence of libressl, especially if the feature doesn't exist in it.

What we don't want is patches make it work only for libressl, but which break it on every other platform.

Please send any and all patches over (GitHub is preferred), and we'll take a look.

We want FreeRADIUS to build "out of the box" on as many platforms as possible.  To that end, I periodically troll through OS-specific archives, and pull in some of the patches.

The main ones which aren't pulled in are ones which break things for others.
Comment 5 Felix Palmen freebsd_committer freebsd_triage 2021-07-30 10:45:08 UTC
(In reply to Alan DeKok from comment #4)
Upstream shouldn't fight with their users. I'm thankful for a nice free software and just want to help others. My experience was *not* about a patch that "broke things for others", instead it was this: https://github.com/FreeRADIUS/freeradius-server/issues/2611

The thing with LibreSSL is that it's always "catching up" with OpenSSL on the API side (and we're talking about an API that's not really following the "information hiding" principle), so often enough, if you want to support it, the best you can do is to check for LibreSSL itself, disregarding the version.

I can perfectly understand the amount of (unnecessary) work that causes, and of course the "blame game" is fruitless. A conscious decision *not* to support LibreSSL at all would also be perfectly understandable, and would shift the burden to distributors who want to support LibreSSL.

That said, your reply sounds like you're willing to take these things into your codebase. Therefore, I'll try again and create a pull request in the next few days.
Comment 6 Alan DeKok 2021-07-30 12:40:25 UTC
The way I read that is specific versions of libressl have specific features. So it's best to have version-specific macros for that.

Simply turning off a feature for all versions of libressl seems wrong.  Doing that means people who do have the feature in libressl will complain.  And we'll then have to figure out which version works, and update the patch.

i.e. we would like to fix the issue once, and correctly.  The alternative is more work for everyone involved.
Comment 7 Felix Palmen freebsd_committer freebsd_triage 2021-07-31 07:47:15 UTC
(In reply to Alan DeKok from comment #6)
> Simply turning off a feature for all versions of libressl seems wrong.

Problem is, then we'd be back to square one. By the time you introduce a check for libressl, it's almost always because *no* version of libressl provides the feature yet. So there's also no way to know which version might introduce it.

Additionally, when they are introduced in libressl, there's typically no extra check needed any more because libressl also defines the openssl version macros. It might still break for users of specific older libressl versions, but I *guess* that's an uncommon scenario.

Yes, indeed, that makes it necessary to revisit all this stuff later :( and of course, that's a PITA. And it's the reason why I'd perfectly understand not wanting to support libressl at all… (although in my personal opinion, the bad design of the openssl API is the real culprit here, but that doesn't make it any better either)

Anyways, as long as you're fine with patches disabling some bits when using libressl, I will prepare a PR soon.
Comment 8 Alan DeKok 2021-07-31 12:33:00 UTC
If the feature isn't in any version of libressl, then just a check for that is fine.

But if we know it's in version X, and not in version Y, then there should be a way to check for that.

If they're leveraging the OpenSSL version macros, then that may be the simplest.

Libressl support was removed from the master branch because it's in heavy development, and is likely to be that way for a while.  No one should be packaging it or using it.  Trying to maintain compatibility is a lot of work.

Once v4 is ready for production, we can probably re-add libressl support fairly quickly.