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

On environment.systemVariables #2953

Closed
kirelagin opened this issue Jun 15, 2014 · 14 comments
Closed

On environment.systemVariables #2953

kirelagin opened this issue Jun 15, 2014 · 14 comments

Comments

@kirelagin
Copy link
Member

Seems that #2692 had an undesirable side-effect: previously it was enough to restart the shell after the environment was updated (e.g. nixos-update switch) but now a relogin is required.

  • We should get back to the old behaviour when environment variables are exported by shell.

Also that's a good chance to reorganise that part of code. I think it might be better to prepare an attrset with all the variables intercalated with : in shell-environment.nix without the actual export statements and then have functions that translate the attrset into shell's syntax for exporting variables (export foo=bar for bash and co, zsh included; setenv foo bar for csh; etc).

  • We still want some variables exported after sudo, and preserving them in sudoers is not an option, as preserving variables as opposed to resetting them to system-defaults might have security implications.

So I propose the following: append systemVariables to variables for the purpose of shell initialisation and have systemVariables set by pam_env in sudo PAM module. As far as I can tell it's the only sane way to set some environment variables for sudo. sudo PAM session is reinitialised on every invocation, so that's acceptable.

sudo is the only case fixed by #2629 I'm aware of. If there are others, please, let me know and we'll try to come up with a more flexible solution.

@vcunat
Copy link
Member

vcunat commented Jun 15, 2014

Note that @edolstra renamed it to sessionVariables in f5055e2; he also implemented part of what you propose, I think 13befa3.

@kirelagin
Copy link
Member Author

Ah, cool.
Just got an idea and decided to share it, didn't check the git log =).

Then this issue is reduced to

  1. Reorganise shell exporting code.
  2. Consider leaving pam_env only in sudo's pam module.

@CMCDragonkai
Copy link
Member

What's the status of this problem? The sudo nix-channel --update with an HTTPS url still doesn't work?

Also the docs http://nixos.org/nixos/manual/#sec-upgrading and current ISOs still use HTTPS url, but then show adding an HTTP based URL. And furthermore the HTTPS url redirects to HTTP?

$ nix-channel --list | grep nixos
nixos https://nixos.org/channels/nixos-unstable
$ nix-channel --add http://nixos.org/channels/channel-name nixos

@vcunat
Copy link
Member

vcunat commented Jul 21, 2014

EDIT: sorry, I didn't notice you're using unstable. It does work for me on unstable/master.

True, the manual is currently slightly inconsistent. IIRC, it was claimed that https isn't possible on the Amazon cloud hosting (at least not with our certificates).

@kirelagin
Copy link
Member Author

Status of this problem is the following: the key proposal has been implemented some time ago; there are also some other things to think about, but their priority is low, so the ticket is kept open—I'll think about all this stuff again when I get some spare time.

@kirelagin
Copy link
Member Author

Status of the https problem is that it has been fixed by #2692 and, I hope, not broken by the fix to this issue.

@CMCDragonkai
Copy link
Member

Did #2692 get reverted?

@kirelagin
Copy link
Member Author

I don't think so.

@CMCDragonkai
Copy link
Member

Oh he reverted it in release-14.04 but merged in master.

@Profpatsch
Copy link
Member

(triage) @kirelagin What’s the status now?

@kirelagin
Copy link
Member Author

Well, the status is that I totally forgot about this issue and I’m not sure if it is still relevant at all. I’ll take a look ASAP (hopefully, today).

@Profpatsch
Copy link
Member

ping @kirelagin

@Shados
Copy link
Member

Shados commented May 8, 2016

FWIW, I need pam_env in more than just sudo's pam module, because as far as I can tell it is the only way to sanely set environment variables before a login shell is started, which is important if you have a shell that depends on one or more variables.

@Profpatsch
Copy link
Member

I suggest we close this (because no followup) and create new issues for related problems. @joachifm

@joachifm joachifm closed this as completed May 8, 2016
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

No branches or pull requests

6 participants