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

nixos.pam.oath: Don't ignore 1st factor authentication #22393

Closed
wants to merge 1 commit into from

Conversation

wizeman
Copy link
Member

@wizeman wizeman commented Feb 2, 2017

Motivation for this change

When setting security.pam.services.sshd.oathAuth = true in configuration.nix, instead of activating 2nd factor authentication, it seems that the OTP ends up replacing the password authentication, i.e., it ends up turning off 1st factor authentication. This has been better described in #14997.

With NixOS 16.09 plus this change, it seems that #14997 is fixed and the OTP now works properly as a 2nd factor. I wasn't able to login anymore by entering an incorrect password followed by a correct OTP.

Please note that I am not a security expert and I'm much less of a PAM expert, so I am not 100% sure that this is the correct way to fix this issue. That said, it seems to work for me.

It might be a good idea to backport this to 16.09.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@wizeman, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @markus2342 and @viric to be potential reviewers.

@wizeman
Copy link
Member Author

wizeman commented Feb 2, 2017

cc @obadz (who wrote the comment in the code I'm modifying).

@grahamc
Copy link
Member

grahamc commented Feb 12, 2017

I'm looking in to this.

@grahamc
Copy link
Member

grahamc commented Feb 12, 2017

I think this is obsoleted by my PR.

@grahamc grahamc closed this Feb 13, 2017
@wizeman wizeman deleted the u/fix-pam-oath branch February 22, 2017 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants