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 <fromberger@tailscale.com>

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

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=

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)
 		}

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
 }

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)
 	}

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)