From 71e55c21e7ea0312d079539da88faeeeeb95a85a Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Mon, 14 Aug 2023 12:09:12 -0700 Subject: [PATCH] server/tailsql: add basic support for setec secrets (#5) Extend the DBSpec type to allow specifying a secret name for the connection string instead of a URL or file. Move URL resolution from the validation step to the open step, so that validation is cheap and idempotent. Add an Option to hold a setec.Store, and export the CheckSources method so the caller can find out which secrets they need to include. Enforce that a secret store is present during construction of the service, if any of the sources requires it. Add a test to exercise using the new secret field (requires tailscale/setec#36). Signed-off-by: M. J. Fromberger --- go.mod | 2 + go.sum | 4 ++ server/tailsql/internal_test.go | 2 +- server/tailsql/options.go | 91 ++++++++++++++++++++++++--------- server/tailsql/tailsql.go | 12 ++++- server/tailsql/tailsql_test.go | 52 +++++++++++++++++-- 6 files changed, 132 insertions(+), 31 deletions(-) diff --git a/go.mod b/go.mod index 1f01c52..01bd9c3 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/google/go-cmp v0.5.9 github.com/klauspost/compress v1.16.7 github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a + github.com/tailscale/setec v0.0.0-20230814184909-2dc1e0e620ea golang.org/x/exp v0.0.0-20230725093048-515e97ebf090 modernc.org/sqlite v1.25.0 tailscale.com v1.1.1-0.20230812230958-239899380412 @@ -72,6 +73,7 @@ require ( github.com/tailscale/netlink v1.1.1-0.20211101221916-cabfb018fe85 // indirect github.com/tailscale/wireguard-go v0.0.0-20230710185534-bb2c8f22eccf // indirect github.com/tcnksm/go-httpstat v0.2.0 // indirect + github.com/tink-crypto/tink-go/v2 v2.0.0 // indirect github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 // indirect github.com/vishvananda/netlink v1.2.1-beta.2 // indirect github.com/vishvananda/netns v0.0.4 // indirect diff --git a/go.sum b/go.sum index e908fae..59b11ce 100644 --- a/go.sum +++ b/go.sum @@ -204,10 +204,14 @@ github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a h1:SJy1Pu0eH1C29X github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a/go.mod h1:DFSS3NAGHthKo1gTlmEcSBiZrRJXi28rLNd/1udP1c8= github.com/tailscale/netlink v1.1.1-0.20211101221916-cabfb018fe85 h1:zrsUcqrG2uQSPhaUPjUQwozcRdDdSxxqhNgNZ3drZFk= github.com/tailscale/netlink v1.1.1-0.20211101221916-cabfb018fe85/go.mod h1:NzVQi3Mleb+qzq8VmcWpSkcSYxXIg0DkI6XDzpVkhJ0= +github.com/tailscale/setec v0.0.0-20230814184909-2dc1e0e620ea h1:mo0toDwzCNYTVuWPmDQyr/LOOFD2dT8CCNcEIYJx9es= +github.com/tailscale/setec v0.0.0-20230814184909-2dc1e0e620ea/go.mod h1:h9qIJN6MO8aKu+YGgjRFmuMQDld312U6POnzLLjqXA8= github.com/tailscale/wireguard-go v0.0.0-20230710185534-bb2c8f22eccf h1:bHQHwIHId353jAF2Lm0cGDjJpse/PYS0I0DTtihL9Ls= github.com/tailscale/wireguard-go v0.0.0-20230710185534-bb2c8f22eccf/go.mod h1:QRIcq2+DbdIC5sKh/gcAZhuqu6WT6L6G8/ALPN5wqYw= github.com/tcnksm/go-httpstat v0.2.0 h1:rP7T5e5U2HfmOBmZzGgGZjBQ5/GluWUylujl0tJ04I0= github.com/tcnksm/go-httpstat v0.2.0/go.mod h1:s3JVJFtQxtBEBC9dwcdTTXS9xFnM3SXAZwPG41aurT8= +github.com/tink-crypto/tink-go/v2 v2.0.0 h1:LutFJapahsM0i/6hKfOkzSYTVeshmFs+jloZXqe9z9s= +github.com/tink-crypto/tink-go/v2 v2.0.0/go.mod h1:QAbyq9LZncomYnScxlfaHImbV4ieNIe6bnu/Xcqqox4= github.com/u-root/u-root v0.11.0 h1:6gCZLOeRyevw7gbTwMj3fKxnr9+yHFlgF3N7udUVNO8= github.com/u-root/u-root v0.11.0/go.mod h1:DBkDtiZyONk9hzVEdB/PWI9B4TxDkElWlVTHseglrZY= github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 h1:YcojQL98T/OO+rybuzn2+5KrD5dBwXIvYBvQ2cD3Avg= diff --git a/server/tailsql/internal_test.go b/server/tailsql/internal_test.go index a034f26..c1675e9 100644 --- a/server/tailsql/internal_test.go +++ b/server/tailsql/internal_test.go @@ -81,7 +81,7 @@ func TestOptions(t *testing.T) { // Test that we can populate options from the config. t.Run("Options", func(t *testing.T) { - dbs, err := opts.sources() + dbs, err := opts.openSources(nil) if err != nil { t.Fatalf("Options: unexpected error: %v", err) } diff --git a/server/tailsql/options.go b/server/tailsql/options.go index b99d06e..3b0df4e 100644 --- a/server/tailsql/options.go +++ b/server/tailsql/options.go @@ -13,11 +13,11 @@ import ( "log" "os" "regexp" - "strings" "sync" "time" "github.com/tailscale/hujson" + "github.com/tailscale/setec/client/setec" "tailscale.com/client/tailscale/apitype" "tailscale.com/types/logger" ) @@ -71,6 +71,10 @@ type Options struct { // checks are performed, and all requests are accepted. Authorize func(src string, info *apitype.WhoIsResponse) error `json:"-"` + // If non-nil, use this store to fetch secret values. This is required if + // any of the sources specifies a named secret for its connection string. + SecretStore *setec.Store + // Optional rules to apply when rendering text for presentation in the UI. // After generating the value string, each rule is matched in order, and the // first match (if any) is applied to rewrite the output. The value returned @@ -81,26 +85,43 @@ type Options struct { Logf logger.Logf `json:"-"` } -// Construct database handles to serve queries from. This returns nil without -// error if no sources are defined. -func (o Options) sources() ([]*dbHandle, error) { +// openSources opens database handles to each of the sources defined by o. +// Sources that require secrets will get them from store. +// Precondition: All the sources of o have already been validated. +func (o Options) openSources(store *setec.Store) ([]*dbHandle, error) { if len(o.Sources) == 0 { return nil, nil } + srcs := make([]*dbHandle, len(o.Sources)) for i, spec := range o.Sources { - if err := spec.checkValid(); err != nil { - return nil, err - } else if spec.Label == "" { + if spec.Label == "" { spec.Label = "(unidentified database)" } - db, err := sql.Open(spec.Driver, spec.URL) + // Resolve the connection string. + var connString string + switch { + case spec.URL != "": + connString = spec.URL + case spec.KeyFile != "": + data, err := os.ReadFile(os.ExpandEnv(spec.KeyFile)) + if err != nil { + return nil, fmt.Errorf("read key file for %q: %w", spec.Source, err) + } + connString = string(data) + case spec.Secret != "": + connString = string(store.Secret(spec.Secret).Get()) + default: + panic("unexpected: no connection source is defined after validation") + } + + db, err := sql.Open(spec.Driver, connString) if err != nil { - return nil, fmt.Errorf("open %s %q: %w", spec.Driver, spec.URL, err) + return nil, fmt.Errorf("open %s %q: %w", spec.Driver, connString, err) } else if err := db.PingContext(context.Background()); err != nil { db.Close() - return nil, fmt.Errorf("ping %s %q: %w", spec.Driver, spec.URL, err) + return nil, fmt.Errorf("ping %s %q: %w", spec.Driver, connString, err) } srcs[i] = &dbHandle{ src: spec.Source, @@ -112,6 +133,21 @@ func (o Options) sources() ([]*dbHandle, error) { return srcs, nil } +// CheckSources validates the sources of o. If this succeeds, it also returns a +// slice of any secret names required by the specified sources, if any. +func (o Options) CheckSources() ([]string, error) { + var secrets []string + for i := range o.Sources { + if err := o.Sources[i].checkValid(); err != nil { + return nil, err + } + if s := o.Sources[i].Secret; s != "" { + secrets = append(secrets, s) + } + } + return secrets, nil +} + func (o Options) localState() (*localState, error) { if o.LocalState == "" { return nil, nil @@ -368,12 +404,28 @@ type DBSpec struct { Named map[string]string `json:"named,omitempty"` // Exactly one of the following fields must be set. - // If URL is set, it is used directly; otherwise KeyFile names the location - // of a file from which the connection string is read. - // The KeyFile, if set, will be expanded by os.ExpandEnv. + // + // If URL is set, it is used directly as the connection string. + // + // If KeyFile is set, it names the location of a file containing the + // connection string. If set, KeyFile is expanded by os.ExpandEnv. + // + // Otherwise, Secret is the name of a secret to fetch from the secrets + // service, whose value is the connection string. This requires that a + // secrets server be configured in the options. URL string `json:"url,omitempty"` // path or connection URL KeyFile string `json:"keyFile,omitempty"` // path to key file + Secret string `json:"secret,omitempty"` // name of secret +} + +func (d *DBSpec) countFields() (n int) { + for _, s := range []string{d.URL, d.KeyFile, d.Secret} { + if s != "" { + n++ + } + } + return } func (d *DBSpec) checkValid() error { @@ -382,17 +434,8 @@ func (d *DBSpec) checkValid() error { return errors.New("missing source") case d.Driver == "": return errors.New("missing driver name") - case d.URL == "" && d.KeyFile == "": - return errors.New("no URL or key file") - case d.URL != "" && d.KeyFile != "": - return errors.New("both URL and a key file are set") - } - if d.KeyFile != "" { - key, err := os.ReadFile(os.ExpandEnv(d.KeyFile)) - if err != nil { - return fmt.Errorf("read key file: %w", err) - } - d.URL = strings.TrimSpace(string(key)) + case d.countFields() != 1: + return errors.New("exactly one connection source must be set") } return nil } diff --git a/server/tailsql/tailsql.go b/server/tailsql/tailsql.go index 0a5bd77..0a670cf 100644 --- a/server/tailsql/tailsql.go +++ b/server/tailsql/tailsql.go @@ -118,7 +118,17 @@ type Server struct { // NewServer constructs a new server with the given Options. func NewServer(opts Options) (*Server, error) { - dbs, err := opts.sources() + // Check the validity of the sources, and get any secret names they require + // from the secrets service. If there are any, we also require that a + // secrets service URL is configured. + sec, err := opts.CheckSources() + if err != nil { + return nil, fmt.Errorf("checking sources: %w", err) + } else if len(sec) != 0 && opts.SecretStore == nil { + return nil, fmt.Errorf("have %d named secrets but no secret store", len(sec)) + } + + dbs, err := opts.openSources(opts.SecretStore) if err != nil { return nil, fmt.Errorf("opening sources: %w", err) } diff --git a/server/tailsql/tailsql_test.go b/server/tailsql/tailsql_test.go index 7381370..e8d746d 100644 --- a/server/tailsql/tailsql_test.go +++ b/server/tailsql/tailsql_test.go @@ -15,10 +15,13 @@ import ( "net/http" "net/http/httptest" "net/url" + "path/filepath" "regexp" "strings" "testing" + "github.com/tailscale/setec/client/setec" + "github.com/tailscale/setec/setectest" "github.com/tailscale/tailsql/authorizer" "github.com/tailscale/tailsql/server/tailsql" "github.com/tailscale/tailsql/uirules" @@ -34,9 +37,10 @@ import ( //go:embed testdata/init.sql var initSQL string -func mustInitSQLite(t *testing.T) *sql.DB { +func mustInitSQLite(t *testing.T) (url string, _ *sql.DB) { t.Helper() - db, err := sql.Open("sqlite", "file::memory:") + url = "file:" + filepath.Join(t.TempDir(), "test.db") + db, err := sql.Open("sqlite", url) if err != nil { t.Fatalf("Open memory db: %v", err) } @@ -45,7 +49,7 @@ func mustInitSQLite(t *testing.T) *sql.DB { if _, err := db.Exec(initSQL); err != nil { t.Fatalf("Initialize database: %v", err) } - return db + return url, db } func mustGetRequest(t *testing.T, url string, headers ...string) *http.Request { @@ -116,8 +120,46 @@ var testUIRules = []tailsql.UIRewriteRule{ uirules.FormatJSONText, } +func TestSecrets(t *testing.T) { + const secretName = "connection-string" + url, _ := mustInitSQLite(t) + db := setectest.NewDB(t, nil) + db.MustPut(db.Superuser, secretName, url) + + ss := setectest.NewServer(t, db, nil) + hs := httptest.NewServer(ss.Mux) + defer hs.Close() + + opts := tailsql.Options{ + Sources: []tailsql.DBSpec{{ + Source: "test", + Label: "Test Database", + Driver: "sqlite", + Secret: secretName, + }}, + } + secrets, err := opts.CheckSources() + if err != nil { + t.Fatalf("Invalid sources: %v", err) + } + st, err := setec.NewStore(context.Background(), setec.StoreConfig{ + Client: setec.Client{Server: hs.URL}, + Secrets: secrets, + }) + if err != nil { + t.Fatalf("Creating setec store: %v", err) + } + opts.SecretStore = st + + ts, err := tailsql.NewServer(opts) + if err != nil { + t.Fatalf("Creating tailsql server: %v", err) + } + ts.Close() +} + func TestServer(t *testing.T) { - db := mustInitSQLite(t) + _, db := mustInitSQLite(t) const testLabel = "hapax legomenon" const testAnchor = "wizboggle-gobsprocket" @@ -303,7 +345,7 @@ func TestAuth(t *testing.T) { } ) - db := mustInitSQLite(t) + _, db := mustInitSQLite(t) // An initially-empty fake client, which we will update between tests. fc := new(fakeClient)