15000: Export safe parts of cluster config at /arvados/v1/config.
authorTom Clegg <tclegg@veritasgenetics.com>
Thu, 6 Jun 2019 14:54:26 +0000 (10:54 -0400)
committerTom Clegg <tclegg@veritasgenetics.com>
Fri, 7 Jun 2019 16:38:07 +0000 (12:38 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

lib/config/cmd.go
lib/config/cmd_test.go
lib/config/export.go [new file with mode: 0644]
lib/config/export_test.go [new file with mode: 0644]
lib/controller/handler.go
lib/controller/handler_test.go

index 41a1d7d2143483b2fc67f647ab8e24492e037420..39df4ec170c4ef13f89ef778b43eb67a1e1e675c 100644 (file)
@@ -12,12 +12,11 @@ import (
        "os"
        "os/exec"
 
-       "git.curoverse.com/arvados.git/lib/cmd"
        "git.curoverse.com/arvados.git/sdk/go/ctxlog"
        "github.com/ghodss/yaml"
 )
 
-var DumpCommand cmd.Handler = dumpCommand{}
+var DumpCommand dumpCommand
 
 type dumpCommand struct{}
 
@@ -48,7 +47,7 @@ func (dumpCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou
        return 0
 }
 
-var CheckCommand cmd.Handler = checkCommand{}
+var CheckCommand checkCommand
 
 type checkCommand struct{}
 
index e4d838f85ac26a1610012f279126d42e754c4a00..31a7544939e02fb235113cb722398af304d1b812 100644 (file)
@@ -7,11 +7,18 @@ package config
 import (
        "bytes"
 
+       "git.curoverse.com/arvados.git/lib/cmd"
        check "gopkg.in/check.v1"
 )
 
 var _ = check.Suite(&CommandSuite{})
 
+var (
+       // Commands must satisfy cmd.Handler interface
+       _ cmd.Handler = dumpCommand{}
+       _ cmd.Handler = checkCommand{}
+)
+
 type CommandSuite struct{}
 
 func (s *CommandSuite) TestBadArg(c *check.C) {
diff --git a/lib/config/export.go b/lib/config/export.go
new file mode 100644 (file)
index 0000000..98263c2
--- /dev/null
@@ -0,0 +1,144 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package config
+
+import (
+       "encoding/json"
+       "errors"
+       "fmt"
+       "io"
+       "strings"
+
+       "git.curoverse.com/arvados.git/sdk/go/arvados"
+)
+
+// ExportJSON writes a JSON object with the safe (non-secret) portions
+// of the cluster config to w.
+func ExportJSON(w io.Writer, cluster *arvados.Cluster) error {
+       buf, err := json.Marshal(cluster)
+       if err != nil {
+               return err
+       }
+       var m map[string]interface{}
+       err = json.Unmarshal(buf, &m)
+       if err != nil {
+               return err
+       }
+       err = redactUnsafe(m, "", "")
+       if err != nil {
+               return err
+       }
+       return json.NewEncoder(w).Encode(m)
+}
+
+// whitelist classifies configs as safe/unsafe to reveal to
+// unauthenticated clients.
+//
+// Every config entry must either be listed explicitly here along with
+// all of its parent keys (e.g., "API" + "API.RequestTimeout"), or
+// have an ancestor listed as false (e.g.,
+// "PostgreSQL.Connection.password" has an ancestor
+// "PostgreSQL.Connection" with a false value). Otherwise, it is a bug
+// which should be caught by tests.
+//
+// Example: API.RequestTimeout is safe because whitelist["API"] == and
+// whitelist["API.RequestTimeout"] == true.
+//
+// Example: PostgreSQL.Connection.password is not safe because
+// whitelist["PostgreSQL.Connection"] == false.
+//
+// Example: PostgreSQL.BadKey would cause an error because
+// whitelist["PostgreSQL"] isn't false, and neither
+// whitelist["PostgreSQL.BadKey"] nor whitelist["PostgreSQL.*"]
+// exists.
+var whitelist = map[string]bool{
+       // | sort -t'"' -k2,2
+       "API":                                      true,
+       "API.MaxItemsPerResponse":                  true,
+       "API.MaxRequestAmplification":              true,
+       "API.RequestTimeout":                       true,
+       "Containers":                               true,
+       "Containers.CloudVMs":                      true,
+       "Containers.CloudVMs.BootProbeCommand":     true,
+       "Containers.CloudVMs.Driver":               true,
+       "Containers.CloudVMs.DriverParameters":     false,
+       "Containers.CloudVMs.Enable":               true,
+       "Containers.CloudVMs.ImageID":              true,
+       "Containers.CloudVMs.MaxCloudOpsPerSecond": true,
+       "Containers.CloudVMs.MaxProbesPerSecond":   true,
+       "Containers.CloudVMs.PollInterval":         true,
+       "Containers.CloudVMs.ProbeInterval":        true,
+       "Containers.CloudVMs.ResourceTags":         true,
+       "Containers.CloudVMs.ResourceTags.*":       true,
+       "Containers.CloudVMs.SSHPort":              true,
+       "Containers.CloudVMs.SyncInterval":         true,
+       "Containers.CloudVMs.TagKeyPrefix":         true,
+       "Containers.CloudVMs.TimeoutBooting":       true,
+       "Containers.CloudVMs.TimeoutIdle":          true,
+       "Containers.CloudVMs.TimeoutProbe":         true,
+       "Containers.CloudVMs.TimeoutShutdown":      true,
+       "Containers.CloudVMs.TimeoutSignal":        true,
+       "Containers.CloudVMs.TimeoutTERM":          true,
+       "Containers.DispatchPrivateKey":            true,
+       "Containers.StaleLockTimeout":              true,
+       "InstanceTypes":                            true,
+       "InstanceTypes.*":                          true,
+       "InstanceTypes.*.*":                        true,
+       "ManagementToken":                          false,
+       "PostgreSQL":                               true,
+       "PostgreSQL.Connection":                    false,
+       "PostgreSQL.ConnectionPool":                true,
+       "RemoteClusters":                           true,
+       "RemoteClusters.*":                         true,
+       "RemoteClusters.*.Host":                    true,
+       "RemoteClusters.*.Insecure":                true,
+       "RemoteClusters.*.Proxy":                   true,
+       "RemoteClusters.*.Scheme":                  true,
+       "Services":                                 true,
+       "Services.*":                               true,
+       "Services.*.ExternalURL":                   true,
+       "Services.*.InternalURLs":                  true,
+       "Services.*.InternalURLs.*":                true,
+       "Services.*.InternalURLs.*.*":              true,
+       "SystemLogs":                               true,
+       "SystemLogs.Format":                        true,
+       "SystemLogs.LogLevel":                      true,
+       "SystemLogs.MaxRequestLogParamsSize":       true,
+       "SystemRootToken":                          false,
+       "TLS":                                      true,
+       "TLS.Certificate":                          true,
+       "TLS.Insecure":                             true,
+       "TLS.Key":                                  false,
+}
+
+func redactUnsafe(m map[string]interface{}, mPrefix, lookupPrefix string) error {
+       var errs []string
+       for k, v := range m {
+               lookupKey := k
+               safe, ok := whitelist[lookupPrefix+k]
+               if !ok {
+                       lookupKey = "*"
+                       safe, ok = whitelist[lookupPrefix+"*"]
+               }
+               if !ok {
+                       errs = append(errs, fmt.Sprintf("config bug: key %q not in whitelist map", lookupPrefix+k))
+                       continue
+               }
+               if !safe {
+                       delete(m, k)
+                       continue
+               }
+               if v, ok := v.(map[string]interface{}); ok {
+                       err := redactUnsafe(v, mPrefix+k+".", lookupPrefix+lookupKey+".")
+                       if err != nil {
+                               errs = append(errs, err.Error())
+                       }
+               }
+       }
+       if len(errs) > 0 {
+               return errors.New(strings.Join(errs, "\n"))
+       }
+       return nil
+}
diff --git a/lib/config/export_test.go b/lib/config/export_test.go
new file mode 100644 (file)
index 0000000..581e54c
--- /dev/null
@@ -0,0 +1,37 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package config
+
+import (
+       "bytes"
+       "regexp"
+       "strings"
+
+       "git.curoverse.com/arvados.git/sdk/go/ctxlog"
+       check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&ExportSuite{})
+
+type ExportSuite struct{}
+
+func (s *ExportSuite) TestExport(c *check.C) {
+       confdata := bytes.Replace(DefaultYAML, []byte("SAMPLE"), []byte("testkey"), -1)
+       cfg, err := Load(bytes.NewBuffer(confdata), ctxlog.TestLogger(c))
+       c.Assert(err, check.IsNil)
+       cluster := cfg.Clusters["xxxxx"]
+       cluster.ManagementToken = "abcdefg"
+
+       var exported bytes.Buffer
+       err = ExportJSON(&exported, &cluster)
+       c.Check(err, check.IsNil)
+       if err != nil {
+               c.Logf("If all the new keys are safe, add these to whitelist in export.go:")
+               for _, k := range regexp.MustCompile(`"[^"]*"`).FindAllString(err.Error(), -1) {
+                       c.Logf("\t%q: true,", strings.Replace(k, `"`, "", -1))
+               }
+       }
+       c.Check(exported.String(), check.Not(check.Matches), `(?ms).*abcdefg.*`)
+}
index 2c3ce1d4f28d189e956cd3e120b8433214861619..12faacdd4398211f8466a4ed7e971283190b9871 100644 (file)
@@ -5,16 +5,19 @@
 package controller
 
 import (
+       "bytes"
        "context"
        "database/sql"
        "errors"
        "fmt"
+       "io"
        "net/http"
        "net/url"
        "strings"
        "sync"
        "time"
 
+       "git.curoverse.com/arvados.git/lib/config"
        "git.curoverse.com/arvados.git/sdk/go/arvados"
        "git.curoverse.com/arvados.git/sdk/go/health"
        "git.curoverse.com/arvados.git/sdk/go/httpserver"
@@ -73,6 +76,18 @@ func (h *Handler) setup() {
                Prefix: "/_health/",
                Routes: health.Routes{"ping": func() error { _, err := h.db(&http.Request{}); return err }},
        })
+
+       mux.Handle("/arvados/v1/config", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+               var buf bytes.Buffer
+               err := config.ExportJSON(&buf, h.Cluster)
+               if err != nil {
+                       httpserver.Error(w, err.Error(), http.StatusInternalServerError)
+                       return
+               }
+               w.Header().Set("Content-Type", "application/json")
+               io.Copy(w, &buf)
+       }))
+
        hs := http.NotFoundHandler()
        hs = prepend(hs, h.proxyRailsAPI)
        hs = h.setupProxyRemoteCluster(hs)
index a1efaacddff5b2b7c52ad8fd78eb79c0500b2be8..06a5a84113dd36222a38f9e8cd971f0f46eac0f7 100644 (file)
@@ -53,6 +53,23 @@ func (s *HandlerSuite) TearDownTest(c *check.C) {
        s.cancel()
 }
 
+func (s *HandlerSuite) TestConfigExport(c *check.C) {
+       s.cluster.Containers.CloudVMs.PollInterval = arvados.Duration(23 * time.Second)
+       req := httptest.NewRequest("GET", "/arvados/v1/config", nil)
+       resp := httptest.NewRecorder()
+       s.handler.ServeHTTP(resp, req)
+       c.Check(resp.Code, check.Equals, http.StatusOK)
+       var cluster arvados.Cluster
+       c.Log(resp.Body.String())
+       err := json.Unmarshal(resp.Body.Bytes(), &cluster)
+       c.Check(err, check.IsNil)
+       c.Check(cluster.ManagementToken, check.Equals, "")
+       c.Check(cluster.SystemRootToken, check.Equals, "")
+       c.Check(cluster.TLS.Insecure, check.Equals, true)
+       c.Check(cluster.Services.RailsAPI, check.DeepEquals, s.cluster.Services.RailsAPI)
+       c.Check(cluster.Containers.CloudVMs.PollInterval, check.Equals, arvados.Duration(23*time.Second))
+}
+
 func (s *HandlerSuite) TestProxyDiscoveryDoc(c *check.C) {
        req := httptest.NewRequest("GET", "/discovery/v1/apis/arvados/v1/rest", nil)
        resp := httptest.NewRecorder()