diff --git a/CHANGELOG.md b/CHANGELOG.md index 62d58ea..26d5a5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## Unreleased +## v0.14.1 + +BUG FIXES: +* fix a panic on init when static roles have names defined as hierarchical paths (https://github.com/hashicorp/vault-plugin-secrets-openldap/pull/115) + ## v0.14.0 ### IMPROVEMENTS: diff --git a/rotation.go b/rotation.go index c3c8d40..0d63670 100644 --- a/rotation.go +++ b/rotation.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/hashicorp/go-secure-stdlib/base62" @@ -32,6 +33,7 @@ const ( // rotations have been processed. It lists the roles from storage and searches // for any that have an associated static account, then adds them to the // priority queue for rotations. + func (b *backend) populateQueue(ctx context.Context, s logical.Storage) { log := b.Logger() log.Info("populating role rotation queue") @@ -42,9 +44,9 @@ func (b *backend) populateQueue(ctx context.Context, s logical.Storage) { log.Warn("unable to load rotation WALs", "error", err) } - roles, err := s.List(ctx, staticRolePath) + roles, err := b.getAllStaticRoles(ctx, s) if err != nil { - log.Warn("unable to list role for enqueueing", "error", err) + log.Warn("unable to list roles for enqueueing", "error", err) return } @@ -61,6 +63,10 @@ func (b *backend) populateQueue(ctx context.Context, s logical.Storage) { log.Warn("unable to read static role", "error", err, "role", roleName) continue } + if role == nil { + log.Error("role not found in storage", "roleName", roleName) + continue + } item := queue.Item{ Key: roleName, @@ -579,3 +585,50 @@ func (b *backend) popFromRotationQueueByKey(name string) (*queue.Item, error) { } return nil, queue.ErrEmpty } + +// getAllStaticRoles returns a slice of all static roles from storage +func (b *backend) getAllStaticRoles(ctx context.Context, s logical.Storage) ([]string, error) { + var allRoles []string + + roles, err := s.List(ctx, staticRolePath) + if err != nil { + return allRoles, fmt.Errorf("unable to list roles: %w", err) + } + + // Roles can be defined with hierarchical paths, e.g. "foo/bar/baz". But + // the storage.List() call to the top-level static role path will only + // return the top-level keys in the hierarchy. So we perform a + // non-recursive breadth-first search through all the keys returned from + // storage.List(). If a given node points to a static role, we append to + // the return slice. + for i := 0; i < len(roles); i++ { + path := roles[i] + + role, err := b.staticRole(ctx, s, path) + if err != nil { + return allRoles, fmt.Errorf("unable to read static role %q: %w", path, err) + } + + if role == nil && strings.HasSuffix(path, "/") { + // this is a directory + subPaths, err := s.List(ctx, staticRolePath+path) + if err != nil { + return allRoles, fmt.Errorf("unable to list roles: %w", err) + } + + // append to the roles slice to continue search in the sub-directory + for _, subPath := range subPaths { + // prevent infinite loop but this should never happen + if subPath == "" { + continue + } + subPath = fmt.Sprintf("%s%s", path, subPath) + roles = append(roles, subPath) + } + } else { + // a role exists at this path, append to the return slice + allRoles = append(allRoles, path) + } + } + return allRoles, nil +} diff --git a/rotation_test.go b/rotation_test.go index 0c61457..824add3 100644 --- a/rotation_test.go +++ b/rotation_test.go @@ -16,6 +16,85 @@ import ( "github.com/stretchr/testify/require" ) +// TestInitQueueHierarchicalPaths tests that the static role rotation queue +// gets initialized with all the roles from storage. +func TestInitQueueHierarchicalPaths(t *testing.T) { + for _, tc := range []struct { + name string + roles []string + }{ + { + "empty", + []string{}, + }, + { + "single-role-non-hierarchical-path", + []string{"a"}, + }, + { + "single-hierarchical-path", + []string{"a/b/c/d"}, + }, + { + "multi-role-non-hierarchical-path", + []string{"a", "b"}, + }, + { + "multi-role-with-hierarchical-path", + []string{"a", "a/b"}, + }, + { + "multi-role-multi-hierarchical-path", + []string{"a", "a/b", "a/b/c/d/e", "f"}, + }, + { + "multi-role-all-hierarchical-path", + []string{"a/b", "a/b/c", "d/e/f", "d/e/f/h/i/j", "d/e/f/h/x"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + config := &logical.BackendConfig{ + Logger: logging.NewVaultLogger(log.Debug), + + System: &logical.StaticSystemView{ + DefaultLeaseTTLVal: defaultLeaseTTLVal, + MaxLeaseTTLVal: maxLeaseTTLVal, + }, + StorageView: &logical.InmemStorage{}, + } + + b := Backend(&fakeLdapClient{throwErrs: false}) + b.Setup(context.Background(), config) + + b.credRotationQueue = queue.New() + initCtx := context.Background() + ictx, cancel := context.WithCancel(initCtx) + b.cancelQueue = cancel + + defer b.Cleanup(ctx) + configureOpenLDAPMount(t, b, config.StorageView) + + for _, r := range tc.roles { + createRole(t, b, config.StorageView, r) + } + + // Reset the rotation queue to simulate startup memory state + b.credRotationQueue = queue.New() + + // Now finish the startup process by populating the queue + b.initQueue(ictx, &logical.InitializationRequest{ + Storage: config.StorageView, + }) + + queueLen := b.credRotationQueue.Len() + if queueLen != len(tc.roles) { + t.Fatalf("unexpected rotated queue length: got=%d, want=%d", queueLen, len(tc.roles)) + } + }) + } +} + func TestAutoRotate(t *testing.T) { t.Run("auto rotate role", func(t *testing.T) { b, storage := getBackend(false)