libsystemd/sd-id128: use only internal hmac, remove khash/OpenSSL support

[bluca]

Some distribution vendors, like Canonical, do not accept that OpenSSL
< 3.0 can link against GPL2+ binaries using the 'system library exception'.
When moving to OpenSSL >= 3.0 which is licensed under Apache2, GPL2+
sources that are built in a binary that links against OpenSSL have to be
distributed under GPL3+ to be compatible with Apache2. This would be
problematic for libsystemd.so, since external programs with various licenses
link to it, and would constitute in effect a license change for the library
for all practical purposes. Many users have demonstrated willingness to go
to extraordinary lengths in order to avoid the GPL3, whether we like it or
not.

The khash implementation is awkward as it requires context switches and
computation inside the kernel, thus leaving the process.

Remove both from libsystemd.so, and use exclusively the internal hmac fallback.
While this is not optimized, the sd-id128 API is not used in
performance-critical contexts where hardware acceleration would make a
noticeable difference.

[poettering]

Uh, what? please, let's not slow down Fedora unnecessarily. If some distros don't want to go for slow khash instead of openssl for this, great, but let's not make that mandatory.

I am not convinced at all this is desirable. This cements one reading of the legal situation, and one I think is not universally accepted.

[bluca]

Uh, what? please, let's not slow down Fedora unnecessarily. If some distros don't want to go for slow khash instead of openssl for this, great, but let's not make that mandatory.

I am not convinced at all this is desirable. This cements one reading of the legal situation, and one I think is not universally accepted.

So, there is disagreement regarding the old OpenSSL license and GPL. But I don't believe there's any widespread disagreement regarding Apache2 plus GPL-2.0. The author of both licenses assert that they are not compatible:

https://www.apache.org/licenses/GPL-compatibility.html
https://www.gnu.org/licenses/license-list.html#apache2

Yes your mileage might vary, and if you pay a lawyer enough they'll give you the answer you prefer, but when the authors of both licenses say "yep these two don't mix" it's quite a strong message to simply ignore.

So the issue is not with the status quo, but with the coming switch to OpenSSL 3 and the uncertainty it causes. Right now, libsystemd.so is unequivocally distributed under LGPL-2.1-or-later, there's no doubt about that. But once you start linking it with an Apache2 library, as an external user the cautionary and conservative approach is to assume that it's distributed under LGPL-3.0-or-later to avoid having any doubt whatsoever about the perceived incompatibility stated by the authors of the licenses. That means, simply by virtue of upgrading from distro release version X to Y, the licensing situation of your third party software linking to libsystemd.so changes, and it changes a lot - way too many people/organizations/companies are unnecessarily and wrongly allergic to GPL-3.0. I don't like it and I don't think it's justified, but it's just how it is and I cannot change it. What I can do is ensuring that external users are not put in an awkward position, and are not forced to have difficult conversations with their company lawyers, because talking to lawyers sucks massively.

Uncertainty is bad, when things are uncertain the default answer in a company setting is almost always "avoid", which I don't think we want.

[poettering]

So you are basically saying that switching Fedora OpenSSL 3 actually creates new license issues instead of removing them? Because before Fedora considered OpenSSL to be part of the core OS and thus circumvented the system lib wording of LGPL. But with the new license that doesn#t work anymore and OpenSSL 3 suddenly will create problems for software relying on this?

[bluca]

So you are basically saying that switching Fedora OpenSSL 3 actually creates new license issues instead of removing them? Because before Fedora considered OpenSSL to be part of the core OS and thus circumvented the system lib wording of LGPL. But with the new license that doesn#t work anymore and OpenSSL 3 suddenly will create problems for software relying on this?

That is my personal interpretation, yes. The fact that they went through all the trouble of relicensing to solve the incompatibility with the GPL, just to pick the ONE permissive license that is still clearly incompatible with a version of the GPL is simply mind-boggling to me, but alas that's where we are. It is a step forward of course: GPL3+ and Apache2 are clearly and unambiguously compatible, so that's good at least - when that's possible, there's a clear way out.

Anyway, the issue is always uncertainty - where there's license doubts there's trouble. There's enormous value in having a situation that is crystal clear and not subject to any possible alternative interpretation. Which is where I'm trying to get us toward.

Here's a fun one, that explains why I'm so happy that I'm not a lawyer: exceptions are not transitive. Even if we had one of the usual "OpenSSL + GPL clauses" acked by all contributors applied to libsystemd.so, it would only apply to the library itself. It would NOT apply to any external program linking to libsystemd.so. I recently learned this. Fun eh? Now if this is true, what does this mean for a vendor that claims "system lib clause" exception? Assuming it's valid, it's clear it applies to said vendor's first-party programs that link to libssl. Does it apply to third party programs that link to a vendor library that links to libssl? Does the third-party license situation change depending on which distro the program is deployed on, even if it's the same identical binary a libsystemd.so builds are ABI-compatible, depending on whether the vendor claims the system exception or not? I haven't the faintest clue. Actually I don't want to know, and I'd really like our users not to have to ponder about that too.

[poettering]

So, we had a brief discussion elsewhere and the thing is that the system library exception isn't affected by the openssl license change: if you your distro was OK with assuming that openssl was a system lib before then it can assume that it is after too, the license change doesn't affect the assumption. Fedora/RHEL famously assume openssl is a system lib, hence it's find there to continue to link against openssl from any gpl code.

Hence, I am pretty sure we should not make this change: what was OK by fedora/rhel rules before should remain OK for them. Hence, the option to link directly to openssl should be kept. If debian/others disagree that is fine, and they can use khash, but I am not convinced we should from upstream mandate these changes for rhel/fedora given they never had a problem with assuming the system extension rule applying here.

Here's another idea we had: we currently embed a copy of SHA into sd-boot, because we cannot really link to anything there. It's copied out of glibc, should have OK licensing for all out users hence. Maybe we should move that from sd-boot into src/fundamental/ so that we can compile it for the userspace version too. If we embed a version of that anyway, we might as well use it at multiple places. If we did this we can drop the khash stuff, which Id like very much.

[bluca]

So, we had a brief discussion elsewhere and the thing is that the system library exception isn't affected by the openssl license change: if you your distro was OK with assuming that openssl was a system lib before then it can assume that it is after too, the license change doesn't affect the assumption. Fedora/RHEL famously assume openssl is a system lib, hence it's find there to continue to link against openssl from any gpl code.

Hence, I am pretty sure we should not make this change: what was OK by fedora/rhel rules before should remain OK for them. Hence, the option to link directly to openssl should be kept. If debian/others disagree that is fine, and they can use khash, but I am not convinced we should from upstream mandate these changes for rhel/fedora given they never had a problem with assuming the system extension rule applying here.

Thing is, as mentioned these exceptions are likely not transitive - while it works for executables shipped by rhel/fedora, it doesn't necessarily apply to third parties shipping their own executables. That's why the various tools and daemons in our repo are fine, and the problem is only libsystemd.so. Think about this: you ship foobard which links to libsystemd.so to both Ubuntu and Fedora users. Ubuntu tells you libsystemd.so is not linked with openssl as that's not allowed. Fedora tells you libsystemd is linked with openssl as that's allowed. They can't both be right, it's the same code with the same licenses. What do you do? This creates doubt and uncertainty, even if things work out in the end. But anyway, looks like we got a good way forward, so it's all moot.

Here's another idea we had: we currently embed a copy of SHA into sd-boot, because we cannot really link to anything there. It's copied out of glibc, should have OK licensing for all out users hence. Maybe we should move that from sd-boot into src/fundamental/ so that we can compile it for the userspace version too. If we embed a version of that anyway, we might as well use it at multiple places. If we did this we can drop the khash stuff, which Id like very much.

I like this! At that point, if we have this sha256 in use in libsystemd.so instead of khash, would you be ok with dropping the openssl sha256 from it?

[poettering]

I like this! At that point, if we have this sha256 in use in libsystemd.so instead of khash, would you be ok with dropping the openssl sha256 from it?

So it's a slow implementation, i.e. doesn't use all those fancy CPU features. typically openssl is much preferable hence.

We could do it like this: use dlopen() to load openssl — witht the RTLD_NOLOAD flag. That way, if the library is already linked to we can just use it, otherwise we'll fall back to the slower implementation we copied from glibc. If people already made the decision that openssl is fine for them to link to from the current process then we'll piggyback on that and also use it. I mean, we use dlopen() stuff all over the place already, this would be just one more place.

[bluca]

I like this! At that point, if we have this sha256 in use in libsystemd.so instead of khash, would you be ok with dropping the openssl sha256 from it?

So it's a slow implementation, i.e. doesn't use all those fancy CPU features. typically openssl is much preferable hence.

We could do it like this: use dlopen() to load openssl — witht the RTLD_NOLOAD flag. That way, if the library is already linked to we can just use it, otherwise we'll fall back to the slower implementation we copied from glibc. If people already made the decision that openssl is fine for them to link to from the current process then we'll piggyback on that and also use it. I mean, we use dlopen() stuff all over the place already, this would be just one more place.

Yeah that could work - but are the sd_id128_get_* APIs performance critical and worth the additiona complications?

[keszybz]

codesearch.debian.net says that those APIs (sd_id128_get_{machine,boot}_app_specific) are only used by us and network-manager (for DHCP). This means that we don't care about speed of the implementation at all — nobody is calling this function in a tight loop, and it's called on time amounts of data, so a fancy implementation that can hash large amounts of data quickly is just as good as an unoptimized implementation.

I think it's quite important to drop the linkage to openssl from libsystemd, completely independently of the license considerations. Thousands of different binaries link to libsystemd, and it means that we pull in openssl into all of them, even if it's unused in 99.9% of cases.

So yeah, please just elevate src/boot/efi/sha256.c to shared code and use it here.

[poettering]

note that what we copied is sha256, not more. we need to build hmac around it, but that should be easy. but it should be done with some test vectors that validate that it is correctly implemented.

[bluca]

All right, let's get this home-grown crypto party going, what could possibly go wrong! I've moved the sha256 over, and implemented hmac following the FIPS spec. Output seems to match openssl's, so it's only ever so slightly possible that it's completely borken.

If we are completely madconfident, we could even remove one of the two gcrypt usages from libsystemd, as the journal API in src/libsystemd/sd-journal/journal-authenticate.c uses it to compute sha256 hmacs.

[bluca]

Just realised sd-id128 was the last usage of khash, so I've nuked it. So long, and thanks for all the hashes!

[keszybz]

I implemented a benchmark and this leads to a massive slowdown:
Before:

/* benchmark_sd_id128_get_machine_app_specific (1000000 iterations) */
1.690203 µs each

After:

/* benchmark_sd_id128_get_machine_app_specific (1000000 iterations) */
1.659611 µs each

I guess that's the price we'll have to pay.

I'll push the benchmark into a branch. Maybe pull it into your PR… If not, I'll submit it later.

EDIT: The branches are:
https://github.com/keszybz/systemd/tree/sd128-benchmark-old
https://github.com/keszybz/systemd/tree/sd128-benchmark-new

[dnicolodi]

I implemented a benchmark and this leads to a massive slowdown:

If the numbers you quote are the right ones, I see a slight improvement.

[bluca]

Cherry-picked the benchmark commits. PTAL!

[bluca]

I implemented a benchmark and this leads to a massive slowdown:

If the numbers you quote are the right ones, I see a slight improvement.

With khash on my machine it's ~5.9 µs, while the new hmac is ~12.5 µs, so it's about twice as slow - which is expected, as there's no hardware acceleration and so on

[poettering]

lgtm. I'd probably go one step further though and simply define in fundamental/macro.h or so that when building for EFI typedef UINTN size_t + #define memcpy(a, b, c) CopyMem(a, b, c).

With that in place we need no ifdeffery in the .c file anymore, and the two identifiers are so well understood that this could not be conflicting with anything.

[bluca]

lgtm. I'd probably go one step further though and simply define in fundamental/macro.h or so that when building for EFI typedef UINTN size_t + #define memcpy(a, b, c) CopyMem(a, b, c).

With that in place we need no ifdeffery in the .c file anymore, and the two identifiers are so well understood that this could not be conflicting with anything.

Ok, implemented as requested. Upgrading green label. Thanks!

[poettering]

lgtm

[bluca]

focal tests are stuck due to disk space issues on the nodes