From 9faeb7a1147435aa7ebc02bb997837ddfbdd2a81 Mon Sep 17 00:00:00 2001 From: GL Date: Mon, 19 Oct 2026 18:49:20 +0300 Subject: [PATCH] fix(iam): treat IAM record names too long for the drive as absent Since RELEASE.2026-09-16T00-00-00Z, LDAP users whose normalized DN is longer than 255 bytes cannot log in. NormDN hex-escapes every non-ASCII byte (go-ldap DN.String()), so one Cyrillic letter takes six characters. IAM looks up records named after the DN: the parent revision check added with cmd/iam-revision.go reads config/iam/users//identity.json, and PolicyDBGet reads config/iam/policydb/sts-users/.json when the mapping is not cached. A drive rejects such an element in one of two ways. checkPathLength refuses more than 255 counted characters, but outside Windows it never counts a backslash, so an escaped DN usually passes it. The filesystem then fails with ENAMETOOLONG once the parent directory exists. toObjectErr reports both as ObjectNameInvalid, and the callers treat only errConfigNotFound as "no record". As a result, AssumeRoleWithLDAPIdentity, access-key creation and STS loading fail. Such a record cannot have been stored. When the object layer's own name validation (checkBucketAndObjectNames) accepts the path, ObjectNameInvalid can only mean a length rejection, so report it as errConfigNotFound. Invalid names and storage errors are returned unchanged, and saving such a record still fails. The etcd store has no such limit and is unchanged. Co-Authored-By: Claude Opus 5.5 Signed-off-by: GL --- cmd/iam-object-store.go | 21 +++++++ cmd/iam-revision_test.go | 129 +++++++++++++++++++++++++++++++++++++++ cmd/sts-handlers_test.go | 108 ++++++++++++++++++++++++++++++++ 3 files changed, 258 insertions(+) diff --git a/cmd/iam-object-store.go b/cmd/iam-object-store.go index c3ece5a2b..946e62eb1 100644 --- a/cmd/iam-object-store.go +++ b/cmd/iam-object-store.go @@ -134,9 +134,30 @@ func decryptData(data []byte, objPath string) ([]byte, error) { return nil, err } +// iamConfigNameTooLong reports whether the drives rejected objPath for its +// length. IAM records are named after external identities: a normalized LDAP +// DN hex-escapes every non-ASCII byte, so one Cyrillic letter takes six +// characters. Such a record cannot have been stored. checkPathLength does not +// count backslashes outside Windows, so an escaped DN can pass it and still +// fail on the filesystem with ENAMETOOLONG once its parent directory exists. +// toObjectErr reports both as ObjectNameInvalid, as it does for names that the +// object layer rejects itself; repeat that validation to tell them apart. +func iamConfigNameTooLong(objPath string, err error) bool { + if errors.Is(err, errFileNameTooLong) { + return true + } + var invalid ObjectNameInvalid + return errors.As(err, &invalid) && checkBucketAndObjectNames(context.Background(), minioMetaBucket, objPath) == nil +} + func (iamOS *IAMObjectStore) loadIAMConfigBytesWithMetadata(ctx context.Context, objPath string) ([]byte, ObjectInfo, error) { data, meta, err := readConfigWithMetadata(ctx, iamOS.objAPI, objPath, ObjectOptions{}) if err != nil { + if iamConfigNameTooLong(objPath, err) { + // A missing record, not a storage failure: lookups of the + // identity proceed, while saving its record still fails. + err = errConfigNotFound + } return nil, meta, err } data, err = decryptData(data, objPath) diff --git a/cmd/iam-revision_test.go b/cmd/iam-revision_test.go index 70f5d71a5..e605a0296 100644 --- a/cmd/iam-revision_test.go +++ b/cmd/iam-revision_test.go @@ -14,7 +14,9 @@ import ( "time" "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/auth" "github.com/minio/minio/internal/grid" + "github.com/pgsty/silo-pkg/v3/ldap" xnet "github.com/pgsty/silo-pkg/v3/net" ) @@ -93,6 +95,133 @@ func TestIAMRevocationTombstoneReplayIsIdempotent(t *testing.T) { } } +// An LDAP parent named by a normalized DN longer than NAME_MAX has no record +// on object storage. Its lookups must find nothing instead of failing login. +func TestLoadIAMRevisionNameTooLong(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + t.Run(backend, func(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t, backend) + store := sys.store.IAMStorageAPI + // 160 bytes of UTF-8 become 384 characters. + parent, err := ldap.NormalizeDN("uid=Сотрудник,ou=Отдел разработки,ou=Дирекция информационных технологий,ou=people,ou=swengg,dc=min,dc=io") + mustIAM(t, err) + if len(parent) <= 255 { + t.Fatalf("normalized DN has only %d characters", len(parent)) + } + parentPath := getUserIdentityPath(parent, regUser) + for _, path := range []string{parentPath, getUserIdentityPath("missing-parent", regUser)} { + r, err := loadIAMRevision(ctx, store, path) + if err != nil || r.Deleted || !r.timestamp().IsZero() || !r.RevokedBefore.IsZero() { + t.Fatalf("absent revision %.40s...: %+v, %v", path, r, err) + } + } + if backend == "object" { + var invalid ObjectNameInvalid + if _, err := loadIAMRevision(ctx, store, iamConfigUsersPrefix+"a//b/"+iamIdentityFile); !errors.As(err, &invalid) { + t.Fatalf("invalid name not caused by length was masked: %v", err) + } + } + + claims := map[string]any{} + mustIAM(t, setIAMParentRevocationClaim(ctx, store, parent, claims)) + if _, ok := claims[iamParentRevocationClaim]; ok { + t.Fatalf("unexpected revocation claim: %v", claims) + } + if _, err := sys.PolicyDBGet(parent); err != nil { + t.Fatalf("policy lookup failed: %v", err) + } + // The call that failed in AssumeRoleWithLDAPIdentity. + secret, err := getTokenSigningKey() + mustIAM(t, err) + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + mustIAM(t, err) + cred.ParentUser = parent + _, err = sys.SetTempUser(ctx, cred.AccessKey, cred, "") + mustIAM(t, err) + if _, ok := sys.store.GetUser(cred.AccessKey); !ok { + t.Fatal("temporary credential not cached") + } + + // Object storage still cannot record a revocation for this parent; + // the failure must be reported. Etcd has no such limit. + err = saveIAMRevision(ctx, store, parentPath, &iamUserRevocation{}) + if backend == "object" { + if !iamConfigNameTooLong(parentPath, err) { + t.Fatalf("expected name-too-long error, got %v", err) + } + return + } + mustIAM(t, err) + if err := checkIAMParentRevision(ctx, store, cred); !errors.Is(err, errIAMStaleUpdate) { + t.Fatalf("revoked parent accepted: %v", err) + } + }) + } +} + +// checkPathLength does not count backslashes outside Windows, so this escaped +// DN passes it. Once config/iam/users exists, as after creating any built-in +// user, the filesystem rejects the element with ENAMETOOLONG instead. +func TestLoadIAMRevisionFilesystemNameTooLong(t *testing.T) { + ctx, sys, obj := prepareIAMRevisionFixture(t) + parent, err := ldap.NormalizeDN("cn=Сидоров П.П.,ou=Отдел ИТ-поддержки,ou=Пользователи,ou=people,ou=swengg,dc=min,dc=io") + mustIAM(t, err) + parentPath := getUserIdentityPath(parent, regUser) + if len(parent) <= 255 || checkPathLength(parentPath) != nil { + t.Fatalf("normalized DN must exceed NAME_MAX yet pass checkPathLength: %d characters, %v", len(parent), checkPathLength(parentPath)) + } + _, err = sys.CreateUser(ctx, "existing-user", madmin.AddOrUpdateUserReq{SecretKey: "synthetic-user-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + mustIAM(t, saveIAMRevision(ctx, sys.store.IAMStorageAPI, getMappedPolicyPath("existing-parent", stsUser, false), &MappedPolicy{Version: 1, Policies: "readwrite"})) + store := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, LDAPUsersSysType)} + mustIAM(t, store.LoadIAMCache(ctx, false)) + r, err := loadIAMRevision(ctx, store.IAMStorageAPI, parentPath) + if err != nil || r.Deleted || !r.timestamp().IsZero() { + t.Fatalf("absent revision: %+v, %v", r, err) + } + // LDAP mode reads policydb/sts-users/.json. + if _, err := store.PolicyDBGet(parent); err != nil { + t.Fatalf("policy lookup failed: %v", err) + } + secret, err := getTokenSigningKey() + mustIAM(t, err) + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + mustIAM(t, err) + cred.ParentUser = parent + if _, err := store.SetTempUser(ctx, cred.AccessKey, cred, ""); err != nil { + t.Fatalf("temporary credential rejected: %v", err) + } + err = saveIAMRevision(ctx, store.IAMStorageAPI, parentPath, &iamUserRevocation{}) + if !iamConfigNameTooLong(parentPath, err) { + t.Fatalf("expected name-too-long error, got %v", err) + } +} + +func TestIAMConfigNameTooLong(t *testing.T) { + long := iamConfigUsersPrefix + strings.Repeat("x", 256) + "/" + iamIdentityFile + short := iamConfigUsersPrefix + strings.Repeat("x", 200) + "/" + iamIdentityFile + invalid := iamConfigUsersPrefix + "a//b/" + iamIdentityFile + for _, tc := range []struct { + name string + path string + err error + want bool + }{ + {"NAME_MAX", long, ObjectNameInvalid{Bucket: minioMetaBucket, Object: long}, true}, + {"filesystem", short, ObjectNameInvalid{Bucket: minioMetaBucket, Object: short}, true}, + {"drive", long, errFileNameTooLong, true}, + {"no error", long, nil, false}, + {"not found", long, errConfigNotFound, false}, + {"offline drive", long, errDiskNotFound, false}, + {"read quorum", long, InsufficientReadQuorum{}, false}, + {"invalid name", invalid, ObjectNameInvalid{Bucket: minioMetaBucket, Object: invalid}, false}, + } { + if got := iamConfigNameTooLong(tc.path, tc.err); got != tc.want { + t.Errorf("%s: got %t, want %t", tc.name, got, tc.want) + } + } +} + // Run with both object storage and etcd through TestIAMRevocation*Lifecycle. // The object-store case uses the real peer RPC and deletion handler, so a // spurious notification actually destroys the parent instead of only counting it. diff --git a/cmd/sts-handlers_test.go b/cmd/sts-handlers_test.go index dc58dcc69..a0c93a29f 100644 --- a/cmd/sts-handlers_test.go +++ b/cmd/sts-handlers_test.go @@ -36,6 +36,7 @@ import ( "testing" "time" + goldap "github.com/go-ldap/ldap/v3" "github.com/klauspost/compress/zip" "github.com/minio/madmin-go/v3" "github.com/minio/minio-go/v7" @@ -1095,6 +1096,7 @@ func TestIAMWithLDAPServerSuite(t *testing.T) { suite.TestLDAPSTSServiceAccountsWithGroups(c) suite.TestLDAPAttributesLookup(c) suite.TestLDAPCyrillicUser(c) + suite.TestLDAPLongNonASCIIDN(c) suite.TestLDAPSlashDN(c) suite.TearDownSuite(c) }, @@ -3291,6 +3293,112 @@ func (s *TestSuiteIAM) TestLDAPCyrillicUser(c *check) { } } +// Normalized DNs hex-escape every non-ASCII byte, so a user in nested Cyrillic +// OUs has a DN longer than NAME_MAX: 160 bytes of UTF-8 become 384 characters. +// Records named after such a DN can never be stored by the object backend. +// The user is entitled only through a group, as is typical for AD deployments. +func (s *TestSuiteIAM) TestLDAPLongNonASCIIDN(c *check) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + const ( + username = "Сотрудник" + parentOU = "ou=Дирекция информационных технологий,ou=people,ou=swengg,dc=min,dc=io" + childOU = "ou=Отдел разработки," + parentOU + userDN = "uid=" + username + "," + childOU + groupDN = "cn=long-dn-staff,ou=groups,ou=swengg,dc=min,dc=io" + ) + normDN, err := ldap.NormalizeDN(userDN) + if err != nil { + c.Fatal(err) + } + if len(normDN) <= 255 { + c.Fatalf("normalized test DN must exceed 255 characters, got %d", len(normDN)) + } + + conn, err := globalIAMSys.LDAPConfig.LDAP.Connect() + if err != nil { + c.Fatalf("LDAP connect failed: %v", err) + } + defer conn.Close() + if err = conn.Bind("cn=admin,dc=min,dc=io", "admin"); err != nil { + c.Fatalf("LDAP admin bind failed: %v", err) + } + newEntry := func(dn string, attrs map[string]string) *goldap.AddRequest { + req := goldap.NewAddRequest(dn, nil) + for name, value := range attrs { + req.Attribute(name, []string{value}) + } + return req + } + entries := []*goldap.AddRequest{ + newEntry(parentOU, map[string]string{"objectClass": "organizationalUnit", "ou": "Дирекция информационных технологий"}), + newEntry(childOU, map[string]string{"objectClass": "organizationalUnit", "ou": "Отдел разработки"}), + newEntry(userDN, map[string]string{"objectClass": "inetOrgPerson", "uid": username, "cn": username, "sn": username, "userPassword": "example"}), + newEntry(groupDN, map[string]string{"objectClass": "groupOfNames", "cn": "long-dn-staff", "member": userDN}), + } + removeEntries := func() { + for i := len(entries) - 1; i >= 0; i-- { + err := conn.Del(goldap.NewDelRequest(entries[i].DN, nil)) + if err != nil && !goldap.IsErrorWithCode(err, goldap.LDAPResultNoSuchObject) { + c.Errorf("Unable to remove LDAP entry %s: %v", entries[i].DN, err) + } + } + } + removeEntries() // left over from an interrupted run + defer removeEntries() + for _, entry := range entries { + if err = conn.Add(entry); err != nil { + c.Fatalf("Unable to add LDAP entry %s: %v", entry.DN, err) + } + } + + groupReq := madmin.PolicyAssociationReq{ + Policies: []string{"readwrite"}, + Group: groupDN, + } + if _, err = s.adm.AttachPolicyLDAP(ctx, groupReq); err != nil { + c.Fatalf("Unable to attach group policy: %v", err) + } + defer func() { + if _, err := s.adm.DetachPolicyLDAP(ctx, groupReq); err != nil { + c.Errorf("Unable to detach group policy: %v", err) + } + }() + + bucket := getRandomBucketName() + if err = s.client.MakeBucket(ctx, bucket, minio.MakeBucketOptions{}); err != nil { + c.Fatalf("bucket create error: %v", err) + } + + ldapID := cr.LDAPIdentity{ + Client: s.TestSuiteCommon.client, + STSEndpoint: s.endPoint, + LDAPUsername: username, + LDAPPassword: "example", + } + value, err := ldapID.Retrieve() + if err != nil { + c.Fatalf("Expected to generate STS creds, got err: %#v", err) + } + u, ok := globalIAMSys.GetUser(ctx, value.AccessKeyID) + if !ok { + c.Fatalf("Expected to find user %s", value.AccessKeyID) + } + if u.Credentials.ParentUser != normDN { + c.Fatalf("unexpected parent user: %s", u.Credentials.ParentUser) + } + c.mustListObjects(ctx, s.getUserClient(c, value.AccessKeyID, value.SecretAccessKey, value.SessionToken), bucket) + + // An access key created by this user inherits the same parent. + userAdmClient := s.getAdminClient(c, value.AccessKeyID, value.SecretAccessKey, value.SessionToken) + svc := c.mustCreateSvcAccount(ctx, value.AccessKeyID, userAdmClient) + c.assertSvcAccS3Access(ctx, s, svc, bucket) + if err = s.adm.DeleteServiceAccount(ctx, svc.AccessKey); err != nil { + c.Fatalf("Unable to delete service account: %v", err) + } +} + func (s *TestSuiteIAM) TestLDAPSlashDN(c *check) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel()