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: Custom ACME endpoint #72007

Merged
merged 2 commits into from Oct 30, 2019
Merged

Conversation

picnoir
Copy link
Member

@picnoir picnoir commented Oct 26, 2019

Motivation for this change

We currently do not allow users to setup a custom ACME endpoint in config.security.acme and hardcode the Let's Encrypt production/staging endpoints.

Let's encrypt is not the only entity issuing certificates using the ACME protocol anymore [1], this PR allow users to choose an alternative issuer both on a per-certificate and global scope.

This PR also has the nice side effect of allowing us to drop the pebble patch we use in the associated nixos test by using pebble's custom directory endpoint instead of the boulder one.

PR idea was suggested by a pebble maintainer.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @flokli @aszlig

@picnoir
Copy link
Member Author

picnoir commented Oct 29, 2019

Could somebody run @GrahamcOfBorg test acme to trigger the relevant nixos test?

I don't have the appropriate credentials to do that yet (NixOS/ofborg#406).

@picnoir
Copy link
Member Author

picnoir commented Oct 29, 2019

Might be blocked by #71291 (comment)

[EDIT] Fixed

@flokli
Copy link
Contributor

flokli commented Oct 29, 2019

Can we refactor the production option to influence server, or maybe just deprecate it alltogether?

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Oct 29, 2019

@GrahamcOfBorg test acme

Add a new option permitting to point certbot to an ACME Directory
Resource URI other than Let's Encrypt production/staging one.

In the meantime, we are deprecating the now useless Let's Encrypt
production flag.
The recent custom endpoint addition allows us to directly point
certbot to the custom Pebble directory endpoint.

Thanks to that, we can ditch the Pebble patch we were using so far;
making this test maintenance easier.
@picnoir
Copy link
Member Author

picnoir commented Oct 30, 2019

Can we refactor the production option to influence server, or maybe just deprecate it alltogether?

Yup, good idea.

Just updated the PR. It now deprecates the production option.

@picnoir
Copy link
Member Author

picnoir commented Oct 30, 2019

@GrahamcOfBorg test acme

[Edit] flaky test?

[Edit2] yes, flaky test.

@picnoir
Copy link
Member Author

picnoir commented Oct 30, 2019

@GrahamcOfBorg test acme

@flokli flokli merged commit 992035c into NixOS:master Oct 30, 2019
@picnoir picnoir deleted the nin-acme-custom-dir-uri branch October 30, 2019 11:33
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Oct 30, 2019
…-uri

nixos/acme: Custom ACME endpoint

(cherry picked from commit 992035c)
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jan 2, 2023

Can we refactor the production option to influence server, or maybe just deprecate it alltogether?

I don't think it was an improvement to the module to remove the option entirely. You can't remember the server URL and when testing you often want to switch to the staging server to not trigger the lets encrypt rate limit.

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

Successfully merging this pull request may close these issues.

None yet

4 participants