Skip to content

nixos-install: fix SSL certificate error#15579

Merged
abbradar merged 1 commit intoNixOS:masterfrom
abbradar:nixos-install-ssl
Jun 23, 2016
Merged

nixos-install: fix SSL certificate error#15579
abbradar merged 1 commit intoNixOS:masterfrom
abbradar:nixos-install-ssl

Conversation

@abbradar
Copy link
Copy Markdown
Member

@abbradar abbradar commented May 20, 2016 •
edited
Loading

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
    • OS X
    • 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.

When trying to run nixos-install from the latest unstable ISO I got could not download https://cache.nixos.org/nix-cache-info with an SSL error. curling this URL worked fine. The problem, IIUC, is because we don't have SSL_CERT_FILE set in the ISO environment, so this (patched now) code hasn't copied the CA bundle. I don't see any reason why not just use CA bundle from nixpkgs, so this patch does just that. My theory appears to be correct because exporting this variable and running nixos-install from an unpatched ISO also fixes this problem.

cc @aszlig as the author of cf7f15c

@abbradar abbradar added 0.kind: bug Something is broken 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels May 20, 2016
@mention-bot
Copy link
Copy Markdown

mention-bot commented May 20, 2016

By analyzing the blame information on this pull request, we identified @aszlig, @edolstra and @lethalman to be potential reviewers

groxxda
groxxda reviewed May 20, 2016
View reviewed changes
nixos/modules/installer/tools/nixos-install.sh
fi
cp -Lf "@cacert@" "$mountPoint/tmp/ca-cert.crt"
export SSL_CERT_FILE=/tmp/ca-cert.crt
# For Nix 1.7
Copy link
Copy Markdown
Contributor

@groxxda groxxda May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still relevant or can we remove it while at it?

Copy link
Copy Markdown
Member Author

@abbradar abbradar Jun 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this -- anyone?

@domenkozar
Copy link
Copy Markdown
Member

domenkozar commented May 20, 2016

We should also set SSL_CERT_FILE in installer tests.

@abbradar
Copy link
Copy Markdown
Member Author

abbradar commented May 20, 2016

@domenkozar I think there's no need to with this patch (the test uses nixos-install which now always uses cacert from Nix store).

@domenkozar domenkozar added this to the 16.03 milestone May 20, 2016
@edolstra
Copy link
Copy Markdown
Member

edolstra commented May 20, 2016

I just did a 16.03 installation which succeeded fine without this, so I don't understand why this is necessary. What error message do you get exactly?

@abbradar
Copy link
Copy Markdown
Member Author

abbradar commented May 20, 2016

Interesting. FWIW I was trying to install unstable release. I'll reproduce with screenshots when I get to the machine again.

Nikolay.

@abbradar
Copy link
Copy Markdown
Member Author

abbradar commented May 20, 2016

Gettting this: https://i.imgur.com/qZmyU0z.png
With nixos-minimal-16.09pre83796.d541e0d-x86_64-linux.iso
When I export SSL_CERT_FILE=/etc/ssl/certs/ca-bundle.crt it works fine -- this could be a fix but this patch just uses cacert from nixpkgs always instead.

@boobiesinc
Copy link
Copy Markdown

boobiesinc commented May 20, 2016

Also see #14874 (comment)

@boobiesinc boobiesinc mentioned this pull request May 21, 2016
Closed
@abbradar
Copy link
Copy Markdown
Member Author

abbradar commented Jun 11, 2016

So... should we merge this? Given that this fixes an installation problem for several people, I'll do it in several days if there are no objections.

@solson solson mentioned this pull request Jun 18, 2016
Closed
@abbradar abbradar merged commit 21f9180 into NixOS:master Jun 23, 2016
@abbradar abbradar mentioned this pull request Jun 24, 2016
Closed
@domenkozar
Copy link
Copy Markdown
Member

domenkozar commented Jun 24, 2016

Backport to 16.03?

@abbradar
Copy link
Copy Markdown
Member Author

abbradar commented Jun 24, 2016

Hm, I'm not sure this problem exists in 16.03. If the patch applies it probably is though -- I'll do it.

@domenkozar
Copy link
Copy Markdown
Member

domenkozar commented Jun 24, 2016

You might be right, I was mislead by @edolstra comment it says it works there.

@abbradar
Copy link
Copy Markdown
Member Author

abbradar commented Jun 24, 2016

Well, this change seems harmless even if there's no problem with 16.03 so let's just backport -- fcd0923

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

0.kind: bug Something is broken 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS

Projects

None yet

Milestone

16.03

Development

Successfully merging this pull request may close these issues.

6 participants

@abbradar @mention-bot @domenkozar @edolstra @boobiesinc @groxxda