Support for OnDemand even for non-wildcard domains

Howdy,

Thank you for Caddy. It’s a way sweet web server and the built-in support for SSL is totally amazing!

I run a small CMS site (https://telegr.am) and am looking to use Caddy to support ssl for every site that’s hosted.

The issue that I’ve run into is that if a user defines a URL that doesn’t currently have DNS pointing at the Caddy-hosted system, Caddy fails to get a cert and fails to start… and that’s a… well. non-starter. :slight_smile:

I’ve put together a patch that does two things: (1) if a domain, even a non-wildcard domain, is specified as OnDemand TLS, then the cert is only retrieved when the site is accessed; (2) added an explicit on_demand property to the tls configuration so one can specify OnDemand without max_certs.

This is my first foray into Go coding… although I’ve got a history of open source work… I founded the Lift web framework.

Given my lack of Go skills, I don’t know how to fork a GitHub repo and create a pull request that is also compatible with Go coding which seems to force one into a single branch, master, from the original author’s repo.

So…

I’m attaching the patch.

If someone could help me understand how to fork/create pull request, that’d be awesome.

If there’s any feedback on the patch, please let me know.

Thanks,

David

PS – If there’s anything I can do better to approach this community, I’m open to feedback. I appreciate the work of every open source community member and want to make sure I’m doing things to optimize for the community.

diff --git a/caddyhttp/httpserver/https.go b/caddyhttp/httpserver/https.go
index 1f5465e..eb480e2 100644
--- a/caddyhttp/httpserver/https.go
+++ b/caddyhttp/httpserver/https.go
@@ -23,9 +23,11 @@ func activateHTTPS(cctx caddy.Context) error {
 
        // place certificates and keys on disk
        for _, c := range ctx.siteConfigs {
-               err := c.TLS.ObtainCert(c.TLS.Hostname, operatorPresent)
-               if err != nil {
-                       return err
+               if !c.TLS.OnDemand {
+                       err := c.TLS.ObtainCert(c.TLS.Hostname, operatorPresent)
+                       if err != nil {
+                               return err
+                       }
                }
        }
 
@@ -73,7 +75,7 @@ func markQualifiedForAutoHTTPS(configs []*SiteConfig) {
 // the returned error value will always be nil.
 func enableAutoHTTPS(configs []*SiteConfig, loadCertificates bool) error {
        for _, cfg := range configs {
-               if cfg == nil || cfg.TLS == nil || !cfg.TLS.Managed {
+               if cfg == nil || cfg.TLS == nil || !cfg.TLS.Managed || cfg.TLS.OnDemand {
                        continue
                }
                cfg.TLS.Enabled = true
diff --git a/caddytls/setup.go b/caddytls/setup.go
index d789674..171e9a7 100644
--- a/caddytls/setup.go
+++ b/caddytls/setup.go
@@ -35,7 +35,7 @@ func setupTLS(c *caddy.Controller) error {
        config.Enabled = true
 
        for c.Next() {
-               var certificateFile, keyFile, loadDir, maxCerts string
+               var certificateFile, keyFile, loadDir, onDemand, maxCerts string
 
                args := c.RemainingArgs()
                switch len(args) {
@@ -144,6 +144,8 @@ func setupTLS(c *caddy.Controller) error {
                        case "max_certs":
                                c.Args(&maxCerts)
                                config.OnDemand = true
+                       case "on_demand":
+                               c.Args(&onDemand)
                        case "dns":
                                args := c.RemainingArgs()
                                if len(args) != 1 {
@@ -176,6 +178,15 @@ func setupTLS(c *caddy.Controller) error {
                        return c.ArgErr()
                }
 
+               if onDemand != "" {
+                       od, err := strconv.ParseBool(onDemand)
+                       config.OnDemand = od
+                       if err != nil {
+                               return c.Err("on_demand must be a bool")
+                       }
+                       config.OnDemand = od
+               }
+
                // set certificate limit if on-demand TLS is enabled
                if maxCerts != "" {
                        maxCertsNum, err := strconv.Atoi(maxCerts) 

Hey David, welcome! Nice job diving into this. Not bad for a new Go programmer. :wink:

If I understand the problem statement correctly, you might be able to work around this behavior by using a wildcard domain for each site in your Caddyfile:

*.usersite1.com {
    tls {
       max_certs 1
    }
}
*.usersite2.com {
    tls {
       max_certs 1
    }
}

I don’t know if that’s allowable in your situation, but if it won’t hurt, I think this would be better than the patch. Here’s why.

I think my suggestion above should work around this, and…

Enabling on-demand issuance without any caps is dangerous, since certificates take up space on disk and someone could DoS your service just by making bogus handshakes all day long.

Also capping the max_certs to 1 as I’ve done in my example above assumes that the first handshake is for the correct hostname, so you hope it comes in first (!!) before any other bogus handshakes do.

Note the max_certs count is reset when Caddy is restarted, but if a valid certificate is already on disk, Caddy will use that instead of getting a new one.

We just have to be really careful changing any of the on-demand logic.

So, I know you’re not gonna like this, but here’s the best solution to the problem: have the domain point at the machine before starting Caddy. Or – I don’t recommend this unless you implement this carefully – you can disable TLS for their site with tls off until they point their domain to your server. But turn TLS on as soon as they do, of course. :slight_smile:

This is a great start. I usually like to direct these specific dev-related discussions into issues on GitHub so we can track it better. You can take a gander at the contributing guidelines if you find that helpful. Would love to have you a part of the community. :+1:

Yeah, this is kind of an annoying aspect of Go OSS development. There is this one weird trick, though, that allows you to do it quite easily, by manipulating your git remotes: How to contribute to a Go open source git repo

2 Likes

Thanks!

That works for www.athena.com, but not for athena.com… at least my tests against 0.9.3 indicated that wildcarding did not correctly pick up the base domain.

Sadly, that solution doesn’t work. It would mean checking for the hundreds of domains that may or may not be pointing at the server each time the domain table is updated… which could be hundreds of times a day.

I think the best bet for me is to maintain a fork with the feature I need.

Thanks for the great work and I look forward to continuing to contribute to the Caddy community.

David

Correct; you’d have to use *.athena.com for athena.com, *.www.athena.com for www.athena.com, etc.

I’m sorry about that… I don’t know a better way right now.

Good luck, but be aware of the risks. And again, nice work diving into the code.

Sadly, the wildcarding does not work as expects. For example, with the following Caddyfile:

*.much4.us {
 tls {
  max_certs 10
}
 root /home/dpp/caddy
}

If you go to http://much4.us , you get: No such site at :80

So, I had to figure a solution that (1) allowed wildcard and base domains to be served correctly including a redirect from http to https; (2) allowed for DNS that was not configured correctly by my users; (3) not causing Caddy to either fail to load or fail to reload.

That solution was to make OnDemand true for both wildcard and non-wildcard domains.

So, I will stick with a fork.

Thanks!

David

Apologies for not addressing your other points in your post, but in particular, this is the result of an incomplete Caddyfile - the wildcarding is indeed working as intended. http://much4.us does not match the host *.much4.us which will only match requests that have a subdomain. To catch the bare domain as well, you’ll need to specify that too.

I believe you should be able to do that with the following, untested:

*.much4.us, much4.us {
    root /home/dpp/caddy
    tls {
        max_certs 10
    }
}
1 Like

You’re right, that’s me not thinking through very clearly… had my mind on a lot of other things. Sorry. Fork is probably the way to go for what you want to do.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.