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

Auto HTTPS changes for better wildcard cert config ergonomics #5447

Open
francislavoie opened this issue Mar 20, 2023 · 3 comments · May be fixed by #6146
Open

Auto HTTPS changes for better wildcard cert config ergonomics #5447

francislavoie opened this issue Mar 20, 2023 · 3 comments · May be fixed by #6146
Labels
discussion 💬 The right solution needs to be found
Milestone

Comments

@francislavoie
Copy link
Member

Followup on #3200, sparked by https://caddy.community/t/feature-request-reconsider-easier-wildcard-certificates-in-caddyfile/19348. See my reply on the forum question for context.

The gist is that we want to be careful with the amount of complexity we introduce. So we need to find a simple solution that doesn't complicate Automatic HTTPS too much.

My current thought is we could add an new option to auto_https called (tentatively) prefer_wildcard. What this would do is change how Auto HTTPS collects hostnames from top-level routes, and what TLS automation policies it creates from those hostnames.

Essentially, it would sort top-level hostnames according to specificity, then remove hostnames from the list if they're covered by another wildcard hostname in the list, and only make policies for the remainder.

So *.foo.com, bar.foo.com and baz.com would result in two policies, only *.foo.com and baz.com.

If a site uses tls, then it might still get its own automation policy (depending on the options used in the directive), so it would ignore using the wildcard cert.

⚠️ The problem is that the implementation might be complex and too difficult to reason about in code. This is mainly thinking about the config surface and a rough ideation of how it could work, but I haven't studied the code to figure out exactly if it fits yet. So this is marked as a discussion for now until I (or someone else if they want to tackle this) look into it further at the code level.

A Caddyfile config using this pattern could look like this, which is flatter than the current recommendation from https://caddyserver.com/docs/caddyfile/patterns#wildcard-certificates:

{
	auto_https prefer_wildcard
}

*.foo.com {
	tls {
		dns <provider>
	}

	# If a route reaches this site, then it wasn't
	# handled by another site block, so it's an unknown
	# subdomain, so you might close the connection.
	abort
}

foo.com {
	respond "FOO"
}

bar.foo.com {
	respond "BAR"
}

foobar.foo.com {
	# This would not use the wildcard cert
	tls /path/to/cert.crt /path/to/key.crt

	respond "FOOBAR"
}

barfoo.foo.com {
	# This would also cause the wildcard not to be used
	# because it would create a different automation policy
	tls {
		ca <ca_dir_url>
	}
	respond "FOOBAR"
}

baz.com {
	respond "BAZ
}
@francislavoie francislavoie added the discussion 💬 The right solution needs to be found label Mar 20, 2023
@francislavoie francislavoie added this to the 2.x milestone Mar 20, 2023
@chisaato
Copy link

How abount adding an int param to prefer_wildcard, and if the count of domains can be merged to one cert has meet the threshold, it will use wildcard cert. Otherwise issue per domain certs.

@francislavoie
Copy link
Member Author

@gzzchh that would necessarily require that the above proposal is accepted as-is since that's an addition to that.

But besides that, I'm not sure that makes sense. I don't think the amount of hostnames is a relevant metric. You must configure the DNS challenge to be able to use wildcard certs. At that point, what's the benefit of using individual domain certs at all for those domains?

Also it means there would be side effects from adding another domain to the config if it crosses the threshold, the other domains would suddenly stop using the cert they already had issued.

I don't think it's something users should need to consider the implications of, I don't think it brings any value.

@francislavoie
Copy link
Member Author

I have an implementation ready for review/testing #6146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion 💬 The right solution needs to be found
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants