From 81fb7750b0f5db9ca26cf64a0b1dd5e4d0a03a52 Mon Sep 17 00:00:00 2001 From: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com> Date: Tue, 19 Oct 2021 14:11:47 -0700 Subject: [PATCH] [VAULT-3379] Add support for contained DBs in MSSQL root rotation and lease revocation (#12839) --- changelog/12839.txt | 3 ++ go.mod | 2 +- go.sum | 2 ++ plugins/database/mssql/mssql.go | 45 ++++++++++++++++++++++++---- plugins/database/mssql/mssql_test.go | 29 ++++++++++++++++++ 5 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 changelog/12839.txt diff --git a/changelog/12839.txt b/changelog/12839.txt new file mode 100644 index 0000000000000..f5919363eeba2 --- /dev/null +++ b/changelog/12839.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/database: Update MSSQL dependency github.com/denisenkom/go-mssqldb to v0.11.0 and include support for contained databases in MSSQL plugin +``` diff --git a/go.mod b/go.mod index 2038a92c79d8e..d99bd5332b342 100644 --- a/go.mod +++ b/go.mod @@ -37,7 +37,7 @@ require ( github.com/containerd/containerd v1.4.3 // indirect github.com/coreos/go-semver v0.3.0 github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf - github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc + github.com/denisenkom/go-mssqldb v0.11.0 github.com/docker/distribution v2.7.1+incompatible // indirect github.com/docker/docker v17.12.0-ce-rc1.0.20200309214505-aa6a9891b09c+incompatible github.com/docker/go-connections v0.4.0 diff --git a/go.sum b/go.sum index 2061970635cad..42c764e4ae39f 100644 --- a/go.sum +++ b/go.sum @@ -304,6 +304,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc h1:VRRKCwnzqk8QCaRC4os14xoKDdbHqqlJtJA0oc1ZAjg= github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc/go.mod h1:xbL0rPBG9cCiLr28tMa8zpbdarY27NDyej4t/EjAShU= +github.com/denisenkom/go-mssqldb v0.11.0 h1:9rHa233rhdOyrz2GcP9NM+gi2psgJZ4GWDpL/7ND8HI= +github.com/denisenkom/go-mssqldb v0.11.0/go.mod h1:xbL0rPBG9cCiLr28tMa8zpbdarY27NDyej4t/EjAShU= github.com/denverdino/aliyungo v0.0.0-20170926055100-d3308649c661 h1:lrWnAyy/F72MbxIxFUzKmcMCdt9Oi8RzpAxzTNQHD7o= github.com/denverdino/aliyungo v0.0.0-20170926055100-d3308649c661/go.mod h1:dV8lFg6daOBZbT6/BDGIz6Y3WFGn8juu6G+CQ6LHtl0= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= diff --git a/plugins/database/mssql/mssql.go b/plugins/database/mssql/mssql.go index 971d0e8a43977..38ae7b4a8c8ad 100644 --- a/plugins/database/mssql/mssql.go +++ b/plugins/database/mssql/mssql.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "strconv" "strings" _ "github.com/denisenkom/go-mssqldb" @@ -30,6 +31,9 @@ type MSSQL struct { *connutil.SQLConnectionProducer usernameProducer template.StringTemplate + + // A flag to let us know to skip cross DB queries and server login checks + containedDB bool } func New() (interface{}, error) { @@ -94,6 +98,20 @@ func (m *MSSQL) Initialize(ctx context.Context, req dbplugin.InitializeRequest) return dbplugin.InitializeResponse{}, fmt.Errorf("invalid username template - did you reference a field that isn't available? : %w", err) } + containedDB := false + containedDBRaw, err := strutil.GetString(req.Config, "contained_db") + if err != nil { + return dbplugin.InitializeResponse{}, fmt.Errorf("failed to retrieve contained_db: %w", err) + } + if containedDBRaw != "" { + containedDB, err = strconv.ParseBool(containedDBRaw) + if err != nil { + return dbplugin.InitializeResponse{}, fmt.Errorf("parsing error: incorrect boolean operator provided for contained_db: %w", err) + } + } + + m.containedDB = containedDB + resp := dbplugin.InitializeResponse{ Config: newConf, } @@ -201,6 +219,19 @@ func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error { return err } + // Check if DB is contained + if m.containedDB { + revokeStmt, err := db.PrepareContext(ctx, fmt.Sprintf("DROP USER IF EXISTS [%s]", username)) + if err != nil { + return err + } + defer revokeStmt.Close() + if _, err := revokeStmt.ExecContext(ctx); err != nil { + return err + } + return nil + } + // First disable server login disableStmt, err := db.PrepareContext(ctx, fmt.Sprintf("ALTER LOGIN [%s] DISABLE;", username)) if err != nil { @@ -311,7 +342,7 @@ func (m *MSSQL) UpdateUser(ctx context.Context, req dbplugin.UpdateUserRequest) func (m *MSSQL) updateUserPass(ctx context.Context, username string, changePass *dbplugin.ChangePassword) error { stmts := changePass.Statements.Commands - if len(stmts) == 0 { + if len(stmts) == 0 && !m.containedDB { stmts = []string{alterLoginSQL} } @@ -329,12 +360,16 @@ func (m *MSSQL) updateUserPass(ctx context.Context, username string, changePass return err } - var exists bool + // Since contained DB users do not have server logins, we + // only query for a login if DB is not a contained DB + if !m.containedDB { + var exists bool - err = db.QueryRowContext(ctx, "SELECT 1 FROM master.sys.server_principals where name = N'$1'", username).Scan(&exists) + err = db.QueryRowContext(ctx, "SELECT 1 FROM master.sys.server_principals where name = N'$1'", username).Scan(&exists) - if err != nil && err != sql.ErrNoRows { - return err + if err != nil && err != sql.ErrNoRows { + return err + } } tx, err := db.BeginTx(ctx, nil) diff --git a/plugins/database/mssql/mssql_test.go b/plugins/database/mssql/mssql_test.go index 6bd0df7ac1d48..7718946831ffd 100644 --- a/plugins/database/mssql/mssql_test.go +++ b/plugins/database/mssql/mssql_test.go @@ -42,6 +42,15 @@ func TestInitialize(t *testing.T) { VerifyConnection: true, }, }, + "contained_db set": { + dbplugin.InitializeRequest{ + Config: map[string]interface{}{ + "connection_url": connURL, + "contained_db": "true", + }, + VerifyConnection: true, + }, + }, } for name, test := range tests { @@ -265,6 +274,26 @@ func TestUpdateUser_password(t *testing.T) { } assertCredsExist(t, connURL, dbUser, test.expectedPassword) + + // Delete user at the end of each test + deleteReq := dbplugin.DeleteUserRequest{ + Username: dbUser, + } + + ctx, cancel = context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + deleteResp, err := db.DeleteUser(ctx, deleteReq) + if err != nil { + t.Fatalf("Failed to delete user: %s", err) + } + + // Protect against future fields that aren't specified + expectedDeleteResp := dbplugin.DeleteUserResponse{} + if !reflect.DeepEqual(deleteResp, expectedDeleteResp) { + t.Fatalf("Fields missing from expected response: Actual: %#v", deleteResp) + } + + assertCredsDoNotExist(t, connURL, dbUser, initPassword) }) } }