Skip to content

Commit

Permalink
[VAULT-3379] Add support for contained DBs in MSSQL root rotation and…
Browse files Browse the repository at this point in the history
… lease revocation (#12839)
  • Loading branch information
vinay-gopalan committed Oct 19, 2021
1 parent 2bcd1c2 commit 81fb775
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 6 deletions.
3 changes: 3 additions & 0 deletions 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
```
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Expand Up @@ -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=
Expand Down
45 changes: 40 additions & 5 deletions plugins/database/mssql/mssql.go
Expand Up @@ -5,6 +5,7 @@ import (
"database/sql"
"errors"
"fmt"
"strconv"
"strings"

_ "github.com/denisenkom/go-mssqldb"
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}
}

Expand All @@ -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)
Expand Down
29 changes: 29 additions & 0 deletions plugins/database/mssql/mssql_test.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})
}
}
Expand Down

0 comments on commit 81fb775

Please sign in to comment.