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/acme: Make challenges world readable #101726

Closed

Conversation

@aneeshusa
Copy link
Contributor

@aneeshusa aneeshusa commented Oct 26, 2020

Motivation for this change

By making it world-readable,
this enables setting a group for a given cert
that does not include the webserver used for an http-01 challenge.
For example, this allows setting the group to postgres
while using nginx to serve the challenge,
without having to create a group containing both postgres and nginx.

Additionally, change the tmpfiles rule
to always create the directory as owned by acme
to avoid duplicate tmpfiles for the same webroot
trying to make it be owned by different groups
(e.g. both nginx and postgres),
which causes:

setting up tmpfiles
/etc/tmpfiles.d/00-nixos.conf:23: Duplicate line for path "/var/lib/acme/acme-challenge/.well-known/acme-challenge", ignoring.

The challenge data does not need to be kept private
(as it is world-accessible over HTTP to fulfill the challenge!)
so this is safe to do.

Note that lego already writes any challenge files out as 644,
but the UMask was impeding this by preventing world-readability.
Omitting it will cause us to use systemd's default of 022.

No units should not leak any sensitive data from this:

  • minica creates self-signed certs which are not sensitive
    since they won't be trusted
  • lego creates account and cert files with 600 permissions.

Note that all units also use PrivateTmp so we only need to consider
files which are directly output into a directory in BindPaths;
they all also chmod their outputs to appropriate permissions,
but that is not sufficient as we don't want any window (i.e. before chmod)
with insecure permissions on relevant files.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
By making it world-readable,
this enables setting a `group` for a given cert
that does not include the webserver used for an http-01 challenge.
For example, this allows setting the group to `postgres`
while using `nginx` to serve the challenge,
without having to create a group containing both `postgres` and `nginx`.

Additionally, change the tmpfiles rule
to always create the directory as owned by acme
to avoid duplicate tmpfiles for the same webroot
trying to make it be owned by different groups
(e.g. both `nginx` and `postgres`),
which causes:
```
setting up tmpfiles
/etc/tmpfiles.d/00-nixos.conf:23: Duplicate line for path "/var/lib/acme/acme-challenge/.well-known/acme-challenge", ignoring.
```

The challenge data does not need to be kept private
(as it is world-accessible over HTTP to fulfill the challenge!)
so this is safe to do.

Note that lego already writes any challenge files out as 644,
but the UMask was impeding this by preventing world-readability.
Omitting it will cause us to use systemd's default of 022.

No units should not leak any sensitive data from this:
- minica creates self-signed certs which are not sensitive
  since they won't be trusted
- lego creates account and cert files with 600 permissions.

Note that all units also use PrivateTmp so we only need to consider
files which are directly output into a directory in BindPaths;
they all also chmod their outputs to appropriate permissions,
but that is not sufficient as we don't want any window (i.e. before chmod)
with insecure permissions on relevant files.
@aneeshusa
Copy link
Contributor Author

@aneeshusa aneeshusa commented Oct 26, 2020

AFAICT this was added in #91121 but I didn't see any discussion/specific rationale for it, lmk if I missed something. (Learned a neat trick with PrivateTmp and BindPath from that PR!)
I think this PR may supersede #100356 but I'm not sure.

Note that I also ran into #101389 (comment) some additional setup to be able to get a cert for PostgreSQL, but this was the first hurdle I ran into and think it will be useful for other non-PostgreSQL services.

Code where lego writes out files:

cc @m1cr0man @NixOS/acme

Copy link
Contributor

@m1cr0man m1cr0man left a comment

This seems good! I gave it a thorough testing locally, and I'm happy with the resulting permissions for all the services. #100356 is still relevant as those examples are (manually) generating certificates that the web servers themselves will use. If you see any other parts of that doc that need updating to reflect this change please update them, but I think it's fine.

@m1cr0man
Copy link
Contributor

@m1cr0man m1cr0man commented Dec 15, 2020

This is also resolved in #106857. Damn, I thought this was merged already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants