Commit 8e75ae24 authored by Matthew Holt's avatar Matthew Holt

Only consume HTTP challenge for names we are solving for (closes #549)

If another ACME client is trying to solve a challenge for a name not
being served by Caddy on the same machine where Caddy is running, the
HTTP challenge will be consumed by Caddy rather than allowing the owner
to use the Caddyfile to proxy the challenge.

With this change, we only consume requests for HTTP challenges for
hostnames that we recognize. Before doing the challenge, we add the
name to a set, and when seeing if we should proxy the challenge, we
first check the path of course to see if it is an HTTP challenge;
if it is, we then check that set to see if the hostname is in the
set. Only if it is, do we consume it.

Otherwise, the request is treated like any other, allowing the owner
to configure a proxy for such requests to another ACME client.
parent dbd76f7a
......@@ -144,9 +144,11 @@ var newACMEClient = func(config *Config, allowPrompts bool) (*ACMEClient, error)
func (c *ACMEClient) Obtain(names []string) error {
Attempts:
for attempts := 0; attempts < 2; attempts++ {
namesObtaining.Add(names)
acmeMu.Lock()
certificate, failures := c.ObtainCertificate(names, true, nil)
acmeMu.Unlock()
namesObtaining.Remove(names)
if len(failures) > 0 {
// Error - try to fix it or report it to the user and abort
var errMsg string // we'll combine all the failures into a single error message
......@@ -294,3 +296,47 @@ func (c *ACMEClient) Revoke(name string) error {
return nil
}
// namesObtaining is a set of hostnames with thread-safe
// methods. A name should be in this set only while this
// package is in the process of obtaining a certificate
// for the name. ACME challenges that are received for
// names which are not in this set were not initiated by
// this package and probably should not be handled by
// this package.
var namesObtaining = nameCoordinator{names: make(map[string]struct{})}
type nameCoordinator struct {
names map[string]struct{}
mu sync.RWMutex
}
// Add adds names to c. It is safe for concurrent use.
func (c *nameCoordinator) Add(names []string) {
c.mu.Lock()
for _, name := range names {
c.names[strings.ToLower(name)] = struct{}{}
}
c.mu.Unlock()
}
// Remove removes names from c. It is safe for concurrent use.
func (c *nameCoordinator) Remove(names []string) {
c.mu.Lock()
for _, name := range names {
delete(c.names, strings.ToLower(name))
}
c.mu.Unlock()
}
// Has returns true if c has name. It is safe for concurrent use.
func (c *nameCoordinator) Has(name string) bool {
hostname, _, err := net.SplitHostPort(name)
if err != nil {
hostname = name
}
c.mu.RLock()
_, ok := c.names[strings.ToLower(hostname)]
c.mu.RUnlock()
return ok
}
......@@ -19,6 +19,9 @@ func HTTPChallengeHandler(w http.ResponseWriter, r *http.Request, altPort string
if !strings.HasPrefix(r.URL.Path, challengeBasePath) {
return false
}
if !namesObtaining.Has(r.Host) {
return false
}
scheme := "http"
if r.TLS != nil {
......
......@@ -8,13 +8,17 @@ import (
)
func TestHTTPChallengeHandlerNoOp(t *testing.T) {
// try base paths that aren't handled by this handler
namesObtaining.Add([]string{"localhost"})
// try base paths and host names that aren't
// handled by this handler
for _, url := range []string{
"http://localhost/",
"http://localhost/foo.html",
"http://localhost/.git",
"http://localhost/.well-known/",
"http://localhost/.well-known/acme-challenging",
"http://other/.well-known/acme-challenge/foo",
} {
req, err := http.NewRequest("GET", url, nil)
if err != nil {
......@@ -46,6 +50,9 @@ func TestHTTPChallengeHandlerSuccess(t *testing.T) {
}
ts.Listener = ln
// Tell this package that we are handling a challenge for 127.0.0.1
namesObtaining.Add([]string{"127.0.0.1"})
// Start our engines and run the test
ts.Start()
defer ts.Close()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment