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.

Sign up for GitHub

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

Jump to bottom

libvirt/qemu race condition fix. #557

Merged
merged 1 commit into from Dec 6, 2016
Merged

libvirt/qemu race condition fix. #557

merged 1 commit into from Dec 6, 2016

Conversation

NotBrianZach
Copy link
Contributor

addresses #554, works on my box, there are two issues with this fix I think,

  1. security, since we're invoking shell to pass environment, if someone hijacks args we pass in, could be worse than just using execve or whatever directly

Can't tell if there is another way to pass the env directly, there is an env attribute to the Popen call, but passing it overrides the environment as far as I can tell, which has quite a bit of state.

  1. related to 1, invoking shell leads to slightly more spam at startup I think where it tells you what .nix files it's evaluating.

@domenkozar
Copy link
Member

Why not just pass the variable via env parameter? https://docs.python.org/2/library/subprocess.html#subprocess.Popen

@NotBrianZach
Copy link
Contributor Author

NotBrianZach commented Nov 26, 2016 •
edited

EDIT: Okay ignore the rest of this, I cleaned it up.

just double checked this, I get the following error simply passing in env with public key to _logged_exec, I believe this is because (and correct me if I'm wrong) env overrides the environment, instead of simply appending any supplied variables/values.

[nix-shell:~/nixops]$ nixops deploy
trace: warning: The option `boot.loader.grub.timeout' defined in `<unknown-file>' has been renamed to `boot.loader.timeout'.
trace: warning: The option `boot.loader.grub.timeout' defined in `<unknown-file>' has been renamed to `boot.loader.timeout'.
trace: warning: The option `boot.loader.grub.timeout' defined in `<unknown-file>' has been renamed to `boot.loader.timeout'.
error: Multiple exceptions: [Errno 2] No such file or directory, [Errno 2] No such file or directory, [Errno 2] No such file or directory


>        base_image = self._logged_exec(
>                 ["nix-build"] + self.depl._eval_flags(self.depl.nix_exprs) +
>                 ["--arg", "checkConfigurationOptions", "false",
>                  "-A", "nodes.{0}.config.deployment.libvirtd.baseImage".format(self.name),
>                  "-o", "{0}/libvirtd-image-{1}".format(self.depl.tempdir, self.name)],
>                 capture_stdout=True, env={ "NIXOPS_LIBVIRTD_PUBKEY": self.client_public_key }).rstrip()
>

@domenkozar domenkozar merged commit 69e608a into NixOS:master Dec 6, 2016
@NotBrianZach NotBrianZach mentioned this pull request May 10, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@NotBrianZach @domenkozar