From f259ed52bb3764ce4fd5d88f1712cb43247c2639 Mon Sep 17 00:00:00 2001 From: jhwz <52683873+jhwz@users.noreply.github.com> Date: Thu, 7 Jul 2022 07:50:07 +1200 Subject: [PATCH] admin: support ETag on config endpoints (#4579) * admin: support ETags * support etags Co-authored-by: Matt Holt --- admin.go | 21 +++++++++++++++++++-- admin_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- caddy.go | 38 +++++++++++++++++++++++++++++++++++--- 3 files changed, 104 insertions(+), 6 deletions(-) diff --git a/admin.go b/admin.go index 214b734d8..e37caf83a 100644 --- a/admin.go +++ b/admin.go @@ -21,10 +21,13 @@ import ( "crypto/tls" "crypto/x509" "encoding/base64" + "encoding/hex" "encoding/json" "errors" "expvar" "fmt" + "hash" + "hash/fnv" "io" "net" "net/http" @@ -894,16 +897,30 @@ func (h adminHandler) originAllowed(origin *url.URL) bool { return false } +// etagHasher returns a the hasher we used on the config to both +// produce and verify ETags. +func etagHasher() hash.Hash32 { return fnv.New32a() } + func handleConfig(w http.ResponseWriter, r *http.Request) error { switch r.Method { case http.MethodGet: w.Header().Set("Content-Type", "application/json") + // Set the ETag as a trailer header. + // The alternative is to write the config to a buffer, and + // then hash that. + w.Header().Set("Trailer", "ETag") - err := readConfig(r.URL.Path, w) + hash := etagHasher() + configWriter := io.MultiWriter(w, hash) + err := readConfig(r.URL.Path, configWriter) if err != nil { return APIError{HTTPStatus: http.StatusBadRequest, Err: err} } + // we could consider setting up a sync.Pool for the summed + // hashes to reduce GC pressure. + w.Header().Set("ETag", r.URL.Path+" "+hex.EncodeToString(hash.Sum(nil))) + return nil case http.MethodPost, @@ -937,7 +954,7 @@ func handleConfig(w http.ResponseWriter, r *http.Request) error { forceReload := r.Header.Get("Cache-Control") == "must-revalidate" - err := changeConfig(r.Method, r.URL.Path, body, forceReload) + err := changeConfig(r.Method, r.URL.Path, body, r.Header.Get("If-Match"), forceReload) if err != nil && !errors.Is(err, errSameConfig) { return err } diff --git a/admin_test.go b/admin_test.go index 608a32c67..32f20c627 100644 --- a/admin_test.go +++ b/admin_test.go @@ -15,7 +15,9 @@ package caddy import ( + "encoding/hex" "encoding/json" + "net/http" "reflect" "sync" "testing" @@ -139,10 +141,57 @@ func TestLoadConcurrent(t *testing.T) { wg.Done() }() } - wg.Wait() } +type fooModule struct { + IntField int + StrField string +} + +func (fooModule) CaddyModule() ModuleInfo { + return ModuleInfo{ + ID: "foo", + New: func() Module { return new(fooModule) }, + } +} +func (fooModule) Start() error { return nil } +func (fooModule) Stop() error { return nil } + +func TestETags(t *testing.T) { + RegisterModule(fooModule{}) + + if err := Load([]byte(`{"apps": {"foo": {"strField": "abc", "intField": 0}}}`), true); err != nil { + t.Fatalf("loading: %s", err) + } + + const key = "/" + rawConfigKey + "/apps/foo" + + // try update the config with the wrong etag + err := changeConfig(http.MethodPost, key, []byte(`{"strField": "abc", "intField": 1}}`), "/"+rawConfigKey+" not_an_etag", false) + if apiErr, ok := err.(APIError); !ok || apiErr.HTTPStatus != http.StatusPreconditionFailed { + t.Fatalf("expected precondition failed; got %v", err) + } + + // get the etag + hash := etagHasher() + if err := readConfig(key, hash); err != nil { + t.Fatalf("reading: %s", err) + } + + // do the same update with the correct key + err = changeConfig(http.MethodPost, key, []byte(`{"strField": "abc", "intField": 1}`), key+" "+hex.EncodeToString(hash.Sum(nil)), false) + if err != nil { + t.Fatalf("expected update to work; got %v", err) + } + + // now try another update. The hash should no longer match and we should get precondition failed + err = changeConfig(http.MethodPost, key, []byte(`{"strField": "abc", "intField": 2}`), key+" "+hex.EncodeToString(hash.Sum(nil)), false) + if apiErr, ok := err.(APIError); !ok || apiErr.HTTPStatus != http.StatusPreconditionFailed { + t.Fatalf("expected precondition failed; got %v", err) + } +} + func BenchmarkLoad(b *testing.B) { for i := 0; i < b.N; i++ { Load(testCfg, true) diff --git a/caddy.go b/caddy.go index d15509228..0c6dfcd00 100644 --- a/caddy.go +++ b/caddy.go @@ -17,6 +17,7 @@ package caddy import ( "bytes" "context" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -111,7 +112,7 @@ func Load(cfgJSON []byte, forceReload bool) error { } }() - err := changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, forceReload) + err := changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, "", forceReload) if errors.Is(err, errSameConfig) { err = nil // not really an error } @@ -125,7 +126,12 @@ func Load(cfgJSON []byte, forceReload bool) error { // occur unless forceReload is true. If the config is unchanged and not // forcefully reloaded, then errConfigUnchanged This function is safe for // concurrent use. -func changeConfig(method, path string, input []byte, forceReload bool) error { +// The ifMatchHeader can optionally be given a string of the format: +// " " +// where is the absolute path in the config and is the expected hash of +// the config at that path. If the hash in the ifMatchHeader doesn't match +// the hash of the config, then an APIError with status 412 will be returned. +func changeConfig(method, path string, input []byte, ifMatchHeader string, forceReload bool) error { switch method { case http.MethodGet, http.MethodHead, @@ -138,6 +144,32 @@ func changeConfig(method, path string, input []byte, forceReload bool) error { currentCfgMu.Lock() defer currentCfgMu.Unlock() + if ifMatchHeader != "" { + // read out the parts + parts := strings.Fields(ifMatchHeader) + if len(parts) != 2 { + return APIError{ + HTTPStatus: http.StatusBadRequest, + Err: fmt.Errorf("malformed If-Match header; expect format \" \""), + } + } + + // get the current hash of the config + // at the given path + hash := etagHasher() + err := unsyncedConfigAccess(http.MethodGet, parts[0], nil, hash) + if err != nil { + return err + } + + if hex.EncodeToString(hash.Sum(nil)) != parts[1] { + return APIError{ + HTTPStatus: http.StatusPreconditionFailed, + Err: fmt.Errorf("If-Match header did not match current config hash"), + } + } + } + err := unsyncedConfigAccess(method, path, input, nil) if err != nil { return err @@ -500,7 +532,7 @@ func finishSettingUp(ctx Context, cfg *Config) error { runLoadedConfig := func(config []byte) error { logger.Info("applying dynamically-loaded config") - err := changeConfig(http.MethodPost, "/"+rawConfigKey, config, false) + err := changeConfig(http.MethodPost, "/"+rawConfigKey, config, "", false) if errors.Is(err, errSameConfig) { return err }