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

dockerTools: Fix pullImage with sandboxing enabled #29777

Closed
wants to merge 1 commit into from
Closed

dockerTools: Fix pullImage with sandboxing enabled #29777

wants to merge 1 commit into from

Conversation

seppeljordan
Copy link
Contributor

@seppeljordan seppeljordan commented Sep 25, 2017 •
edited

When pulling images with skopeo, we have to disable certificate
validation, because certificate authorities don't seem to be available
in nix builders. This shouldn't be a problem though because we check
the sha checksum after downloading the image anyway.

check #29636

Motivation for this change

dockertool.pullImage did not work with nix sandboxes. No it does.

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 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@seppeljordan
dockerTools: Fix pullImage with sandboxing enabled
f2c58c4 
When pulling images with skopeo, we have to disable certificate
validation, because certificate authorities don't seem to be available
in nix builders.  This shouldn't be a problem though because we check
the sha checksum after downloading the image anyway.
@aneeshusa
Copy link
Contributor

I am able to use pullImage with sandboxing enabled without disabling certificate validation, and I would prefer to not decrease the security here. You can make the CA list available inside sandbox builders from nix.conf, e.g. I have build-sandbox-paths = /etc/ssl/certs/ca-certificates.crt in my nix.conf (path depends on your distro).

The existing use of --insecure for curl is also undesirable IMO, but more understandable as we download via HTTPS from a wide variety of sites and sometimes they have their TLS setups break, so being able to fallback is nice. For pullImage, almost all uses are going to hit the default Docker registry, which should not be having HTTPS issues.

@seppeljordan
Copy link
Contributor Author

seppeljordan commented Sep 25, 2017 •
edited

I do not understand how we decrease security if we check the sha256 sum afterwards. Sorry, for noob question.

@aneeshusa
Copy link
Contributor

It's true that that the sha256 hash ensures we are getting the right file (modulo sha256 being broken).
However, TLS as used in HTTPS also provides confidentiality. By verifying the server certificate, we can ensure we are talking to the server we meant to talk to and not a man-in-the-middle (MITM). This also prevents leaking the URLs we are requesting to any MITM.

We should respect our users' privacy; happy to help get your certificates working inside the sandbox.

See also #4107 and #26273.

@seppeljordan
Copy link
Contributor Author

Thanks for the clarification.

@seppeljordan
Copy link
Contributor Author

I will close this PR again, I agree with @aneeshusa on the issue of privacy/security.

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
@seppeljordan @aneeshusa