Initial structuring for merging nm-overrides with nix-mineral.nix

[cynicsketch]

No description provided.

[wyyllou]

looks good so far

[cynicsketch]

Attempt to evaluate the current draft version of nix-mineral.nix fails with an error:

building Nix...
building the system configuration...
error:
       … while calling the 'head' builtin

         at /nix/var/nix/profiles/per-user/root/channels/nixos/lib/attrsets.nix:1575:11:

         1574|         || pred here (elemAt values 1) (head values) then
         1575|           head values
             |           ^
         1576|         else

       … while evaluating the attribute 'value'

         at /nix/var/nix/profiles/per-user/root/channels/nixos/lib/modules.nix:809:9:

          808|     in warnDeprecation opt //
          809|       { value = builtins.addErrorContext "while evaluating the option `${showOption loc}':" value;
             |         ^
          810|         inherit (res.defsFinal') highestPrio;

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: ‘mkIf’ called with a non-Boolean condition

Not sure why.

[cynicsketch]

As far as I'm aware, l.types.bool is called the same number of times as l.mkIf so the only explanation is a typo of one of the options?

[cynicsketch]

Build works properly.

Next steps:

1. Break apart defaults into separate options i.e (filesystem, sysctl, boot param, entropy, etc.)
2. Lint with nixfmt and statix
3. Finally work on backlogged issues

[wyyllou]

are the defaults better put in the overrides section (like overrides.defaults) or in their own defaults section?

[cynicsketch]

are the defaults better put in the overrides section (like overrides.defaults) or in their own defaults section?

I'm thinking it's own section: nix-mineral.defaults.***

Doesn't really make sense to put it in overrides, overrides "override" "defaults" and they'll never be set unless the user explicitly does so.

[upidapi]

Have you thought about using just options? Where the default state could be the default and allow the to enable a disabled option and vice versa.

{
  # instead of overrides 
  current = l.mkMerge [
    (l.mkIf cfg.overrides.compatibility.allow-busmaster-bit {
      boot.kernelParams = l.mkOverride 100 ["mitigations=off"];
    })

    (l.mkIf cfg.overrides.compatibility.iommu-passthrough {
      boot.kernelParams = l.mkOverride 100 ["iommu.passthrough=1"];
    })

    {
      boot.kernelParams = [
        "mitigations=off"
        "iommu.passthrough=0"
      ];
    }
  ];
  
  # use the values directly
  alternative = l.mkMerge [
    {
      boot.kernelParams = [
        "mitigations=${if cfg.overrides.compatibility.allow-busmaster-bit then "off" else "on"}"
        "iommu.passthrough=${if cfg.overrides.compatibility.iommu-passthrough then "1" else "0"}"
      ];
    }
  ];
}

Proably then also rename the options, but maybe keep the old names as aliases

[cynicsketch]

Proably then also rename the options, but maybe keep the old names as aliases

The idea of aliases (for backwards compatibility, since many options were renamed), I like.

As for structuring overrides like this:

 alternative = l.mkMerge [
   {
     boot.kernelParams = [
       "mitigations=${if cfg.overrides.compatibility.allow-busmaster-bit then "off" else "on"}"
       "iommu.passthrough=${if cfg.overrides.compatibility.iommu-passthrough then "1" else "0"}"
     ];
   }
 ];
}

I don't like that sort of structure, because some of the overrides (i.e, disable-intelme-kmodules, disable-bluetooth-kmodules, and usbguard-gnome-integration) are additive to the defaults.

This means that there are three states as far nix-mineral is concerned with dealing with:

1. Set, but then changed. mitigations=auto,nosmt > mitigations=auto
2. Never set, but then changed. [nixos-default] > mitigations=auto
3. Set, but never changed. mitigations=auto,nosmt

Shuffling these together might be a bit disorganized or inconvenient for development purposes, IMO. Though, if there's any other implementation which can keep these separated, or something I'm missing, please do let me know.

[upidapi]

2. Set, but then changed. mitigations=auto,nosmt > mitigations=auto

Sorry, i might have been unclear. (my native language isn't English) Only in this case is where id like a change.

I don't mean to change how e.g, disable-intelme-kmodules is handled, that's the same way I'd do it. It's more that i dislike the internal use of mkOverride and mkForce, and using the .override namespace for configuration.

I think that doing stuff with parts that can be enabled / disabled. Makes it easier for the user to select what they want. And avoid accidentally settings something that they dont want. They also force you to devide the behavior into parts. Since no one wants a .other.enable option for all other settings.

Or maybe im just used to how its usually done in nixpkgs, You made this so you probably know best

So something more like this:

{
    options.nix-mineral = 
    # default to true
    let mkDisableOption = n: mkOption {
      type = types.bool;
      default = true;
      description = n;
    };
    in {
    # maybe put the notes about problems with certain options in the description?
    disable-unsigned-modules = mkDisableOption ''
      Requires all kernel modules to be signed. This prevents out-of-tree
      kernel modules from working unless signed. 
    '';
    disable-binfmt-misc = mkDisableOption ''
      Disable binfmt. Breaks Roseta.  
    '';
    disable-ip-forward = mkDisableOption ''
      Disables ip forwarding
      Reduces attack surface. May be needed for VM networking.
    '';
  };

  confg = let 
    to10 = cond: if cond then 1 else 0;
    toOnOff = cond: if cond then "on" else "off";
  in mkIf cfg.enable (mkMerge [
    {
      boot.kernelParams = [ 
        "module.sig_enforce=${to10 (!cfg.allow-unsigned-modules)}" 
      ];

      boot.kernel.sysctl."fs.binfmt_misc.status" = to10 cfg.allow-binfmt-misc;
    }
    
    # (there is probably some better way to do this)
    (mkIf cfg.allow-ip-forward {
      boot.kernel.sysctl = {
        "net.ipv4.ip_forward" = "1";
        "net.ipv4.conf.all.forwarding" = "1";
        "net.ipv4.conf.default.forwarding" = "1";
        "net.ipv6.conf.all.forwarding" = "1";
        "net.ipv6.conf.default.forwarding" = "1";
      };
    })
    (mkIf (!cfg.allow-ip-forward) {
      boot.kernel.sysctl = {
        "net.ipv4.ip_forward" = "0";
        "net.ipv4.conf.all.forwarding" = "0";
        "net.ipv4.conf.default.forwarding" = "0";
        "net.ipv6.conf.all.forwarding" = "0";
        "net.ipv6.conf.default.forwarding" = "0";
      };
    })
  ]);
}

(id be happy to do this myself and create a pr)

[cynicsketch]

Or maybe im just used to how its usually done in nixpkgs, You made this so you probably know best

So something more like this:

{
    options.nix-mineral = 
    # default to true
    let mkDisableOption = n: mkOption {
      type = types.bool;
      default = true;
      description = n;
    };
    in {
    # maybe put the notes about problems with certain options in the description?
    disable-unsigned-modules = mkDisableOption ''
      Requires all kernel modules to be signed. This prevents out-of-tree
      kernel modules from working unless signed. 
    '';
    disable-binfmt-misc = mkDisableOption ''
      Disable binfmt. Breaks Roseta.  
    '';
    disable-ip-forward = mkDisableOption ''
      Disables ip forwarding
      Reduces attack surface. May be needed for VM networking.
    '';
  };

  confg = let 
    to10 = cond: if cond then 1 else 0;
    toOnOff = cond: if cond then "on" else "off";
  in mkIf cfg.enable (mkMerge [
    {
      boot.kernelParams = [ 
        "module.sig_enforce=${to10 (!cfg.allow-unsigned-modules)}" 
      ];

      boot.kernel.sysctl."fs.binfmt_misc.status" = to10 cfg.allow-binfmt-misc;
    }
    
    # (there is probably some better way to do this)
    (mkIf cfg.allow-ip-forward {
      boot.kernel.sysctl = {
        "net.ipv4.ip_forward" = "1";
        "net.ipv4.conf.all.forwarding" = "1";
        "net.ipv4.conf.default.forwarding" = "1";
        "net.ipv6.conf.all.forwarding" = "1";
        "net.ipv6.conf.default.forwarding" = "1";
      };
    })
    (mkIf (!cfg.allow-ip-forward) {
      boot.kernel.sysctl = {
        "net.ipv4.ip_forward" = "0";
        "net.ipv4.conf.all.forwarding" = "0";
        "net.ipv4.conf.default.forwarding" = "0";
        "net.ipv6.conf.all.forwarding" = "0";
        "net.ipv6.conf.default.forwarding" = "0";
      };
    })
  ]);
}

(id be happy to do this myself and create a pr)

Thanks for showing me what you mean. Go ahead and make a pr if you want to, it looks good!

[cynicsketch]

Trying to get to perfection while also not having time to do so is probably not a wise idea.

I'm going to break apart the defaults into sysctl, boot params, etc, but not further for now and set mkDefault on everything per #27 (comment), then merge so that we can work on addressing the backlog of other issues.

This should be, at the very least, better than what we started with, and the other 50% of the way to maturity can happen later.