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

armTrustedFirmwareRK3399 (misc/arm-trusted-firmware/default.nix): incorrect license, should be unfreeRedistributable #148890

Closed
ghost opened this issue Dec 6, 2021 · 7 comments
Labels

Comments

@ghost
Copy link

ghost commented Dec 6, 2021

Describe the bug

arm-trusted-firmware has the wrong license when built for RK3399.

This file is a binary with no source code. This is not peripheral firmware (lib.licenses.unfreeRedistributableFirmware) -- the binary file in this package is code that runs on the main CPU (at a privilege level even higher than the kernel or hypervisor!).

Although the name implies that it ostensibly has something to do with HDCP, there is no way to verify that it does not perform other functions. It is also impossible to audit it for security to ensure that it cannot be exploited. Remember, this code runs on the main CPU in EL3, above the kernel.

One possible resolution is to excise the blob from the firmware, like vinceb does in this patch

Steps To Reproduce

Steps to reproduce the behavior:

nix-env -iA armTrustedFirmwareRK3399

@ghost ghost added the 0.kind: bug Something is broken label Dec 6, 2021
@ghost
Copy link
Author

ghost commented Feb 6, 2022

ping

@ghost
Copy link
Author

ghost commented Feb 6, 2022

I have submitted two PRs; merging either one of this will fix the issue. There is no need to merge both.

@lukegb
Copy link
Contributor

lukegb commented Feb 7, 2022

I don't think the BSD license actually does require the distribution of source code, so it could be correct that the binary blob is being licensed in the upstream repo under BSD-3.

AIUI: BSD-3 is very lax and doesn't do a whole bunch beyond permitting end-user redistribution - it doesn't require that source code is distributed alongside, or that you will actually even be able to install the modified binary (although you are permitted to modify it and distribute it).

Do you have a statement from upstream that this file is not intended to be considered as licensed as BSD-3?

@ghost
Copy link
Author

ghost commented Feb 7, 2022

I don't think the BSD license actually does require the distribution of source code, so it could be correct that the binary blob is being licensed in the upstream repo under BSD-3.

For free software licenses, the nixpkgs license field refers to the licensing terms of the source code of the relevant package. This should be obvious.

If this were not the case, then licenses.nix would need to set unfree=true for the BSD license, since binaries without source code do not meet the definition of free software, which states that "accessibility of source code is a necessary condition for free software."

It is not self-consistent to argue that both (a) lib.licenses.bsd3 includes binaries which lack source code and (b) lib.licenses.bsd3.unfree should be false. You can have one or the other, but not both.

@lukegb
Copy link
Contributor

lukegb commented Feb 7, 2022

Replying first:

It isn't a philosophical change; it corrects an oversight to bring licensing in line with longstanding policy.

When I said change here in this context, I meant "the combination of both of these PRs" or "this issue". Given you've opened two separately PRs for this I want to avoid copying and pasting the same messages twice in both places and splitting the conversation... which you've already ended up doing, by replying with the exact same message (+ a bit in one of them) in both places.

Overall though, yeah, I think that's probably a reasonable stance to take. meta.licenses can't really encode the nuance that we would need to make this work well to encode what everyone is looking for.

I think the right thing to do in this case is:

  1. Add a new parameter to the package (unfreeIncludeHDCPBlob ? true, or similar?)
  2. If it's false, apply the patch to drop the blob.
  3. If it's true, don't apply the patch, and add unfreeRedistributable to the list of meta.license. The argument you make that this shouldn't be considered unfreeRedistributableFirmware since it's a blob that runs on the CPU at high PL is probably reasonable.

That's the best compromise, I think: people who haven't opted in to nonfree will get told at buildtime that they're pulling in something they might not like (and they can opt out of the feature using an overlay), and we don't lose the functionality for people relying on it.

I specifically think this way around is better than flipping it off by default because it means that it's much more noticable that you can't build and need to flip a flag - since it'll fail noisily and obviously - than it is to have some functionality missing, but WDYT?

@ghost
Copy link
Author

ghost commented Feb 8, 2022

I want to avoid copying and pasting the same messages twice in both places and splitting the conversation... which you've already ended up doing, by replying with the exact same message (+ a bit in one of them) in both places.

Sorry about that.

Overall though, yeah, I think that's probably a reasonable stance to take. meta.licenses can't really encode the nuance that we would need to make this work well to encode what everyone is looking for.

I think the right thing to do in this case is:

1. Add a new parameter to the package (`unfreeIncludeHDCPBlob ? true`, or similar?)

2. If it's `false`, apply the patch to drop the blob.

3. If it's `true`, don't apply the patch, and _add_ `unfreeRedistributable` to the list of meta.license. The argument you make that this shouldn't be considered `unfreeRedistributableFirmware` since it's a blob that runs on the CPU at high PL is probably reasonable.

That's the best compromise, I think: people who haven't opted in to nonfree will get told at buildtime that they're pulling in something they might not like (and they can opt out of the feature using an overlay), and we don't lose the functionality for people relying on it.

I specifically think this way around is better than flipping it off by default because it means that it's much more noticable that you can't build and need to flip a flag - since it'll fail noisily and obviously - than it is to have some functionality missing, but WDYT?

I agree with all of this. I will update my patch to implement this approach later tonight.

@ghost
Copy link
Author

ghost commented Feb 8, 2022

I have pushed a commit which implements @lukegb's idea at #158310; let's continue the discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants