add derivation for the virtualbox oracle extension pack

[cdepillabout]

Motivation for this change

This PR implements the first stage of the plan laid out by @flokli for managing the VirtualBox Oracle Extension Pack as a separate derivation:

#34796 (comment)

Currently, the VirtualBox Oracle Extension Pack needs to be downloaded directly by the end-user with the command nix-prefetch-url. The virtualbox derivation takes a boolean enableExtensionPack flag that the user needs to set.

This PR makes the following changes/additions:

1. Add the Virtualbox PUEL to the licenses file. It has been added with free = false. This means that only uses that have config.allowUnfree = true will be able to use it.
2. Split the VirtualBox Oracle Extension Pack into a separate derivation. It is now available in the all-packages.nix file as an attribute called virtualboxExtpack.
3. Change the main virtualbox derivation so that instead of taking in a boolean flag called enableExtensionPack, it instead just takes a derivation it calls extensionPack. When specified, this will normally just be the top-level virtualboxExtpack.
4. Change the NixOS virtualbox-host module to enable the extension pack when virtualisation.virtualbox.host.enableExtensionPack is set to true. This of course won't work unless allowUnfree is set to true.

Note that as explained in #34796, the end-user will still have to compile VirtualBox on their own machine. However, they will NOT have to download the Oracle Extension Pack by hand. I see this as an improvement.

This PR lays the ground-work for future improvements around VirtualBox and the Oracle Extension Pack.

Here is a NixOps file that can be used to test this change:

{
  network.description = "NixOS virtualbox test";

  # This is the configuration for a specific machine.  It follows the NixOS
  # configuration specification, which can be found below:
  # https://nixos.org/nixos/manual/index.html#ch-configuration
  vbox-test-server =
    { config, pkgs, ...}:
    {
      imports = [ ./vbox-test.nix ];

      deployment.targetEnv = "virtualbox";
      # deployment.virtualbox.headless = true;
      deployment.virtualbox.memorySize = 2024;
      deployment.virtualbox.vcpu = 1;
    };
}

And the corresponding vbox-test.nix file:

{ config, lib, pkgs, ... }:
{
  nixpkgs.config.allowUnfree = true;
  virtualisation.virtualbox.host.enable = true;
  virtualisation.virtualbox.host.enableExtensionPack = true;

  environment.systemPackages = with pkgs; [
    bash
    gitAndTools.gitFull
    firefoxWrapper
    xterm
  ];

  services.xserver.enable = true;
  services.xserver.layout = "us";
  services.xserver.desktopManager = {
    xfce.enable = true;
    default = "xfce";
  };
}

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Fits CONTRIBUTING.md.

---

[cdepillabout]

I should also ping @svanderburg since he is a maintainer for VirtualBox.

[flokli]

Thanks for this PR!

What's your opinion on adding the vbox-test.nix file mentioned in the PR description too, so somebody doing changes can locally run these tests (or a variant of the already existing virtualbox test, but calling it with virtualboxWithExtpack)

[flokli]

why do we want to have the lowPrio here? Just to make it more deterministic if somebody installs both virtualbox and virtualboxWithExtpack which one will win?

[cdepillabout]

@flokli Thanks for reviewing this!

To be honest, I'm actually not sure what role lowPrio is playing.

You can't tell because Github cuts off the all-packages.nix file, but there are actually a couple different versions of the virtualbox derivation:

  virtualbox = callPackage ../applications/virtualization/virtualbox {
    stdenv = stdenv_32bit;
    inherit (gnome2) libIDL;
    pulseSupport = config.pulseaudio or true;
  };

  virtualboxHardened = lowPrio (virtualbox.override {
    enableHardening = true;
  });

  virtualboxHeadless = lowPrio (virtualbox.override {
    enableHardening = true;
    headless = true;
  });

I setup virtualboxWithExtpack to use lowPrio because that's what virtualboxHardened and virtualboxHeadless derivations were doing.

I'm not sure if it is "correct" to use lowPrio here or not.

If you think that lowPrio shouldn't be used here, definitely let me know!

[cdepillabout]

What's your opinion on adding the vbox-test.nix file mentioned in the PR description too, so somebody doing changes can locally run these tests (or a variant of the already existing virtualbox test, but calling it with virtualboxWithExtpack)

I'm relatively new with NixOS. This is the first non-trivial PR I've sent recently. I've seen that there are pkgs/test/ and nixos/tests/ directories, but I've never actually looked inside them! I don't know enough about how the tests work to know whether or not my vbox-test.nix file should be added.

It looks like there is a section on testing in the NixOS manual, so I'll take a look at that and then take a look at the existing VirtualBox tests.

[zimbatm]

How is the license being accepted? It seems like it's possible to set config.allowUnfree = true without having read the license. It's a blanked agreement to all unfree licenses. Usually EULA require an explicit approval (which is questionable, but that's what is happening).

[zimbatm]

One idea would be that the user has to provide the sha256 of the license as a "proof" that he read and accepted it

[cdepillabout]

Hi @zimbatm, thanks for taking a look at this PR.

How is the license being accepted?

Originally (the current state of nixpkgs), if you enable virtualization.virtualbox.host.enableExtensionPack, nix-build will fail and you are presented with a message telling you to read the PUEL and asking you to download the download the Oracle Extension Pack directly by using nix-prefetch-url.

This PR changes it so that the license is accepted and the Extension Pack will be downloaded during nix-build if you set both config.allowUnfree = true and virtualization.virtualbox.host.enableExtensionPack = true.

One idea would be that the user has to provide the sha256 of the license as a "proof" that he read and accepted it

As I said in the original issue, I don't know the legal ramifications of setting it up to work one way vs another way.

However, I'm not sure I understand the reasoning for handling the PUEL as a somewhat special case, and requiring the end user to do something like provide a sha256 of the license.

I took a look at another random non-free license, and it seems to have similar wording to the PUEL. Here is the Elastic License, which is used in the non-free version of kibana, logstash, etc:

https://github.com/elastic/elasticsearch/blob/master/licenses/ELASTIC-LICENSE.txt

Here is the PUEL for comparison:

https://www.virtualbox.org/wiki/VirtualBox_PUEL

Here is the page that allows you download the Extension Pack and links to the above license:

https://github.com/elastic/elasticsearch/blob/master/licenses/ELASTIC-LICENSE.txt

---

With that said, if everyone agrees that the PUEL is special in this regard, and we should do something like require the user to provide a sha256 of the license text, then I would be happy to change this PR to do that!

My main goal is to get rid of the manual steps in installing Virtualbox with the Extension Pack.

[cdepillabout]

Just a quick follow up, in the original issue, someone said that the license situation in nixpkgs would be much better if it worked like in Gentoo, where you could directly specify a list of licenses you accept.

This would all the end-user to, for instance, accept the PUEL and not accept the Elastic License.

I think this would probably be a better design, but unfortunately nixpkgs is not set up that way. Currently setting allowUnfree = true implies acceptance of all the unfree licenses. I think that the PUEL should also be accepted by setting allowUnfree = true, just like all the other unfree licenses.

Ideally, at some point in time, it would be changed to the more granular approach of Gentoo.

[zimbatm]

Okay so this is complementary. pkgs/stdenv/generic/check-meta.nix there is a allowUnfreePredicate that would allow to build a list of authorized licenses.

I think the issue is around the notion of explicit consent, a user should not be able to use software on which he hasn't agreed on the license. allowUnfree = true is quite a low bar to say "Yes I have read and understand all the unfree license and I agree with all of them".

But like you say the mechanism is like that currently, it's independent and shouldn't hold back this PR.

[cdepillabout]

@flokli

In commit 2ae9907 I've added a test to make sure that the VirtualBox Extension Pack is successfully installed and can be used.

This test can be run like the following:

$ cd nixpkgs/nixos/tests/
$ nix-build virtualbox.nix -A enable-extension-pack

You can run all the virtualbox tests like the following:

$ nix-build virtualbox.nix

[cdepillabout]

This PR had a few conflicts with master. They have been fixed in commit e04e92d.

Unless anyone else notices anything that can be majorly improved, I think this PR should be ready to be merged as-is.

It would be great to get it merged in time for 18.09.

[zimbatm]

LGTM!

[aszlig]

@cdepillabout, @zimbatm: From the PUEL:

(2) You may not do any of the following: (a) modify any part of the Product, except to the extent allowed in the documentation accompanying the Product; (b) rent, lease, lend, re-distribute, or encumber the Product; (c) remove or alter any proprietary legends or notices contained in the Product; or (d) decompile, or reverse engineer the Product (except to the extent permitted by applicable law).

IANAL, but I think the PUEL is pretty clear about that, so I think we should not re-distribute the extension pack. Unfortunately since this adds a test with the extension pack activated, we now do exactly that:

$ nix path-info --store https://cache.nixos.org/ --json /nix/store/6f470485gpl6ri8kv7dzii7nyngxk40d-Oracle_VM_VirtualBox_Extension_Pack-5.2.14-123301.vbox-extpack | jq
[
  {
    "path": "/nix/store/6f470485gpl6ri8kv7dzii7nyngxk40d-Oracle_VM_VirtualBox_Extension_Pack-5.2.14-123301.vbox-extpack",
    "narHash": "sha256:03js53j30ddpfy1l7cg0qyr0iwnmmxv1qcjdnsrgyrf25javmxfw",
    "narSize": 19589328,
    "references": [],
    "deriver": "/nix/store/3l4m8vjmf6rqjvzknxvn4lradjjlfdsm-Oracle_VM_VirtualBox_Extension_Pack-5.2.14-123301.vbox-extpack.drv",
    "signatures": [
      "cache.nixos.org-1:JW0RhCmX5Jn8/Zq5BAByVjXqMnyADKSLuPyeO9wPjtDvDIBIGlguijePcmDJzqcFPvBWqFIPxa2RNcB53nW5Bw=="
    ],
    "url": "nar/1px2nyzd47x3hwc6z43ir3xvv2xhjc0saiw2ziyznbprs63zj7zs.nar.xz",
    "downloadHash": "sha256:1px2nyzd47x3hwc6z43ir3xvv2xhjc0saiw2ziyznbprs63zj7zs",
    "downloadSize": 19240484
  }
]

[cdepillabout]

@aszlig Thanks for bringing this up.

I didn't realize that adding a test with the extension pack would effectively add it to cache.nixos.org.

Do you know of any way to keep the test (since it is generally useful), but make sure the extension pack is not cached on cache.nixos.org? Is there maybe some way to set the test as "do not run on cache.nixos.org"?

It looks like there are other tests in nixos/tests that use code with a similar warning in the license.

Specifically, the elk.nix code:

nixpkgs/nixos/tests/elk.nix

Lines 1 to 7 in 7e0d2e9

	{ system ? builtins.currentSystem, enableUnfree ? false }:
	with import ../lib/testing.nix { inherit system; };
	with pkgs.lib;
	let
	esUrl = "http://localhost:9200";
	mkElkTest = name : elk : makeTest {

nixpkgs/nixos/tests/elk.nix

Lines 103 to 113 in 7e0d2e9

	if enableUnfree
	then {
	elasticsearch = pkgs.elasticsearch6;
	logstash = pkgs.logstash6;
	kibana = pkgs.kibana6;
	}
	else {
	elasticsearch = pkgs.elasticsearch6-oss;
	logstash = pkgs.logstash6-oss;
	kibana = pkgs.kibana6-oss;
	};

If you specify enableUnfree, then ElasticSearch with the following license is used:

https://github.com/elastic/elasticsearch/blob/master/licenses/ELASTIC-LICENSE.txt

However, it looks like enableUnfree defaults to false, so I'm betting that these aren't cached on cache.nixos.org.

Maybe the virtualbox tests should be changed to work like this as well?

(Also, this is only tangentially related, but I couldn't find an explanation of what the --store flag does in the nix path-info command. It doesn't appear to be explained in the output of nix --help or nix path-info --help. Is there an explanation of what it is doing somewhere?)

[flokli]

Hmm, the extpack test is now integrated inside the virtualbox vm tests, part of nixos/release.nix and not as a separate test file, as suggested initially.

However, from my understanding, unfree derivations should not be evaluated on hydra (and for that reason not land in cache.nixos.org) - instead I'd have guessed this would have blocked channel advancement - not sure what went wrong here…

We need to make sure this test gets skipped via hydra, either by putting it into a separate file not included from release.nix, or figuring out how to make hydra skip that specific test.

[zimbatm]

good point @aszlig. @cdepillabout can you make another PR that disables the test if allowUnfree is turned off?

[cdepillabout]

@flokli @zimbatm Sounds good, in the next couple days I'll send a PR modifying the virtualbox tests to work like the ELK tests linked above.

[grahamc]

I wonder why the NixOS tests will build tests with nonfree dependencies -- that seems like the real bug here.

[aszlig]

@grahamc: It won't, that's why this PR added nixpkgs.config.allowUnfree.

[grahamc]

I see. We should probably not do that?

…

On Aug 20, 2018, at 1:58 PM, aszlig ***@***.***> wrote: @grahamc: It won't, that's why this PR added nixpkgs.config.allowUnfree. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[cdepillabout]

I see. We should probably not do that?

@grahamc This PR is adding a derivation that will only compile if allowUnfree is set to true. The test is testing this derivation, so allowUnfree needs to be set to true to actually build it.

As I understand it, it seems like there are two problems:

1. The tests probably shouldn't set allowUnfree to true. If they DO need to do this, it should probably be behind a configuration option (similar to how the ELK tests linked above work). I am planning on sending a PR to fix this for the virtualbox tests.
2. Even if the tests do set allowUnfree to true, cache.nixos.org should either refuse to run them, or not distribute the unfree derivations. It sounds like you are suggesting that this is also a bug that needs to be fixed.

[cdepillabout]

I've created a PR to fix this issue: #45415