backport/vault-1.16: honor static role ttl across restarts (#142)

* backport/1.16: honor static role ttl across restarts

* update deps

.go-version

@@ -1 +1 @@
-1.23.4
+1.23.6

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Unreleased
 
+## v0.14.4
+
 BUG FIXES:
 
 * Update static role rotation to generate a new password after 2 failed attempts (https://github.com/hashicorp/vault-plugin-secrets-openldap/pull/125)
@@ -59,6 +61,27 @@ IMPROVEMENTS:
    * `github.com/hashicorp/vault/sdk` v0.11.1-0.20240325190132-c20eae3e84c5 -> v0.12.0
    * `github.com/stretchr/testify` v1.8.4 -> v1.9.0
 
+
+## v0.12.4
+
+BUG FIXES:
+
+* Fix a bug where static role passwords are erroneously rotated across backend restarts when using skip import rotation. (https://github.com/hashicorp/vault-plugin-secrets-openldap/pull/140)
+
+## v0.12.3
+
+BUG FIXES:
+
+* Update static role rotation to generate a new password after 2 failed attempts (https://github.com/hashicorp/vault-plugin-secrets-openldap/pull/125)
+
+IMPROVEMENTS:
+* Updated dependencies (https://github.com/hashicorp/vault-plugin-secrets-openldap/pull/133):
+  * `github.com/docker/docker` v24.0.9+incompatible -> v25.0.6+incompatible
+  * `github.com/hashicorp/go-retryablehttp` v0.7.1 -> v0.7.7
+  * `github.com/go-jose/go-jose/v3` v3.0.1 -> v3.0.3
+  * `golang.org/x/net` v0.17.0 -> v0.28.0
+  * `google.golang.org/protobuf` v1.30.0 ->  v1.34.2
+
 ## v0.12.2
 
 ### BUG FIXES:

backend_test.go

@@ -26,8 +26,43 @@ var (
 	testPasswordFromPolicy2 = "TestPolicy2Password"
 )
 
+// getBackend returns an initialized test backend with InmemStorage
 func getBackend(throwsErr bool) (*backend, logical.Storage) {
-	config := &logical.BackendConfig{
+	b, config := getBackendWithConfig(testBackendConfig(), throwsErr)
+	return b, config.StorageView
+}
+
+// getBackendWithConfig returns an initialized test backend for the given
+// config
+func getBackendWithConfig(c *logical.BackendConfig, throwsErr bool) (*backend, *logical.BackendConfig) {
+	b := Backend(&fakeLdapClient{throwErrs: throwsErr})
+	b.Setup(context.Background(), c)
+
+	b.credRotationQueue = queue.New()
+	// Create a context with a cancel method for processing any WAL entries and
+	// populating the queue
+	initCtx := context.Background()
+	ictx, cancel := context.WithCancel(initCtx)
+	b.cancelQueue = cancel
+
+	// Load managed LDAP users into memory from storage
+	err := b.loadManagedUsers(ictx, c.StorageView)
+	if err != nil {
+		// TODO: make this fatal? Requires refactoring all tests to pass in a testing.T
+		b.Logger().Error("error configuring backend: could not read roles from storage")
+	}
+
+	// Load queue and kickoff new periodic ticker
+	b.initQueue(ictx, &logical.InitializationRequest{
+		Storage: c.StorageView,
+	})
+
+	return b, c
+}
+
+// testBackendConfig returns a backend config with inmem storage
+func testBackendConfig() *logical.BackendConfig {
+	return &logical.BackendConfig{
 		Logger: logging.NewVaultLogger(log.Debug),
 
 		System: &logical.StaticSystemView{
@@ -44,23 +79,6 @@ func getBackend(throwsErr bool) (*backend, logical.Storage) {
 		},
 		StorageView: &logical.InmemStorage{},
 	}
-
-	b := Backend(&fakeLdapClient{throwErrs: throwsErr})
-	b.Setup(context.Background(), config)
-
-	b.credRotationQueue = queue.New()
-	// Create a context with a cancel method for processing any WAL entries and
-	// populating the queue
-	initCtx := context.Background()
-	ictx, cancel := context.WithCancel(initCtx)
-	b.cancelQueue = cancel
-
-	// Load queue and kickoff new periodic ticker
-	b.initQueue(ictx, &logical.InitializationRequest{
-		Storage: config.StorageView,
-	})
-
-	return b, config.StorageView
 }
 
 var _ ldapClient = (*fakeLdapClient)(nil)

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/ory/dockertest/v3 v3.10.0
 	github.com/stretchr/testify v1.8.4
-	golang.org/x/text v0.17.0
+	golang.org/x/text v0.21.0
 )
 
 require (
@@ -79,11 +79,11 @@ require (
 	github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
 	github.com/xeipuuv/gojsonschema v1.2.0 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
-	golang.org/x/crypto v0.26.0 // indirect
+	golang.org/x/crypto v0.31.0 // indirect
 	golang.org/x/mod v0.17.0 // indirect
-	golang.org/x/net v0.28.0 // indirect
-	golang.org/x/sync v0.8.0 // indirect
-	golang.org/x/sys v0.23.0 // indirect
+	golang.org/x/net v0.33.0 // indirect
+	golang.org/x/sync v0.10.0 // indirect
+	golang.org/x/sys v0.28.0 // indirect
 	golang.org/x/time v0.0.0-20220411224347-583f2d630306 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect

go.sum

@@ -280,8 +280,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
 golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
-golang.org/x/crypto v0.26.0 h1:RrRspgV4mU+YwB4FYnuBoKsUapNIL5cohGAmSH3azsw=
-golang.org/x/crypto v0.26.0/go.mod h1:GY7jblb9wI+FOo5y8/S2oY4zWP07AkOJ4+jxCqdqn54=
+golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
+golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
@@ -299,17 +299,17 @@ golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qx
 golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
 golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
 golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
-golang.org/x/net v0.28.0 h1:a9JDOJc5GMUJ0+UDqmLT86WiEy7iWyIhz8gz8E4e5hE=
-golang.org/x/net v0.28.0/go.mod h1:yqtgsTWOOnlGLG9GFRrK3++bGOUEkNBoHZc8MEDWPNg=
+golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
+golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
-golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
+golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
@@ -335,8 +335,8 @@ golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-golang.org/x/sys v0.23.0 h1:YfKFowiIMvtgl1UERQoTPPToxltDeZfbj4H7dVUCwmM=
-golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
+golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
@@ -349,8 +349,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
 golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
 golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
 golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
-golang.org/x/text v0.17.0 h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc=
-golang.org/x/text v0.17.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY=
+golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
+golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
 golang.org/x/time v0.0.0-20220411224347-583f2d630306 h1:+gHMid33q6pen7kv9xvT+JRinntgeXO2AeZVd0AWD3w=
 golang.org/x/time v0.0.0-20220411224347-583f2d630306/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=

path_static_roles.go

@@ -302,16 +302,30 @@ func (b *backend) pathStaticRoleCreateUpdate(ctx context.Context, req *logical.R
 		skipRotation = c.SkipStaticRoleImportRotation
 	}
 
-	// lvr represents the role's LastVaultRotation
-	lvr := role.StaticAccount.LastVaultRotation
+	lastVaultRotation := role.StaticAccount.LastVaultRotation
 
 	// Only call setStaticAccountPassword if we're creating the role for the first time
 	var item *queue.Item
 	switch req.Operation {
 	case logical.CreateOperation:
-		// if we were asked to not rotate, just add the entry - this essentially becomes an update operation, except
-		// the item is new
 		if skipRotation {
+			b.Logger().Debug("skipping static role import rotation", "role", name)
+
+			// Synthetically set lastVaultRotation to now so that it gets
+			// queued correctly.
+			// NOTE: We intentionally do not set role.StaticAccount.LastVaultRotation
+			// because the zero value indicates Vault has not rotated the
+			// password yet.
+			lastVaultRotation = time.Now()
+
+			// NextVaultRotation allows calculating the TTL on GET /static-creds
+			// requests and to calculate the queue priority in populateQueue()
+			// across restarts. We can't rely on LastVaultRotation in these
+			// cases because, when import rotation is skipped, LastVaultRotation
+			// is set to a zero value in storage.
+			role.StaticAccount.SetNextVaultRotation(lastVaultRotation)
+
+			// we were told not to rotate, just add the entry
 			entry, err := logical.StorageEntryJSON(staticRolePath+name, role)
 			if err != nil {
 				return nil, err
@@ -324,8 +338,6 @@ func (b *backend) pathStaticRoleCreateUpdate(ctx context.Context, req *logical.R
 			item = &queue.Item{
 				Key: name,
 			}
-			// synthetically set lvr to now, so that it gets queued correctly
-			lvr = time.Now()
 			break
 		} else {
 			// setStaticAccountPassword calls Storage.Put and saves the role to storage
@@ -348,7 +360,7 @@ func (b *backend) pathStaticRoleCreateUpdate(ctx context.Context, req *logical.R
 				return nil, err
 			}
 			// guard against RotationTime not being set or zero-value
-			lvr = resp.RotationTime
+			lastVaultRotation = resp.RotationTime
 			item = &queue.Item{
 				Key: name,
 			}
@@ -373,7 +385,7 @@ func (b *backend) pathStaticRoleCreateUpdate(ctx context.Context, req *logical.R
 		}
 	}
 
-	item.Priority = lvr.Add(role.StaticAccount.RotationPeriod).Unix()
+	item.Priority = lastVaultRotation.Add(role.StaticAccount.RotationPeriod).Unix()
 
 	// Add their rotation to the queue
 	if err := b.pushItem(item); err != nil {
@@ -405,9 +417,14 @@ type staticAccount struct {
 	// This is returned on credential requests if it exists.
 	LastPassword string `json:"last_password"`
 
-	// LastVaultRotation represents the last time Vault rotated the password
+	// LastVaultRotation represents the last time Vault rotated the password. A
+	// zero value indicates the the password has never been rotated by Vault.
 	LastVaultRotation time.Time `json:"last_vault_rotation"`
 
+	// NextVaultRotation represents the next time Vault is expected to rotate
+	// the password
+	NextVaultRotation time.Time `json:"next_vault_rotation"`
+
 	// RotationPeriod is number in seconds between each rotation, effectively a
 	// "time to live". This value is compared to the LastVaultRotation to
 	// determine if a password needs to be rotated
@@ -417,7 +434,13 @@ type staticAccount struct {
 // NextRotationTime calculates the next rotation by adding the Rotation Period
 // to the last known vault rotation
 func (s *staticAccount) NextRotationTime() time.Time {
-	return s.LastVaultRotation.Add(s.RotationPeriod)
+	return s.NextVaultRotation
+}
+
+// SetNextVaultRotation sets the next vault rotation to time t plus the role's
+// rotation period.
+func (s *staticAccount) SetNextVaultRotation(t time.Time) {
+	s.NextVaultRotation = t.Add(s.RotationPeriod)
 }
 
 // PasswordTTL calculates the approximate time remaining until the password is

path_static_roles_test.go

@@ -682,7 +682,6 @@ func TestRoles_NewPasswordGeneration(t *testing.T) {
 		if resp.Data["password"] == initialPassword {
 			t.Fatalf("expected password to be different after second retry")
 		}
-
 	})
 
 	t.Run("updating password policy should generate new password", func(t *testing.T) {
@@ -734,7 +733,6 @@ func TestRoles_NewPasswordGeneration(t *testing.T) {
 		// Ensure WAL is flushed
 		walIDs = requireWALs(t, storage, 0)
 	})
-
 }
 
 func TestListRoles(t *testing.T) {
@@ -978,3 +976,15 @@ func createRole(t *testing.T, b *backend, storage logical.Storage, roleName stri
 		t.Fatal(err)
 	}
 }
+
+func createStaticRoleWithData(t *testing.T, b *backend, s logical.Storage, name string, d map[string]interface{}) (*logical.Response, error) {
+	t.Helper()
+	req := &logical.Request{
+		Operation: logical.CreateOperation,
+		Path:      staticRolePath + name,
+		Storage:   s,
+		Data:      d,
+	}
+
+	return b.HandleRequest(context.Background(), req)
+}

rotation.go

@@ -410,6 +410,7 @@ func (b *backend) setStaticAccountPassword(ctx context.Context, s logical.Storag
 	// lvr is the known LastVaultRotation
 	lvr := time.Now()
 	input.Role.StaticAccount.LastVaultRotation = lvr
+	input.Role.StaticAccount.SetNextVaultRotation(lvr)
 	input.Role.StaticAccount.LastPassword = input.Role.StaticAccount.Password
 	input.Role.StaticAccount.Password = newPassword
 	output.RotationTime = lvr

rotation_test.go

@@ -118,6 +118,78 @@ func TestAutoRotate(t *testing.T) {
 			t.Fatal("expected last_password to be equal to old password after auto rotation")
 		}
 	})
+
+	t.Run("skip_import_rotation is true and rotates after ttl expiration", func(t *testing.T) {
+		b, storage := getBackend(false)
+		defer b.Cleanup(context.Background())
+
+		configureOpenLDAPMount(t, b, storage)
+
+		data := map[string]interface{}{
+			"username":             "hashicorp",
+			"dn":                   "uid=hashicorp,ou=users,dc=hashicorp,dc=com",
+			"rotation_period":      "5s",
+			"skip_import_rotation": true,
+		}
+
+		roleName := "hashicorp"
+		createStaticRoleWithData(t, b, storage, roleName, data)
+		resp := readStaticCred(t, b, storage, roleName)
+
+		if resp.Data["password"] != "" {
+			t.Fatal("expected password to be empty, it wasn't: skip_import_rotation was enabled, password should not be rotated on import")
+		}
+
+		// Wait for auto rotation (5s) + 1 second for breathing room
+		time.Sleep(time.Second * 6)
+
+		resp = readStaticCred(t, b, storage, roleName)
+
+		if resp.Data["password"] == "" {
+			t.Fatal("expected password to be set after auto rotation, it wasn't")
+		}
+		if resp.Data["last_password"] != "" {
+			t.Fatal("expected last_password to be empty after auto rotation, it wasn't")
+		}
+	})
+
+	t.Run("skip_import_rotation is true and does not rotate after backend reload", func(t *testing.T) {
+		b, config := getBackendWithConfig(testBackendConfig(), false)
+		defer b.Cleanup(context.Background())
+		storage := config.StorageView
+
+		configureOpenLDAPMount(t, b, storage)
+
+		data := map[string]interface{}{
+			"username":             "hashicorp",
+			"dn":                   "uid=hashicorp,ou=users,dc=hashicorp,dc=com",
+			"rotation_period":      "10m",
+			"skip_import_rotation": true,
+		}
+
+		roleName := "hashicorp"
+		createStaticRoleWithData(t, b, storage, roleName, data)
+		resp := readStaticCred(t, b, storage, roleName)
+
+		if resp.Data["password"] != "" {
+			t.Fatal("expected password to be empty, it wasn't: skip_import_rotation was enabled, password should not be rotated on import")
+		}
+		if resp.Data["last_password"] != "" {
+			t.Fatal("expected last_password to be empty, it wasn't")
+		}
+
+		// Reload backend to similate a Vault restart/startup memory state
+		getBackendWithConfig(config, false)
+
+		resp = readStaticCred(t, b, storage, roleName)
+
+		if resp.Data["password"] != "" {
+			t.Fatal("expected password to be empty after backend reload, it wasn't: skip_import_rotation was enabled, password should not be rotated yet")
+		}
+		if resp.Data["last_password"] != "" {
+			t.Fatal("expected last_password to be empty after backend reload, it wasn't")
+		}
+	})
 }
 
 // TestPasswordPolicyModificationInvalidatesWAL tests that modification of the