From f43fdf54ab6b754d08267dbd730a276526faa1b4 Mon Sep 17 00:00:00 2001 From: Vault Automation Date: Mon, 9 Mar 2026 00:51:29 -0400 Subject: [PATCH] Vault 42257 root rotation in LDAP auth method for AD schema (#12223) (#12595) * adding root rotation for ldap auth method for schema AD * adding test cases for root rotation * code fix and adding TestRotateRoot_EncodeUTF16LEBytes * adding constants * schema validation + unit test * updated unit test * removed duplicate enum * adding acceptance test, unit test, changelog and updating schemaType to schema * adding logs and comments for debugging * added validation for config params * adding validation and test cases to enforce encrypted connection requirements for AD password rotation * adding fix to data race error in CI pipeline * addressing PR comments * fix for backward compatibility for schema and test * adding validation and tests for multiple URLs for AD root rotation --------- Co-authored-by: Stuti Srivastava Co-authored-by: Prajna Nayak --- builtin/credential/ldap/backend_test.go | 1 + builtin/credential/ldap/path_config.go | 6 + .../ldap/path_config_rotate_root.go | 107 ++++- .../ldap/path_config_rotate_root_test.go | 389 +++++++++++++++++- builtin/credential/ldap/path_config_test.go | 113 +++++ changelog/_12223.txt | 4 + helper/testhelpers/ldap/ldaphelper.go | 21 +- sdk/helper/ldaputil/config.go | 40 ++ sdk/helper/ldaputil/config_test.go | 294 ++++++++++++- 9 files changed, 962 insertions(+), 13 deletions(-) create mode 100644 builtin/credential/ldap/path_config_test.go create mode 100644 changelog/_12223.txt diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index adcfb7922e..e8425fa187 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -1543,6 +1543,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) { UsernameAsAlias: false, DerefAliases: "never", MaximumPageSize: 1000, + Schema: ldaputil.SchemaOpenLDAP, // Default schema when not specified }, } diff --git a/builtin/credential/ldap/path_config.go b/builtin/credential/ldap/path_config.go index f990ce8db6..975fc5ecf5 100644 --- a/builtin/credential/ldap/path_config.go +++ b/builtin/credential/ldap/path_config.go @@ -128,6 +128,12 @@ func (b *backend) Config(ctx context.Context, req *logical.Request) (*ldapConfig persistNeeded = true } + // Upgrade path: Set default schema for configs created before schema field was added + if result.Schema == "" { + result.Schema = ldaputil.SchemaOpenLDAP + persistNeeded = true + } + if persistNeeded && (b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary|consts.ReplicationPerformanceStandby)) { entry, err := logical.StorageEntryJSON("config", result) if err != nil { diff --git a/builtin/credential/ldap/path_config_rotate_root.go b/builtin/credential/ldap/path_config_rotate_root.go index a683a30240..580d0e4344 100644 --- a/builtin/credential/ldap/path_config_rotate_root.go +++ b/builtin/credential/ldap/path_config_rotate_root.go @@ -5,7 +5,11 @@ package ldap import ( "context" + "encoding/binary" "errors" + "fmt" + "strings" + "unicode/utf16" "github.com/go-ldap/ldap/v3" "github.com/hashicorp/vault/sdk/framework" @@ -83,6 +87,19 @@ func (b *backend) rotateRootCredential(ctx context.Context, req *logical.Request return responseError{errors.New("auth is not using authenticated search, no root to rotate")} } + // Validate TLS requirements for AD password rotation + schema := ldaputil.NormalizedSchema(cfg.Schema) + if schema == ldaputil.SchemaAD { + // Validate URL(s) which will actually be used for rotation + urlToValidate := cfg.Url + if cfg.RotationUrl != "" { + urlToValidate = cfg.RotationUrl + } + if err := validateADRotationURLs(urlToValidate, cfg.StartTLS); err != nil { + return responseError{err} + } + } + // grab our ldap client client := ldaputil.Client{ Logger: b.Logger(), @@ -98,16 +115,14 @@ func (b *backend) rotateRootCredential(ctx context.Context, req *logical.Request if err != nil { return err } + // Close the connection when done to avoid leaking connections, especially during repeated rotation attempts.defer conn.Close() + defer conn.Close() err = conn.Bind(u, p) if err != nil { return err } - lreq := &ldap.ModifyRequest{ - DN: cfg.BindDN, - } - var newPassword string if cfg.PasswordPolicy != "" { newPassword, err = b.System().GeneratePasswordFromPolicy(ctx, cfg.PasswordPolicy) @@ -118,12 +133,38 @@ func (b *backend) rotateRootCredential(ctx context.Context, req *logical.Request return err } - lreq.Replace("userPassword", []string{newPassword}) + switch schema { + case ldaputil.SchemaAD: + // AD root rotation requires: + // 1) Quoted password + // 2) UTF-16LE encoding + // 3) Encrypted connection (LDAPS/StartTLS) + // Without these, AD rejects password updates. + b.Logger().Debug("rotating root password using AD schema") + quotedPwd := fmt.Sprintf("\"%s\"", newPassword) + utf16Bytes := encodeUTF16LEBytes(quotedPwd) - err = conn.Modify(lreq) - if err != nil { - return err + modReq := ldap.NewModifyRequest(cfg.BindDN, nil) + modReq.Replace("unicodePwd", []string{string(utf16Bytes)}) + + if err := conn.Modify(modReq); err != nil { + return fmt.Errorf("failed to modify AD password for %q: %w", cfg.BindDN, err) + } + + case ldaputil.SchemaOpenLDAP: + b.Logger().Debug("rotating root password using openldap schema") + lreq := &ldap.ModifyRequest{ + DN: cfg.BindDN, + } + lreq.Replace("userPassword", []string{newPassword}) + + if err := conn.Modify(lreq); err != nil { + return fmt.Errorf("failed to modify OpenLDAP password: %w", err) + } + default: + return responseError{fmt.Errorf("unsupported schema type for password rotation: %s", schema)} } + // update config with new password cfg.BindPassword = newPassword entry, err := logical.StorageEntryJSON("config", cfg) @@ -138,6 +179,56 @@ func (b *backend) rotateRootCredential(ctx context.Context, req *logical.Request return nil } +// encodeUTF16LEBytes encodes a string as UTF-16LE bytes for AD password changes. +// This encoding is required for Active Directory password changes via the unicodePwd attribute. +func encodeUTF16LEBytes(s string) []byte { + utf16Runes := utf16.Encode([]rune(s)) + buf := make([]byte, len(utf16Runes)*2) + for i, r := range utf16Runes { + binary.LittleEndian.PutUint16(buf[i*2:], r) + } + return buf +} + +// validateADRotationURLs validates that all URLs in the provided URL string +// meet the security requirements for AD password rotation. +func validateADRotationURLs(urlString string, startTLS bool) error { + // AD password rotation requires encrypted connections (LDAPS or StartTLS) + // Supported configurations: + // 1. ldaps:// with proper certificate validation (recommended) + // 2. ldaps:// with insecure_tls=true (skips certificate validation) + // 3. ldap:// with starttls=true (with or without explicit certificates) + if strings.TrimSpace(urlString) == "" { + return errors.New("AD password rotation requires a configured URL") + } + // Split on commas to handle multiple URLs + rawURLs := strings.Split(urlString, ",") + hasNonTLSURL := false + for _, rawURL := range rawURLs { + rawURL := strings.TrimSpace(rawURL) + if rawURL == "" { + continue + } + urlLower := strings.ToLower(rawURL) + isLDAPS := strings.HasPrefix(urlLower, "ldaps://") + isLDAP := strings.HasPrefix(urlLower, "ldap://") + + // Validate that URL uses a supported protocol + if !isLDAPS && !isLDAP { + return fmt.Errorf("AD password rotation requires ldap:// or ldaps:// protocol, got: %s", rawURL) + } + // Track if any URL uses non-TLS ldap:// + if isLDAP { + hasNonTLSURL = true + } + } + // If any URL uses ldap:// (non-TLS), require StartTLS to ensure encryption + if hasNonTLSURL && !startTLS { + return errors.New("AD password rotation with ldap:// requires starttls=true for encrypted connection") + } + return nil +} + const pathConfigRotateRootHelpSyn = ` Request to rotate the LDAP credentials used by Vault ` diff --git a/builtin/credential/ldap/path_config_rotate_root_test.go b/builtin/credential/ldap/path_config_rotate_root_test.go index c4790eca87..bc48882ed9 100644 --- a/builtin/credential/ldap/path_config_rotate_root_test.go +++ b/builtin/credential/ldap/path_config_rotate_root_test.go @@ -4,18 +4,24 @@ package ldap import ( + "bytes" "context" + "encoding/binary" "os" + "strings" "testing" + "unicode/utf16" "github.com/hashicorp/vault/helper/testhelpers/ldap" logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical" + "github.com/hashicorp/vault/sdk/helper/ldaputil" "github.com/hashicorp/vault/sdk/logical" ) +// TestRotateRoot_DefaultSchema tests that the default schema type is used when not explicitly set and that rotation works in that case. // This test relies on a docker ldap server with a suitable person object (cn=admin,dc=planetexpress,dc=com) -// with bindpassword "admin". `PrepareTestContainer` does this for us. - see the backend_test for more details -func TestRotateRoot(t *testing.T) { +// with bindpassword "admin". `PrepareTestContainer` does this for us - see the backend_test for more details. +func TestRotateRoot_DefaultSchema(t *testing.T) { if os.Getenv(logicaltest.TestEnvVar) == "" { t.Skip("skipping rotate root tests because VAULT_ACC is unset") } @@ -57,6 +63,12 @@ func TestRotateRoot(t *testing.T) { } newCFG, err := b.Config(ctx, req) + if err != nil { + t.Fatalf("failed to get config after rotation: %s", err) + } + if newCFG == nil { + t.Fatal("config is nil after rotation") + } if newCFG.BindDN != cfg.BindDN { t.Fatalf("a value in config that should have stayed the same changed: %s", cfg.BindDN) } @@ -113,6 +125,12 @@ func TestRotateRootWithRotationUrl(t *testing.T) { } newCFG, err := b.Config(ctx, req) + if err != nil { + t.Fatalf("failed to get config after rotation: %s", err) + } + if newCFG == nil { + t.Fatal("config is nil after rotation") + } if newCFG.BindDN != cfg.BindDN { t.Fatalf("BindDN %q changed unexpectedly, found new value %q", cfg.BindDN, newCFG.BindDN) } @@ -124,3 +142,370 @@ func TestRotateRootWithRotationUrl(t *testing.T) { t.Fatalf("URL %q changed unexpectedly, found new value %q", mainDummyUrl, newCFG.Url) } } + +// TestRotateRoot_Schema_OpenLDAP tests that rotation for OpenLDAP schema +func TestRotateRoot_Schema_OpenLDAP(t *testing.T) { + if os.Getenv(logicaltest.TestEnvVar) == "" { + t.Skip("skipping rotate root tests because VAULT_ACC is unset") + } + ctx := context.Background() + b, store := createBackendWithStorage(t) + cleanup, cfg := ldap.PrepareTestContainer(t, ldap.DefaultVersion) + defer cleanup() + // set up auth config + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config", + Storage: store, + Data: map[string]interface{}{ + "url": cfg.Url, + "binddn": cfg.BindDN, + "bindpass": cfg.BindPassword, + "userdn": cfg.UserDN, + "schema": ldaputil.SchemaOpenLDAP, + }, + } + resp, err := b.HandleRequest(ctx, req) + if err != nil { + t.Fatalf("failed to initialize ldap auth config: %s", err) + } + if resp != nil && resp.IsError() { + t.Fatalf("failed to initialize ldap auth config: %s", resp.Data["error"]) + } + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config/rotate-root", + Storage: store, + } + _, err = b.HandleRequest(ctx, req) + if err != nil { + t.Fatalf("failed to rotate password: %s", err) + } + newCFG, err := b.Config(ctx, req) + if err != nil { + t.Fatalf("failed to get config after rotation: %s", err) + } + if newCFG == nil { + t.Fatal("config is nil after rotation") + } + if newCFG.BindDN != cfg.BindDN { + t.Fatalf("a value in config that should have stayed the same changed: %s", cfg.BindDN) + } + if newCFG.BindPassword == cfg.BindPassword { + t.Fatalf("the password should have changed, but it didn't") + } +} + +// TestRotateRoot_UnsupportedSchema tests unsupported schema handling +func TestRotateRoot_UnsupportedSchema(t *testing.T) { + if os.Getenv(logicaltest.TestEnvVar) == "" { + t.Skip("skipping rotate root tests because VAULT_ACC is unset") + } + ctx := context.Background() + b, store := createBackendWithStorage(t) + cleanup, cfg := ldap.PrepareTestContainer(t, ldap.DefaultVersion) + defer cleanup() + // set up auth config + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config", + Storage: store, + Data: map[string]interface{}{ + "url": cfg.Url, + "binddn": cfg.BindDN, + "bindpass": cfg.BindPassword, + "userdn": cfg.UserDN, + "schema": "unsupported_schema", // unsupported schema type + }, + } + + resp, err := b.HandleRequest(ctx, req) + if err != nil { + t.Fatalf("failed to initialize ldap auth config: %s", err) + } + // Config creation should fail with logical error + if resp == nil || !resp.IsError() { + t.Fatal("expected config creation to fail with unsupported schema type, but it succeeded") + } + // Verify error message contains expected text + errMsg := resp.Error().Error() + if !strings.Contains(errMsg, "unsupported schema type") { + t.Fatalf("expected error containing 'unsupported schema type', got: %s", errMsg) + } +} + +// TestRotateRoot_EncodeUTF16LEBytes tests the encoding of UTF-16LE bytes for AD password modification. +func TestRotateRoot_EncodeUTF16LEBytes(t *testing.T) { + tests := []struct { + name string + input string + }{ + {name: "empty_string", input: ""}, + {name: "single_char", input: "A"}, + {name: "quoted_password", input: "\"password\""}, + {name: "alphanumeric_with_special", input: "Pass123!"}, + {name: "unicode_chars", input: "Pāsswörd✓"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := encodeUTF16LEBytes(tt.input) + r := utf16.Encode([]rune(tt.input)) + expected := make([]byte, len(r)*2) + for i, v := range r { + binary.LittleEndian.PutUint16(expected[i*2:], v) + } + + if !bytes.Equal(got, expected) { + t.Fatalf("encodeUTF16LEBytes(%q) = %v, want %v", tt.input, got, expected) + } + }) + } +} + +// adTLSTestCase defines a test case for AD TLS validation +type adTLSTestCase struct { + name string + url string + insecureTLS bool + startTLS bool + certificate string + passwordPolicy string + expectError bool + errorContains string +} + +// TestRotateRoot_AD_TLS_Validation tests TLS validation for AD schema +func TestRotateRoot_AD_TLS_Validation(t *testing.T) { + tests := []adTLSTestCase{ + // Valid: ldaps - TLS validation passes, connection refused immediately (no real server). + { + name: "ldaps_valid", + url: "ldaps://127.0.0.1:1", + insecureTLS: true, + }, + // Valid: ldap + starttls - with optional cert and password policy + { + name: "ldap_with_starttls_valid", + url: "ldap://127.0.0.1:1", + startTLS: true, + certificate: testCert, + passwordPolicy: "test-policy", + }, + // Invalid: ldap without starttls - must surface as LogicalErrorResponse + { + name: "ldap_without_starttls_invalid", + url: "ldap://example.com", + expectError: true, + errorContains: "AD password rotation with ldap:// requires starttls=true for encrypted connection", + }, + // Invalid: unsupported protocol - must surface as LogicalErrorResponse + { + name: "invalid_protocol", + url: "http://example.com", + expectError: true, + errorContains: "AD password rotation requires ldap:// or ldaps:// protocol, got: http://example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testADTLSValidation(t, tt) + }) + } +} + +// Valid self-signed certificate for testing (shared across tests) +const testCert = `-----BEGIN CERTIFICATE----- +MIIB1jCCAUGgAwIBAgIFAMv4K9YwCwYJKoZIhvcNAQELMCkxEDAOBgNVBAoTB0Fj +bWUgQ28xFTATBgNVBAMTDEVkZGFyZCBTdGFyazAeFw0xNTA1MDYwMzU2NDBaFw0x +NjA1MDYwMzU2NDBaMCUxEDAOBgNVBAoTB0FjbWUgQ28xETAPBgNVBAMTCEpvbiBT +bm93MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDK6NU0R0eiCYVquU4RcjKc +LzGfx0aa1lMr2TnLQUSeLFZHFxsyyMXXuMPig3HK4A7SGFHupO+/1H/sL4xpH5zg +8+Zg2r8xnnney7abxcuv0uATWSIeKlNnb1ZO1BAxFnESc3GtyOCr2dUwZHX5mRVP ++Zxp2ni5qHNraf3wE2VPIQIDAQABoxIwEDAOBgNVHQ8BAf8EBAMCAKAwCwYJKoZI +hvcNAQELA4GBAIr2F7wsqmEU/J/kLyrCgEVXgaV/sKZq4pPNnzS0tBYk8fkV3V18 +sBJyHKRLL/wFZASvzDcVGCplXyMdAOCyfd8jO3F9Ac/xdlz10RrHJT75hNu3a7/n +9KNwKhfN4A1CQv2x372oGjRhCW5bHNCWx4PIVeNzCyq/KZhyY9sxHE1g +-----END CERTIFICATE-----` + +// testADTLSValidation is a helper function that runs AD TLS validation tests +func testADTLSValidation(t *testing.T, tt adTLSTestCase) { + t.Helper() + ctx := context.Background() + // Create fresh backend and storage for each subtest to avoid state bleeding + b, store := createBackendWithStorage(t) + + data := map[string]interface{}{ + "url": tt.url, + "binddn": "cn=admin,dc=example,dc=com", + "bindpass": "password", + "userdn": "ou=users,dc=example,dc=com", + "schema": ldaputil.SchemaAD, + "insecure_tls": tt.insecureTLS, + "starttls": tt.startTLS, + "certificate": tt.certificate, + "password_policy": tt.passwordPolicy, + } + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config", + Storage: store, + Data: data, + } + + resp, err := b.HandleRequest(ctx, req) + if err != nil { + t.Fatalf("unexpected error during config: %v", err) + } + if resp != nil && resp.IsError() { + t.Fatalf("unexpected response error during config: %v", resp.Error()) + } + + // Try to rotate - this is where TLS validation happens + rotateReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config/rotate-root", + Storage: store, + } + + rotateResp, rotateErr := b.HandleRequest(ctx, rotateReq) + + if tt.expectError { + // responseError is returned as logical.ErrorResponse with nil error + if rotateResp == nil || !rotateResp.IsError() { + t.Fatalf("expected error response, got resp=%v", rotateResp) + } + errMsg := rotateResp.Error().Error() + if !strings.Contains(errMsg, tt.errorContains) { + t.Fatalf("expected error containing %q, got: %q", tt.errorContains, errMsg) + } + } else { + // For valid configs, TLS validation passes and we expect a connection error + // (no real LDAP server). The backend logs these at ERROR level, + // which is expected and does not indicate a test failure. + // Assert neither the Go error nor the response error is a TLS validation failure, + // proving execution got past the TLS checks. + if rotateErr != nil { + if isTLSValidationError(rotateErr.Error()) { + t.Fatalf("unexpected TLS validation error: %s", rotateErr) + } + // connection/bind error is expected — TLS validation passed + } + if rotateResp != nil && rotateResp.IsError() { + errMsg := rotateResp.Error().Error() + if isTLSValidationError(errMsg) { + t.Fatalf("unexpected TLS validation error in response: %s", errMsg) + } + // connection/bind error in response is expected — TLS validation passed + } + } +} + +// isTLSValidationError checks if an error message is a TLS validation error +func isTLSValidationError(msg string) bool { + return strings.Contains(msg, "AD password rotation with ldap:// requires starttls=true for encrypted connection") || + strings.Contains(msg, "AD password rotation requires ldap:// or ldaps:// protocol, got:") +} + +// TestValidateADRotationURLs tests the URL validation logic for AD password rotation +func TestValidateADRotationURLs(t *testing.T) { + tests := []struct { + name string + urlString string + startTLS bool + wantErr bool + errMsg string + }{ + { + name: "single ldaps URL - valid", + urlString: "ldaps://secure.example.com", + startTLS: false, + wantErr: false, + }, + { + name: "single ldap URL with StartTLS - valid", + urlString: "ldap://example.com", + startTLS: true, + wantErr: false, + }, + { + name: "single ldap URL without StartTLS - invalid", + urlString: "ldap://example.com", + startTLS: false, + wantErr: true, + errMsg: "starttls=true", + }, + { + name: "multiple ldaps URLs - valid", + urlString: "ldaps://server1.example.com,ldaps://server2.example.com", + startTLS: false, + wantErr: false, + }, + { + name: "multiple ldap URLs with StartTLS - valid", + urlString: "ldap://server1.example.com,ldap://server2.example.com", + startTLS: true, + wantErr: false, + }, + { + name: "mixed ldaps and ldap with StartTLS - valid", + urlString: "ldaps://server1.example.com,ldap://server2.example.com", + startTLS: true, + wantErr: false, + }, + { + name: "mixed ldaps and ldap without StartTLS - invalid", + urlString: "ldaps://server1.example.com,ldap://server2.example.com", + startTLS: false, + wantErr: true, + errMsg: "starttls=true", + }, + { + name: "multiple ldap URLs without StartTLS - invalid", + urlString: "ldap://server1.example.com,ldap://server2.example.com", + startTLS: false, + wantErr: true, + errMsg: "starttls=true", + }, + { + name: "invalid protocol - invalid", + urlString: "http://example.com", + startTLS: false, + wantErr: true, + errMsg: "ldap:// or ldaps://", + }, + { + name: "empty URL - invalid", + urlString: "", + startTLS: false, + wantErr: true, + errMsg: "requires a configured URL", + }, + { + name: "URL with spaces - valid", + urlString: "ldaps://server1.example.com, ldaps://server2.example.com", + startTLS: false, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateADRotationURLs(tt.urlString, tt.startTLS) + if tt.wantErr { + if err == nil { + t.Errorf("validateADRotationURLs() expected error but got none") + return + } + if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("validateADRotationURLs() error = %v, want error containing %q", err, tt.errMsg) + } + } else { + if err != nil { + t.Errorf("validateADRotationURLs() unexpected error = %v", err) + } + } + }) + } +} diff --git a/builtin/credential/ldap/path_config_test.go b/builtin/credential/ldap/path_config_test.go new file mode 100644 index 0000000000..4b2e377d2c --- /dev/null +++ b/builtin/credential/ldap/path_config_test.go @@ -0,0 +1,113 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package ldap + +import ( + "context" + "encoding/json" + "testing" + + "github.com/hashicorp/vault/sdk/helper/ldaputil" + "github.com/hashicorp/vault/sdk/logical" +) + +// TestConfig_UpgradePath_Schema verifies that a ConfigEntry written to storage +// before the "schema" field existed (i.e. Schema == "") is transparently +// upgraded to the default value (SchemaOpenLDAP) when read back via Config(). +// This mirrors the upgrade handling already in place for CaseSensitiveNames +// and UsePre111GroupCNBehavior. +func TestConfig_UpgradePath_Schema(t *testing.T) { + ctx := context.Background() + b, store := createBackendWithStorage(t) + + // Simulate a pre-existing stored config that has no "schema" key — + // i.e. data written before the field was introduced. We do this by + // marshalling a map that deliberately omits the field. + oldEntry := map[string]interface{}{ + "url": "ldap://127.0.0.1", + "userdn": "ou=People,dc=example,dc=org", + "binddn": "cn=admin,dc=example,dc=org", + "bindpass": "secret", + // "schema" intentionally absent + "CaseSensitiveNames": false, + "use_pre111_group_cn_behavior": true, + } + raw, err := json.Marshal(oldEntry) + if err != nil { + t.Fatalf("failed to marshal old config entry: %s", err) + } + entry := &logical.StorageEntry{ + Key: "config", + Value: raw, + } + if err := store.Put(ctx, entry); err != nil { + t.Fatalf("failed to write legacy config to storage: %s", err) + } + + // Read back via Config() — this should trigger the upgrade path. + req := &logical.Request{Storage: store} + cfg, err := b.Config(ctx, req) + if err != nil { + t.Fatalf("Config() returned unexpected error: %s", err) + } + if cfg == nil { + t.Fatal("Config() returned nil; expected a valid ConfigEntry") + } + + // Schema must be defaulted to SchemaOpenLDAP, not left as "". + if cfg.Schema != ldaputil.SchemaOpenLDAP { + t.Errorf("expected Schema=%q after upgrade, got %q", ldaputil.SchemaOpenLDAP, cfg.Schema) + } + + // Verify the migrated value was persisted to storage. + persisted, err := store.Get(ctx, "config") + if err != nil { + t.Fatalf("failed to read persisted config: %s", err) + } + if persisted == nil { + t.Fatal("expected config to be re-persisted after upgrade, but nothing found in storage") + } + var persistedMap map[string]interface{} + if err := json.Unmarshal(persisted.Value, &persistedMap); err != nil { + t.Fatalf("failed to decode persisted config: %s", err) + } + if persistedMap["schema"] != ldaputil.SchemaOpenLDAP { + t.Errorf("persisted schema=%q; expected %q", persistedMap["schema"], ldaputil.SchemaOpenLDAP) + } +} + +// TestConfig_UpgradePath_Schema_NoOverwrite ensures that an already-set schema +// value is not overwritten by the upgrade logic. +func TestConfig_UpgradePath_Schema_NoOverwrite(t *testing.T) { + ctx := context.Background() + b, store := createBackendWithStorage(t) + + oldEntry := map[string]interface{}{ + "url": "ldap://127.0.0.1", + "userdn": "ou=People,dc=example,dc=org", + "binddn": "cn=admin,dc=example,dc=org", + "bindpass": "secret", + "schema": ldaputil.SchemaAD, // explicitly set to AD + "CaseSensitiveNames": false, + "use_pre111_group_cn_behavior": true, + } + raw, err := json.Marshal(oldEntry) + if err != nil { + t.Fatalf("failed to marshal config entry: %s", err) + } + entry := &logical.StorageEntry{Key: "config", Value: raw} + if err := store.Put(ctx, entry); err != nil { + t.Fatalf("failed to write config to storage: %s", err) + } + + req := &logical.Request{Storage: store} + cfg, err := b.Config(ctx, req) + if err != nil { + t.Fatalf("Config() returned unexpected error: %s", err) + } + + if cfg.Schema != ldaputil.SchemaAD { + t.Errorf("expected Schema=%q to be preserved, got %q", ldaputil.SchemaAD, cfg.Schema) + } +} diff --git a/changelog/_12223.txt b/changelog/_12223.txt new file mode 100644 index 0000000000..2f5a406c3f --- /dev/null +++ b/changelog/_12223.txt @@ -0,0 +1,4 @@ +```release-note:bug +ldap auth (enterprise): Fix root password rotation for Active Directory by implementing UTF-16LE encoding and schema-specific handling. Adds new 'schema' config field (defaults to 'openldap' for backward compatibility). +``` + diff --git a/helper/testhelpers/ldap/ldaphelper.go b/helper/testhelpers/ldap/ldaphelper.go index 4de3e40db0..2b022ac170 100644 --- a/helper/testhelpers/ldap/ldaphelper.go +++ b/helper/testhelpers/ldap/ldaphelper.go @@ -9,6 +9,7 @@ import ( "fmt" "runtime" "strings" + "sync" "testing" "time" @@ -17,6 +18,24 @@ import ( "github.com/hashicorp/vault/sdk/helper/ldaputil" ) +// safeBuffer is a thread-safe bytes.Buffer wrapper +type safeBuffer struct { + buf bytes.Buffer + mu sync.Mutex +} + +func (s *safeBuffer) Write(p []byte) (n int, err error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.buf.Write(p) +} + +func (s *safeBuffer) String() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.buf.String() +} + // DefaultVersion is the default version of the container to pull. // NOTE: This is currently pinned to a sha instead of "master", see: https://github.com/rroemhild/docker-test-openldap/issues/62 const DefaultVersion = "sha256:f4d9c5ba97f9662e9aea082b4aa89233994ca6e232abc1952d5d90da7e16b0eb" @@ -28,7 +47,7 @@ func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ld t.Skip("Skipping, as this image is not supported on ARM architectures") } - logsWriter := bytes.NewBuffer([]byte{}) + logsWriter := &safeBuffer{} runner, err := docker.NewServiceRunner(docker.RunOptions{ ImageRepo: "ghcr.io/rroemhild/docker-test-openldap", diff --git a/sdk/helper/ldaputil/config.go b/sdk/helper/ldaputil/config.go index 0ed2eb3845..a4ac61647b 100644 --- a/sdk/helper/ldaputil/config.go +++ b/sdk/helper/ldaputil/config.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-secure-stdlib/tlsutil" "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/strutil" ) var ldapDerefAliasMap = map[string]int{ @@ -26,6 +27,18 @@ var ldapDerefAliasMap = map[string]int{ "always": ldap.DerefAlways, } +const ( + SchemaAD = "ad" + SchemaOpenLDAP = "openldap" +) + +// SupportedSchemas returns a slice of different LDAP schemas supported +// by the plugin. This is used to change behavior when modifying +// user passwords (for example, during root password rotation). +func SupportedSchemas() []string { + return []string{SchemaOpenLDAP, SchemaAD} +} + // ConfigFields returns all the config fields that can potentially be used by the LDAP client. // Not all fields will be used by every integration. func ConfigFields() map[string]*framework.FieldSchema { @@ -287,6 +300,11 @@ Default: ({{.UserAttr}}={{.Username}})`, Description: "If true, matching sAMAccountName attribute values will be allowed to login when upndomain is defined.", Default: false, }, + "schema": { + Type: framework.TypeString, + Description: "LDAP schema type: 'ad' for Active Directory, 'openldap' for OpenLDAP. Determines root password rotation behavior.", + Default: SchemaOpenLDAP, + }, } } @@ -468,6 +486,14 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry if _, ok := d.Raw["enable_samaccountname_login"]; ok || !hadExisting { cfg.EnableSamaccountnameLogin = d.Get("enable_samaccountname_login").(bool) } + if _, ok := d.Raw["schema"]; ok || !hadExisting { + rawSchema := d.Get("schema").(string) + cfg.Schema = NormalizedSchema(rawSchema) + // Validate the normalized schema, not the raw input string, to allow for case-insensitive input while still enforcing valid schema types. + if !strutil.StrListContains(SupportedSchemas(), cfg.Schema) { + return nil, fmt.Errorf("unsupported schema type %q: must be one of %v", rawSchema, SupportedSchemas()) + } + } return cfg, nil } @@ -498,6 +524,7 @@ type ConfigEntry struct { ConnectionTimeout int `json:"connection_timeout"` // deprecated: use RequestTimeout DerefAliases string `json:"dereference_aliases"` MaximumPageSize int `json:"max_page_size"` + Schema string `json:"schema"` // These json tags deviate from snake case because there was a past issue // where the tag was being ignored, causing it to be jsonified as "CaseSensitiveNames", etc. @@ -541,6 +568,7 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} { "dereference_aliases": c.DerefAliases, "max_page_size": c.MaximumPageSize, "enable_samaccountname_login": c.EnableSamaccountnameLogin, + "schema": NormalizedSchema(c.Schema), // LDAP schema type for password operations } if c.CaseSensitiveNames != nil { m["case_sensitive_names"] = *c.CaseSensitiveNames @@ -593,6 +621,10 @@ func (c *ConfigEntry) Validate() error { return errwrap.Wrapf("failed to parse client X509 key pair: {{err}}", err) } } + normalizedSchema := NormalizedSchema(c.Schema) + if !strutil.StrListContains(SupportedSchemas(), normalizedSchema) { + return fmt.Errorf("unsupported schema type %q: must be one of %v", c.Schema, SupportedSchemas()) + } return nil } @@ -648,3 +680,11 @@ func min(a, b int) int { } return b } + +func NormalizedSchema(schema string) string { + normalizedSchema := strings.ToLower(strings.TrimSpace(schema)) + if normalizedSchema == "" { + return SchemaOpenLDAP + } + return normalizedSchema +} diff --git a/sdk/helper/ldaputil/config_test.go b/sdk/helper/ldaputil/config_test.go index 31bd42359e..ba6b8fff32 100644 --- a/sdk/helper/ldaputil/config_test.go +++ b/sdk/helper/ldaputil/config_test.go @@ -5,6 +5,7 @@ package ldaputil import ( "encoding/json" + "strings" "testing" "github.com/go-test/deep" @@ -68,6 +69,292 @@ func TestConfig(t *testing.T) { t.Errorf("expected false UseTokenGroups from JSON but got %t", configFromJSON.UseTokenGroups) } }) + + t.Run("default_schema_type", func(t *testing.T) { + if config.Schema != SchemaOpenLDAP { + t.Errorf("expected default Schema %s but got %s", SchemaOpenLDAP, config.Schema) + } + }) +} + +func TestNormalizedSchema(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "empty_string_defaults_to_openldap", + input: "", + expected: SchemaOpenLDAP, + }, + { + name: "lowercase_ad", + input: "ad", + expected: SchemaAD, + }, + { + name: "uppercase_AD", + input: "AD", + expected: SchemaAD, + }, + { + name: "mixed_case_Ad", + input: "Ad", + expected: SchemaAD, + }, + { + name: "lowercase_openldap", + input: "openldap", + expected: SchemaOpenLDAP, + }, + { + name: "uppercase_OPENLDAP", + input: "OPENLDAP", + expected: SchemaOpenLDAP, + }, + { + name: "mixed_case_OpenLDAP", + input: "OpenLDAP", + expected: SchemaOpenLDAP, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := NormalizedSchema(tt.input) + if result != tt.expected { + t.Errorf("NormalizedSchema(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestSchemaInConfigEntry(t *testing.T) { + t.Run("new_config_defaults_to_openldap", func(t *testing.T) { + s := &framework.FieldData{Schema: ConfigFields()} + config, err := NewConfigEntry(nil, s) + if err != nil { + t.Fatal("error getting default config") + } + + if config.Schema != SchemaOpenLDAP { + t.Errorf("expected default Schema %s but got %s", SchemaOpenLDAP, config.Schema) + } + }) + + t.Run("schema_normalized_on_create", func(t *testing.T) { + schema := ConfigFields() + data := &framework.FieldData{ + Raw: map[string]interface{}{ + "schema": "AD", + }, + Schema: schema, + } + + config, err := NewConfigEntry(nil, data) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if config.Schema != SchemaAD { + t.Errorf("expected normalized Schema 'ad' but got %s", config.Schema) + } + }) + + t.Run("schema_in_passwordless_map", func(t *testing.T) { + config := &ConfigEntry{ + Schema: SchemaAD, + } + + m := config.PasswordlessMap() + schema, ok := m["schema"] + if !ok { + t.Error("schema not found in PasswordlessMap") + } + + if schema != SchemaAD { + t.Errorf("expected schema %s in map but got %v", SchemaAD, schema) + } + }) + + t.Run("schema_update_existing_config", func(t *testing.T) { + existingConfig := &ConfigEntry{ + Url: "ldap://127.0.0.1", + Schema: SchemaOpenLDAP, + } + + schema := ConfigFields() + data := &framework.FieldData{ + Raw: map[string]interface{}{ + "schema": "AD", + }, + Schema: schema, + } + + updatedConfig, err := NewConfigEntry(existingConfig, data) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if updatedConfig.Schema != SchemaAD { + t.Errorf("expected updated Schema 'ad' but got %s", updatedConfig.Schema) + } + }) +} + +func TestSupportedSchemas(t *testing.T) { + schemas := SupportedSchemas() + + expectedSchemas := []string{SchemaOpenLDAP, SchemaAD} + if len(schemas) != len(expectedSchemas) { + t.Errorf("expected %d schemas but got %d", len(expectedSchemas), len(schemas)) + } + + for _, expected := range expectedSchemas { + found := false + for _, schema := range schemas { + if schema == expected { + found = true + break + } + } + if !found { + t.Errorf("expected schema %q not found in SupportedSchemas()", expected) + } + } +} + +func TestSchemaValidation(t *testing.T) { + t.Run("valid_openldap_schema", func(t *testing.T) { + config := &ConfigEntry{ + Url: "ldap://127.0.0.1", + UserDN: "ou=users,dc=example,dc=com", + Schema: SchemaOpenLDAP, + TLSMinVersion: "tls12", + TLSMaxVersion: "tls12", + } + + if err := config.Validate(); err != nil { + t.Errorf("expected no error for valid openldap schema, got: %v", err) + } + }) + + t.Run("valid_ad_schema", func(t *testing.T) { + config := &ConfigEntry{ + Url: "ldap://127.0.0.1", + UserDN: "ou=users,dc=example,dc=com", + Schema: SchemaAD, + TLSMinVersion: "tls12", + TLSMaxVersion: "tls12", + } + + if err := config.Validate(); err != nil { + t.Errorf("expected no error for valid ad schema, got: %v", err) + } + }) + + t.Run("empty_schema_passes_validation", func(t *testing.T) { + config := &ConfigEntry{ + Url: "ldap://127.0.0.1", + UserDN: "ou=users,dc=example,dc=com", + Schema: "", + TLSMinVersion: "tls12", + TLSMaxVersion: "tls12", + } + + if err := config.Validate(); err != nil { + t.Errorf("expected no error for empty schema (defaults to openldap), got: %v", err) + } + }) + + t.Run("generic_unsupported_schema_fails_validation", func(t *testing.T) { + config := &ConfigEntry{ + Url: "ldap://127.0.0.1", + UserDN: "ou=users,dc=example,dc=com", + Schema: "unsupported_schema", + TLSMinVersion: "tls12", + TLSMaxVersion: "tls12", + } + + err := config.Validate() + if err == nil { + t.Error("expected error for unsupported schema, got nil") + } + + if err != nil && !strings.Contains(err.Error(), "unsupported schema") { + t.Errorf("expected error message to contain 'unsupported schema', got: %v", err) + } + }) + + t.Run("freeipa_schema_fails_validation", func(t *testing.T) { + config := &ConfigEntry{ + Url: "ldap://127.0.0.1", + UserDN: "ou=users,dc=example,dc=com", + Schema: "freeipa", + TLSMinVersion: "tls12", + TLSMaxVersion: "tls12", + } + + err := config.Validate() + if err == nil { + t.Error("expected error for unsupported schema 'freeipa', got nil") + } + + if err != nil && !strings.Contains(err.Error(), "freeipa") { + t.Errorf("expected error message to include 'freeipa', got: %v", err) + } + }) + + t.Run("typo_in_schema_fails_validation", func(t *testing.T) { + // Test common typos like "adc" instead of "ad" + config := &ConfigEntry{ + Url: "ldap://127.0.0.1", + UserDN: "ou=users,dc=example,dc=com", + Schema: "adc", + TLSMinVersion: "tls12", + TLSMaxVersion: "tls12", + } + + err := config.Validate() + if err == nil { + t.Error("expected error for typo schema 'adc', got nil") + } + + // Verify error message is helpful + if err != nil && !strings.Contains(err.Error(), "unsupported schema") { + t.Errorf("expected error message to contain 'unsupported schema', got: %v", err) + } + if err != nil && !strings.Contains(err.Error(), "adc") { + t.Errorf("expected error message to include the unsupported value 'adc', got: %v", err) + } + }) + + t.Run("normalized_unsupported_schema_fails_validation", func(t *testing.T) { + // Test that even after normalization (lowercase), unsupported schemas fail + schema := ConfigFields() + data := &framework.FieldData{ + Raw: map[string]interface{}{ + "url": "ldap://127.0.0.1", + "userdn": "ou=users,dc=example,dc=com", + "schema": "UNSUPPORTED_SCHEMAS", + }, + Schema: schema, + } + + // NewConfigEntry should fail because schema validation happens during creation + _, err := NewConfigEntry(nil, data) + if err == nil { + t.Error("expected error from NewConfigEntry for unsupported schema, got nil") + } + + // Verify the error message is helpful + if err != nil { + if !strings.Contains(err.Error(), "unsupported schema") { + t.Errorf("expected error message to contain %q, got: %v", "unsupported schema", err) + } + } + }) } func testConfig(t *testing.T) *ConfigEntry { @@ -84,6 +371,7 @@ func testConfig(t *testing.T) *ConfigEntry { ConnectionTimeout: 15, ClientTLSCert: "", ClientTLSKey: "", + Schema: SchemaOpenLDAP, } } @@ -144,7 +432,8 @@ var jsonConfig = []byte(`{ "request_timeout": 30, "connection_timeout": 15, "ClientTLSCert": "", - "ClientTLSKey": "" + "ClientTLSKey": "", + "schema": "openldap" }`) var jsonConfigDefault = []byte(` @@ -179,6 +468,7 @@ var jsonConfigDefault = []byte(` "CaseSensitiveNames": false, "ClientTLSCert": "", "ClientTLSKey": "", - "enable_samaccountname_login": false + "enable_samaccountname_login": false, + "schema": "openldap" } `)