CVE-2024-24747 (Go, published 2024-01-31T22:15Z)
Description: value: MinIO is a High Performance Object Storage. When someone creates an access key, it inherits the permissions of the parent key. Not only for `s3:*` actions, but also `admin:*` actions. Which means unless somewhere above in the access-key hierarchy, the `admin` rights are denied, access keys will be able to simply override their own `s3` permissions to something more permissive. The vulnerability is fixed in RELEASE.2024-01-31T20-20-33Z.
Severity: nan (CVSS v3 base score: 8.8)
Weakness: CWE-269 \u2014 Improper Privilege Management
The product does not properly assign, modify, track, or check privileges for an actor, creating an unintended sphere of control for that actor.
Repository: https://github.com/minio/minio
Files changed:
cmd/admin-handlers-users.go
cmd/admin-handlers-users_test.go
cmd/auth-handler.go
cmd/iam.go
cmd/signature-v4-utils.go
cmd/sts-handlers_test.go
Commit message: fix: permission checks for editing access keys (#18928)

With this change, only a user with `UpdateServiceAccountAdminAction`
permission is able to edit access keys.

We would like to let a user edit their own access keys, however the
feature needs to be re-designed for better security and integration with
external systems like AD/LDAP and OpenID.

This change prevents privilege escalation via service accounts.

Vulnerable code:
		requestUser := cred.AccessKey
		if cred.ParentUser != "" {
			requestUser = cred.ParentUser
		}

		if requestUser != svcAccount.ParentUser {
			writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
			return
		}
	// 4. Check that svc account can restrict the policy, and that the
	// session policy can be updated.
	c.assertSvcAccSessionPolicyUpdate(ctx, s, userAdmClient, accessKey, bucket)

	// 4. Check that service account's secret key and account status can be
	// updated.
	c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, userAdmClient, accessKey, bucket)

		// We only support admin credentials to access admin APIs.
	// Add all the necessary claims for the service accounts.
	// Now check if we have a sessionPolicy.
	spolicy, ok := args.Claims[sessionPolicyNameExtracted]
	if !ok {
		return false
	}

	spolicyStr, ok := spolicy.(string)
	if !ok {
		// Sub policy if set, should be a string reject
		// malformed/malicious requests.
		return false
	}

	// Check if policy is parseable.
	subPolicy, err := policy.ParseConfig(bytes.NewReader([]byte(spolicyStr)))
	if err != nil {
		// Log any error in input session policy config.
		logger.LogIf(GlobalContext, err)
		return false
	}

	// This can only happen if policy was set but with an empty JSON.
	if subPolicy.Version == "" && len(subPolicy.Statements) == 0 {
		return isOwnerDerived || combinedPolicy.IsAllowed(parentArgs)
	}

	if subPolicy.Version == "" {
		return false
	return subPolicy.IsAllowed(parentArgs) && (isOwnerDerived || combinedPolicy.IsAllowed(parentArgs))
	return hasSessionPolicy, subPolicy.IsAllowed(args)
			// Credentials will be invalid but and disabled
			// return a different error in such a scenario.
	// 4. Check that svc account can restrict the policy, and that the
	// session policy can be updated.
	c.assertSvcAccSessionPolicyUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)

	// 4. Check that service account's secret key and account status can be
	// updated.
	c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)

	// 4. Check that svc account can restrict the policy, and that the
	// session policy can be updated.
	c.assertSvcAccSessionPolicyUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)

	// 4. Check that service account's secret key and account status can be
	// updated.
	c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)

	// 4. Check that svc account can restrict the policy, and that the
	// session policy can be updated.
	c.assertSvcAccSessionPolicyUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)

	// 4. Check that service account's secret key and account status can be
	// updated.
	c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)

	// 4. Check that svc account can restrict the policy, and that the
	// session policy can be updated.
	c.assertSvcAccSessionPolicyUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)

	// 4. Check that service account's secret key and account status can be
	// updated.
	c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket)


Fixed code:
	// Permission checks:
	//
	// 1. Any type of account (i.e. access keys (previously/still called service
	// accounts), STS accounts, internal IDP accounts, etc) with the
	// policy.UpdateServiceAccountAdminAction permission can update any service
	// account.
	//
	// 2. We would like to let a user update their own access keys, however it
	// is currently blocked pending a re-design. Users are still able to delete
	// and re-create them.
		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
		return
				suite.TestServiceAccountPrivilegeEscalationBug(c)
func (s *TestSuiteIAM) TestServiceAccountPrivilegeEscalationBug(c *check) {
	ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout)
	defer cancel()

	err := s.client.MakeBucket(ctx, "public", minio.MakeBucketOptions{})
	if err != nil {
		c.Fatalf("bucket creat error: %v", err)
	}

	err = s.client.MakeBucket(ctx, "private", minio.MakeBucketOptions{})
	if err != nil {
		c.Fatalf("bucket creat error: %v", err)
	}

	pubPolicyBytes := []byte(`{
 "Version": "2012-10-17",
 "Statement": [
  {
   "Effect": "Allow",
   "Action": [
    "s3:*"
   ],
   "Resource": [
    "arn:aws:s3:::public",
    "arn:aws:s3:::public/*"
   ]
  }
 ]
}`)

	fullS3PolicyBytes := []byte(`{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "s3:*"
      ],
      "Resource": [
        "arn:aws:s3:::*"
      ]
    }
  ]
}
`)

	// Create a service account for the root user.
	cr, err := s.adm.AddServiceAccount(ctx, madmin.AddServiceAccountReq{
		TargetUser: globalActiveCred.AccessKey,
		Policy:     pubPolicyBytes,
	})
	if err != nil {
		c.Fatalf("admin should be able to create service account for themselves %s", err)
	}

	svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "")

	// Check that the service account can access the public bucket.
	buckets, err := svcClient.ListBuckets(ctx)
	if err != nil {
		c.Fatalf("err fetching buckets %s", err)
	}
	if len(buckets) != 1 || buckets[0].Name != "public" {
		c.Fatalf("service account should only have access to public bucket")
	}

	// Create an madmin client with the service account creds.
	svcAdmClient, err := madmin.NewWithOptions(s.endpoint, &madmin.Options{
		Creds:  credentials.NewStaticV4(cr.AccessKey, cr.SecretKey, ""),
		Secure: s.secure,
	})
	if err != nil {
		c.Fatalf("Err creating svcacct admin client: %v", err)
	}
	svcAdmClient.SetCustomTransport(s.TestSuiteCommon.client.Transport)

	// Attempt to update the policy on the service account.
	err = svcAdmClient.UpdateServiceAccount(ctx, cr.AccessKey,
		madmin.UpdateServiceAccountReq{
			NewPolicy: fullS3PolicyBytes,
		})

	if err == nil {
		c.Fatalf("service account should not be able to update policy on itself")
	} else if !strings.Contains(err.Error(), "Access Denied") {
		c.Fatalf("unexpected error: %v", err)
	}
}


		// Get credential information from the request.
	// Add all the necessary claims for the service account.
	// 3. If an inline session-policy is present, evaluate it.
	hasSessionPolicy, isAllowedSP := isAllowedBySessionPolicy(args)
	if hasSessionPolicy {
		return isAllowedSP && (isOwnerDerived || combinedPolicy.IsAllowed(parentArgs))
	// Sub policy not set. Evaluate only the parent policies.
	return (isOwnerDerived || combinedPolicy.IsAllowed(parentArgs))
	// As the session policy exists, even if the parent is the root account, it
	// must be restricted by it. So, we set `.IsOwner` to false here
	// unconditionally.
	sessionPolicyArgs := args
	sessionPolicyArgs.IsOwner = false

	return hasSessionPolicy, subPolicy.IsAllowed(sessionPolicyArgs)
			// Credentials could be valid but disabled - return a different
			// error in such a scenario.