Medium· 5.3GHSA-m979-w9wj-qfj9 CVE-2020-10660CWE-269Published Jan 30, 2024

HashiCorp Vault Improper Privilege Management

Research is free — Hunters explains how the bug works, the root-cause code pattern, how the fix addresses it, and how to test whether a target is affected, in chat. Investigate & write exploit is a paid run — the engine reads the advisory and fix commits, then builds and validates a working proof-of-concept exploit with reproduction steps.

Affected versions

0.9.0 → fixed in 1.3.4

Details

HashiCorp Vault and Vault Enterprise versions 0.9.0 through 1.3.3 may, under certain circumstances, have an Entity's Group membership inadvertently include Groups the Entity no longer has permissions to. Fixed in 1.3.4.

The fix

Make sure if a user gets removed from all groups in the

Nick Cabatoff· Mar 23, 2020, 08:38 PM+267−7040ec08f429

helper/testhelpers/ldap/ldaphelper.go+1 −1

@@ -49,7 +49,7 @@ func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ld
 		cfg.BindDN = "cn=admin,dc=planetexpress,dc=com"
 		cfg.BindPassword = "GoodNewsEveryone"
 		cfg.GroupDN = "ou=people,dc=planetexpress,dc=com"
-		cfg.GroupAttr = "memberOf"
+		cfg.GroupAttr = "cn"
 		cfg.RequestTimeout = 60
 		conn, err := client.DialLDAP(cfg)
 		if err != nil {

vault/expiration.go+1 −3

@@ -1004,9 +1004,7 @@ func (m *ExpirationManager) RenewToken(ctx context.Context, req *logical.Request
 	resp.Auth.ClientToken = te.ID
  
 	// Refresh groups
-	if resp.Auth.EntityID != "" &&
-		resp.Auth.GroupAliases != nil &&
-		m.core.identityStore != nil {
+	if resp.Auth.EntityID != "" && m.core.identityStore != nil {
 		validAliases, err := m.core.identityStore.refreshExternalGroupMembershipsByEntityID(ctx, resp.Auth.EntityID, resp.Auth.GroupAliases)
 		if err != nil {
 			return nil, err

vault/external_tests/identity/identity_test.go+261 −60

@@ -1,6 +1,7 @@
 package identity
  
 import (
+	"fmt"
 	"testing"
  
 	"github.com/go-ldap/ldap/v3"
@@ -11,11 +12,15 @@ import (
 	ldaphelper "github.com/hashicorp/vault/helper/testhelpers/ldap"
 	vaulthttp "github.com/hashicorp/vault/http"
 	"github.com/hashicorp/vault/sdk/helper/ldaputil"
+	"github.com/hashicorp/vault/sdk/helper/logging"
+	"github.com/hashicorp/vault/sdk/helper/strutil"
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/hashicorp/vault/vault"
 )
  
 func TestIdentityStore_Integ_GroupAliases(t *testing.T) {
+	t.Parallel()
+ 
 	var err error
 	coreConfig := &vault.CoreConfig{
 		DisableMlock: true,
@@ -189,60 +194,13 @@ func TestIdentityStore_Integ_GroupAliases(t *testing.T) {
  
 	// Re-read the admin_staff, ship_crew and devops group. This entity ID should have
 	// been added to admin_staff but not ship_crew.
-	assertMember := func(groupName, groupID string, expectFound bool) {
-		secret, err = client.Logical().Read("identity/group/id/" + groupID)
-		if err != nil {
-			t.Fatal(err)
-		}
-		groupMap := secret.Data
-		found := false
-		for _, entityIDRaw := range groupMap["member_entity_ids"].([]interface{}) {
-			if entityIDRaw.(string) == entityID {
-				found = true
-			}
-		}
-		if found != expectFound {
-			negation := ""
-			if !expectFound {
-				negation = "not "
-			}
-			t.Fatalf("expected entity ID %q to %sbe part of %q group", entityID, negation, groupName)
-		}
-	}
- 
-	assertMember("ship_crew", shipCrewGroupID, false)
-	assertMember("admin_staff", adminStaffGroupID, true)
-	assertMember("devops", devopsGroupID, true)
-	assertMember("engineer", devopsGroupID, true)
+	assertMember(t, client, entityID, "ship_crew", shipCrewGroupID, false)
+	assertMember(t, client, entityID, "admin_staff", adminStaffGroupID, true)
+	assertMember(t, client, entityID, "devops", devopsGroupID, true)
+	assertMember(t, client, entityID, "engineer", devopsGroupID, true)
  
 	// Now add Hermes to ship_crew
-	{
-		logger := log.New(nil)
-		ldapClient := ldaputil.Client{LDAP: ldaputil.NewLDAP(), Logger: logger}
-		// LDAP server won't accept changes unless we connect with TLS.  This
-		// isn't the default config returned by PrepareTestContainer because
-		// the Vault LDAP backend won't work with it, even with InsecureTLS,
-		// because the ServerName should be planetexpress.com and not localhost.
-		conn, err := ldapClient.DialLDAP(cfg)
-		if err != nil {
-			t.Fatal(err)
-		}
-		defer conn.Close()
- 
-		err = conn.Bind(cfg.BindDN, cfg.BindPassword)
-		if err != nil {
-			t.Fatal(err)
-		}
- 
-		hermesDn := "cn=Hermes Conrad,ou=people,dc=planetexpress,dc=com"
-		shipCrewDn := "cn=ship_crew,ou=people,dc=planetexpress,dc=com"
-		ldapreq := ldap.ModifyRequest{DN: shipCrewDn}
-		ldapreq.Add("member", []string{hermesDn})
-		err = conn.Modify(&ldapreq)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
+	addLdapGroupMember(t, cfg, "ship_crew", "hermes conrad")
  
 	// Re-login with LDAP
 	secret, err = client.Logical().Write("auth/ldap/login/hermes conrad", map[string]interface{}{
@@ -253,10 +211,10 @@ func TestIdentityStore_Integ_GroupAliases(t *testing.T) {
 	}
  
 	// Hermes should now be in ship_crew external group
-	assertMember("ship_crew", shipCrewGroupID, true)
-	assertMember("admin_staff", adminStaffGroupID, true)
-	assertMember("devops", devopsGroupID, true)
-	assertMember("engineer", devopsGroupID, true)
+	assertMember(t, client, entityID, "ship_crew", shipCrewGroupID, true)
+	assertMember(t, client, entityID, "admin_staff", adminStaffGroupID, true)
+	assertMember(t, client, entityID, "devops", devopsGroupID, true)
+	assertMember(t, client, entityID, "engineer", devopsGroupID, true)
  
 	identityStore := cores[0].IdentityStore()
  
@@ -330,10 +288,10 @@ func TestIdentityStore_Integ_GroupAliases(t *testing.T) {
 		t.Fatal(err)
 	}
  
-	assertMember("ship_crew", shipCrewGroupID, true)
-	assertMember("admin_staff", adminStaffGroupID, true)
-	assertMember("devops", devopsGroupID, true)
-	assertMember("engineer", devopsGroupID, true)
+	assertMember(t, client, entityID, "ship_crew", shipCrewGroupID, true)
+	assertMember(t, client, entityID, "admin_staff", adminStaffGroupID, true)
+	assertMember(t, client, entityID, "devops", devopsGroupID, true)
+	assertMember(t, client, entityID, "engineer", devopsGroupID, true)
  
 	// Remove user hermes conrad from the devops group in LDAP backend
 	secret, err = client.Logical().Write("auth/ldap/users/hermes conrad", map[string]interface{}{
@@ -359,3 +317,246 @@ func TestIdentityStore_Integ_GroupAliases(t *testing.T) {
 		t.Fatalf("failed to remove entity ID from the group")
 	}
 }
+ 
+func TestIdentityStore_Integ_RemoveFromExternalGroup(t *testing.T) {
+	t.Parallel()
+	var err error
+	coreConfig := &vault.CoreConfig{
+		DisableMlock: true,
+		DisableCache: true,
+		Logger:       log.NewNullLogger(),
+		CredentialBackends: map[string]logical.Factory{
+			"ldap": ldapcred.Factory,
+		},
+	}
+ 
+	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
+		HandlerFunc: vaulthttp.Handler,
+		Logger:      logging.NewVaultLogger(log.Debug),
+	})
+ 
+	cluster.Start()
+	defer cluster.Cleanup()
+ 
+	cores := cluster.Cores
+	client := cores[0].Client
+ 
+	err = client.Sys().EnableAuthWithOptions("ldap", &api.EnableAuthOptions{
+		Type: "ldap",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	auth, err := client.Sys().ListAuth()
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	accessor := auth["ldap/"].Accessor
+ 
+	adminPolicy := "admin_policy"
+	secret, err := client.Logical().Write("identity/group", map[string]interface{}{
+		"type":     "external",
+		"name":     "ldap_admin_staff",
+		"policies": []string{adminPolicy},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	adminStaffGroupID := secret.Data["id"].(string)
+	adminGroupName := "admin_staff"
+ 
+	secret, err = client.Logical().Write("identity/group-alias", map[string]interface{}{
+		"name":           adminGroupName,
+		"canonical_id":   adminStaffGroupID,
+		"mount_accessor": accessor,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	secret, err = client.Logical().Read("identity/group/id/" + adminStaffGroupID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	aliasMap := secret.Data["alias"].(map[string]interface{})
+	if aliasMap["canonical_id"] != adminStaffGroupID ||
+		aliasMap["name"] != adminGroupName ||
+		aliasMap["mount_accessor"] != accessor {
+		t.Fatalf("bad: group alias: %#v\n", aliasMap)
+	}
+ 
+	cleanup, cfg := ldaphelper.PrepareTestContainer(t, "latest")
+	defer cleanup()
+ 
+	// Configure LDAP auth
+	secret, err = client.Logical().Write("auth/ldap/config", map[string]interface{}{
+		"url":       cfg.Url,
+		"userattr":  cfg.UserAttr,
+		"userdn":    cfg.UserDN,
+		"groupdn":   cfg.GroupDN,
+		"groupattr": cfg.GroupAttr,
+		"binddn":    cfg.BindDN,
+		"bindpass":  cfg.BindPassword,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	// Create a local user in LDAP
+	secret, err = client.Logical().Write("auth/ldap/users/hermes conrad", map[string]interface{}{
+		"policies": "default",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	// Login with LDAP and create a token
+	secret, err = client.Logical().Write("auth/ldap/login/hermes conrad", map[string]interface{}{
+		"password": "hermes",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	token := secret.Auth.ClientToken
+	tokenPolicies, err := secret.TokenPolicies()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strutil.StrListContains(tokenPolicies, adminPolicy) {
+		t.Fatalf("expected token policies to contain %s, got: %v", adminPolicy, tokenPolicies)
+	}
+ 
+	// Lookup the token to get the entity ID
+	secret, err = client.Auth().Token().Lookup(token)
+	if err != nil {
+		t.Fatal(err)
+	}
+	entityID := secret.Data["entity_id"].(string)
+ 
+	assertMember(t, client, entityID, adminGroupName, adminStaffGroupID, true)
+ 
+	// Now remove Hermes from admin_staff
+	removeLdapGroupMember(t, cfg, adminGroupName, "hermes conrad")
+ 
+	// Re-login with LDAP
+	secret, err = client.Logical().Write("auth/ldap/login/hermes conrad", map[string]interface{}{
+		"password": "hermes",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	// Hermes should now be out of admin_staff group
+	assertMember(t, client, entityID, adminGroupName, adminStaffGroupID, false)
+	tokenPolicies, err = secret.TokenPolicies()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strutil.StrListContains(tokenPolicies, adminPolicy) {
+		t.Fatalf("expected token policies to not contain %s, got: %v", adminPolicy, tokenPolicies)
+	}
+ 
+	// Add Hermes back to admin_staff
+	addLdapGroupMember(t, cfg, adminGroupName, "hermes conrad")
+ 
+	// Re-login with LDAP
+	secret, err = client.Logical().Write("auth/ldap/login/hermes conrad", map[string]interface{}{
+		"password": "hermes",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	// Hermes should now be back in admin_staff group
+	assertMember(t, client, entityID, adminGroupName, adminStaffGroupID, true)
+	tokenPolicies, err = secret.TokenPolicies()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strutil.StrListContains(tokenPolicies, adminPolicy) {
+		t.Fatalf("expected token policies to contain %s, got: %v", adminPolicy, tokenPolicies)
+	}
+ 
+	// Remove Hermes from admin_staff once again
+	removeLdapGroupMember(t, cfg, adminGroupName, "hermes conrad")
+ 
+	oldToken := client.Token()
+	client.SetToken(secret.Auth.ClientToken)
+	secret, err = client.Auth().Token().RenewSelf(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	client.SetToken(oldToken)
+	assertMember(t, client, entityID, adminGroupName, adminStaffGroupID, false)
+	tokenPolicies, err = secret.TokenPolicies()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strutil.StrListContains(tokenPolicies, adminPolicy) {
+		t.Fatalf("expected token policies to not contain %s, got: %v", adminPolicy, tokenPolicies)
+	}
+}
+ 
+func assertMember(t *testing.T, client *api.Client, entityID, groupName, groupID string, expectFound bool) {
+	t.Helper()
+	secret, err := client.Logical().Read("identity/group/id/" + groupID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	groupMap := secret.Data
+	found := false
+	for _, entityIDRaw := range groupMap["member_entity_ids"].([]interface{}) {
+		if entityIDRaw.(string) == entityID {
+			found = true
+		}
+	}
+	if found != expectFound {
+		negation := ""
+		if !expectFound {
+			negation = "not "
+		}
+		t.Fatalf("expected entity ID %q to %sbe part of %q group", entityID, negation, groupName)
+	}
+}
+ 
+func removeLdapGroupMember(t *testing.T, cfg *ldaputil.ConfigEntry, groupCN, userCN string) {
+	userDN := fmt.Sprintf("cn=%s,ou=people,dc=planetexpress,dc=com", userCN)
+	groupDN := fmt.Sprintf("cn=%s,ou=people,dc=planetexpress,dc=com", groupCN)
+	ldapreq := ldap.ModifyRequest{DN: groupDN}
+	ldapreq.Delete("member", []string{userDN})
+	addRemoveLdapGroupMember(t, cfg, userCN, &ldapreq)
+}
+ 
+func addLdapGroupMember(t *testing.T, cfg *ldaputil.ConfigEntry, groupCN, userCN string) {
+	userDN := fmt.Sprintf("cn=%s,ou=people,dc=planetexpress,dc=com", userCN)
+	groupDN := fmt.Sprintf("cn=%s,ou=people,dc=planetexpress,dc=com", groupCN)
+	ldapreq := ldap.ModifyRequest{DN: groupDN}
+	ldapreq.Add("member", []string{userDN})
+	addRemoveLdapGroupMember(t, cfg, userCN, &ldapreq)
+}
+ 
+func addRemoveLdapGroupMember(t *testing.T, cfg *ldaputil.ConfigEntry, userCN string, req *ldap.ModifyRequest) {
+	logger := log.New(nil)
+	ldapClient := ldaputil.Client{LDAP: ldaputil.NewLDAP(), Logger: logger}
+	// LDAP server won't accept changes unless we connect with TLS.  This
+	// isn't the default config returned by PrepareTestContainer because
+	// the Vault LDAP backend won't work with it, even with InsecureTLS,
+	// because the ServerName should be planetexpress.com and not localhost.
+	conn, err := ldapClient.DialLDAP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+ 
+	err = conn.Bind(cfg.BindDN, cfg.BindPassword)
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	err = conn.Modify(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+}

vault/request_handling.go+4 −6

@@ -1077,13 +1077,11 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
 			}
  
 			auth.EntityID = entity.ID
-			if auth.GroupAliases != nil {
-				validAliases, err := c.identityStore.refreshExternalGroupMembershipsByEntityID(ctx, auth.EntityID, auth.GroupAliases)
-				if err != nil {
-					return nil, nil, err
-				}
-				auth.GroupAliases = validAliases
+			validAliases, err := c.identityStore.refreshExternalGroupMembershipsByEntityID(ctx, auth.EntityID, auth.GroupAliases)
+			if err != nil {
+				return nil, nil, err
 			}
+			auth.GroupAliases = validAliases
 		}
  
 		// Determine the source of the login

Fix for the issue with not removing users from external

Nick Cabatoff· Mar 18, 2020, 04:24 PM+267−7018485ee9d4