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

duosec: fix module [20.03] #87006

Merged
merged 1 commit into from May 5, 2020
Merged

duosec: fix module [20.03] #87006

merged 1 commit into from May 5, 2020

Conversation

@aanderse
Copy link
Contributor

aanderse commented May 5, 2020

Motivation for this change

#86852

@reanimus I'm looking to you for all testing relating to this. Also a big FYI you probably shouldn't be using this module from 20.03 or before as it leaks sensitive data into the nix store. I would highly suggest cherry picking this module from 20.09 (current unstable).

If I had to guess I would wonder if #63103 was the culprit for the error you experienced... but I'm not going to spend any time investigating or thinking about it as this is a non issue in master 🤷‍♂️

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@reanimus
Copy link
Contributor

reanimus commented May 5, 2020

it leaks sensitive data into the nix store

oh dear. how do i go about backporting this module?

@aanderse
Copy link
Contributor Author

aanderse commented May 5, 2020

@reanimus as long as you only use this on a single user machine with either an encrypted drive (or physically secure) you shouldn't need to worry too much... but in any other scenario see https://nixos.org/nixos/manual/index.html#sec-replace-modules

Specifically:

imports = [ <nixos-unstable/nixos/modules/security/duosec.nix> ];
disabledModules = [ "modules/security/duosec.nix" ];

Regardless... can I get you to test this so I can merge? 😄

@reanimus
Copy link
Contributor

reanimus commented May 5, 2020

@aanderse it's an internet-facing server so I'd prefer to be safe. thanks for the heads up.

I just tested nixos-rebuild build with your branch and it no longer errors out. I checked result/etc/duo/pam_duo.conf and it has the expected content.

This looks fine. I'll see about cherry-picking the module now. Thanks!

@aanderse aanderse requested a review from rnhmjoj May 5, 2020
@rnhmjoj
rnhmjoj approved these changes May 5, 2020
};
};

pamCfgFile = optional cfg.pam.enable {

This comment has been minimized.

Copy link
@rnhmjoj

rnhmjoj May 5, 2020

Contributor

Uhm, loginCfgFile is an attrset but the optional here is producing a list. It shouldn't be an error yet: loaOf is deprecated but it should just raise a warning, not sure what's going on here.
In any case this is the proper fix for the future removal of loaOf.

This comment has been minimized.

Copy link
@aanderse

aanderse May 5, 2020

Author Contributor

Yeah... wild stab in the dark. Not worth effort as master is different anyways. Thanks for taking a look!

@rnhmjoj rnhmjoj merged commit 8258818 into NixOS:release-20.03 May 5, 2020
13 checks passed
13 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="55f53dd"; rev="55f53dd232ac7b6047656b6e283c1312293d0ddc"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="55f53dd"; rev="55f53dd232ac7b6047656b6e283c1312293d0ddc"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="55f53dd"; rev="55f53dd232ac7b6047656b6e283c1312293d0ddc"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="55f53dd"; rev="55f53dd232ac7b6047656b6e283c1312293d0ddc"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="55f53dd"; rev="55f53dd232ac7b6047656b6e283c1312293d0ddc"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="55f53dd"; rev="55f53dd232ac7b6047656b6e283c1312293d0ddc"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="55f53dd"; rev="55f53dd232ac7b6047656b6e283c1312293d0ddc"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@aanderse aanderse deleted the aanderse:duo-20.03-fix branch May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.