caddytls: Make on-demand 'ask' permission modular #6055
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I still think |
This makes the 'ask' endpoint a module, which means that developers can write custom plugins for granting permission for on-demand certificates. Kicking myself that we didn't do it this way at the beginning, but who coulda known...
|
Rebased & lint |
|
Ah thanks! I need to see if I can configure my VS Code to follow those rules as well. |
ankon
reviewed
Jan 22, 2024
|
Thanks for the review!
Huh, I coulda sworn I wrote code to account for that: I even remember returning an error message about a conflicting configuration. Thanks for pointing this out. I know I wrote it, not sure why it didn't get committed or even saved... 😅 I'll restore it... |
|
Ok, restored the error message if there's conflicting configuration. Either "ask" or "permission" must be set, but not both. Thanks for finding that! |
jroyalty
reviewed
Jan 29, 2024
|
We're missing Caddyfile support for permission modules here. We should try to get that in for 2.8.0. (If anyone's reading this, PRs welcome if you want a little task) |
I've created an issue so it's more visible and easier to track |
peterldowns
added a commit
to peterldowns/localias
that referenced
this pull request
May 12, 2024
…#31) ## Bugfix Fix #30 by changing how daemonization works. - Previously: `start`/`reload` would apply the latest config in the foreground process, then fork(), then start Caddy. - Now: `start`/`reload` apply the latest config, starts Caddy in the foreground process, then fork()s. - By starting Caddy in the foreground process, any activities requiring sudo/root will happen while the program is still interactive. - This fixes the problem (adding the root cert to the system store requires sudo/root privileges, but because Caddy was running in a background process, it couldn't request that permission.) - The foreground process exits cleanly and the background process starts Caddy again, this time without any need for sudo/root privileges because any of the work it needed to do was already done in the foreground process. ## Friendlier CLI This change encouraged me to make the following improvements to the CLI commands: - `stop` just kills the daemon, if it's running. If the daemon wasn't running, `stop` will now exit cleanly. Previously, it would throw an error, but that's user-hostile because the goal of running `stop` is to ensure no running daemon — if that goal is accomplished by stopping a running daemon, or confirming no daemon was running, doesn't really matter. - `start` will start a new daemon. If one was already running, it will be killed and replaced by a new one. The user's goal is to ensure a daemon is running, with the latest config, and this is achieved regardless of whether or not there was an existing daemon. - `reload` becomes an alias for `start`, because they have the exact same behavior — ensure that a daemon is running with the latest configuiration. ## Get rid of `caddymodules` A while ago, localias was built using `gomod2nix`, and there was an incompatibility between that helper and the opentelemetry modules included in Caddy. To work around this, I created a `caddymodules` package that imported all of the Caddy modules _except_ opentelemetry, which was fine because this project doesn't use the opentelemetry modules in any way. Because localias no longer uses `gomod2nix`, this PR gets rid of the `caddymodules` hack entirely. This then allowed me to upgrade the version of Caddy that is being installed, and it will make it easier to stay up to date as Caddy receives further improvements. ## SSL renewal server With an upgraded Caddy came a problem — for SSL issuance, Caddy now requires you to implement an "automation policy" server that confirms that it can issue a new certificate for a given domain. This is primarily aimed at issuing certificates for real life domains accessible to the public, not for internal development aliases, but the restriction still stands. To do this, I used Caddy itself to respond to these requests. For more information, read: - https://caddyserver.com/docs/automatic-https#on-demand-tls - https://caddy.community/t/serving-tens-of-thousands-of-domains-over-https-with-caddy/11179 - caddyserver/caddy#6055 In the future, I could implement this by writing a custom policy module instead of using the HTTP ask, but this works for now. ## Dependencies cleanup - General updates of all imported golang packages. - Update the `flake.nix` and `flake.lock` files, switch to `buildGoModule` instead of `buildGo120Module` to make it easier to use this flake with an override `nixpkgs` upstream.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5986
This makes the 'ask' endpoint a module, which means that developers can write custom plugins for granting permission for on-demand certificates.
Module namespace:
tls.permission.*The default HTTP permission module is the same code as the previous "ask" implementation.
This was a little more involved than I expected. 😅
/cc @ankon @danillouz