Process multiple chains correctly #21

New issue

Closed

opened 2023-11-16 12:54:34 +00:00 by fyrchik · 2 comments

fyrchik commented

2023-11-16 12:54:34 +00:00

Owner

We have multiple chains in the contract, the order is unspecified: we can define it to be ordered by name.
Multiple IAM rules use "process DENY first" rule.

We need to describe this, likely, change the implementation

cc @dkirillov @dstepanov-yadro @aarifullin @alexvanin

1. We have multiple chains in the contract, the order is unspecified: we can _define_ it to be ordered by name. 2. Multiple IAM rules use "process DENY first" rule. We need to describe this, likely, change the implementation cc @dkirillov @dstepanov-yadro @aarifullin @alexvanin

dstepanov-yadro commented

2023-11-16 12:58:53 +00:00

Member

Do not forget about backward compatibility with eacl: for eacl the checks work sequentially.

Do not forget about backward compatibility with `eacl`: for `eacl` the checks work sequentially.

dkirillov commented

2023-11-29 09:13:09 +00:00

Member

Yes from IAM policy evaluation logic

Remember, an explicit deny in any of these policies overrides the allow.

So the following tests should pass (test must be run in iam package).
In test we have two chain:

{
  "Rules": [{
    "Status": 0,
    "Actions": {"Names": ["PutObject"]},
    "Resources": {"Names": ["native:object/*"]},
    "Condition": [{
        "Op": 0,
        "Object": 1,
        "Key": "$Actor:publicKey",
        "Value": "root/user-name/resolvedValue"
    }]
  }]
}

{
  "Rules": [{
      "Status": 2,
      "Actions": {"Names": ["*"]},
      "Resources": {"Names": ["native:object//test-bucket/resolvedValues/*"]},
      "Condition": [{
          "Op": 4,
          "Object": 1,
          "Key": "$Actor:publicKey",
          "Value": "*"
      }]
  }]
}

that applied in mentioned order and expectation that we get access denied. The same expectation for one chain with multiple rules:

{
  "Rules": [
    {
      "Status": 0,
      "Actions": {"Names": ["PutObject"]},
      "Resources": {"Names": ["native:object/*"]},
      "Condition": [{
          "Op": 0,
          "Object": 1,
          "Key": "$Actor:publicKey",
          "Value": "root/user-name/resolvedValue"
      }]
    },
    {
      "Status": 2,
      "Actions": {"Names": ["*"]},
      "Resources": {"Names": ["native:object//test-bucket/resolvedValues/*"]},
      "Any": false,
      "Condition": [{
          "Op": 4,
          "Object": 1,
          "Key": "$Actor:publicKey",
          "Value": "*"
      }]
    }
  ]
}


func TestProcessDenyFirst(t *testing.T) {
	identityBasedPolicyStr := `
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
			"Principal": {
				"AWS": [ "arn:aws:iam::root:user/user-name" ]
			},
            "Action": ["s3:PutObject" ],
            "Resource": "arn:aws:s3:::*"
        }
    ]
}
`

	resourceBasedPolicyStr := `
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Deny",
            "Principal": "*",
            "Action": "s3:*",
            "Resource": [ "arn:aws:s3:::test-bucket/*" ]
        }
    ]
}
`

	var identityPolicy Policy
	err := json.Unmarshal([]byte(identityBasedPolicyStr), &identityPolicy)
	require.NoError(t, err)

	var resourcePolicy Policy
	err = json.Unmarshal([]byte(resourceBasedPolicyStr), &resourcePolicy)
	require.NoError(t, err)

	mockResolver := newMockUserResolver([]string{"root/user-name"}, []string{"test-bucket"})

	identityNativePolicy, err := ConvertToNativeChain(identityPolicy, mockResolver)
	require.NoError(t, err)

	resourceNativePolicy, err := ConvertToNativeChain(resourcePolicy, mockResolver)
	require.NoError(t, err)

	s := inmemory.NewInMemory()

	target := engine.NamespaceTarget("ns")

	err = s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, target, identityNativePolicy)
	require.NoError(t, err)

	err = s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, target, resourceNativePolicy)
	require.NoError(t, err)

	resource := testutil.NewResource(fmt.Sprintf(native.ResourceFormatRootContainerObjects, mockResolver.buckets["test-bucket"]), nil)
	request := testutil.NewRequest("PutObject", resource, map[string]string{native.PropertyKeyActorPublicKey: mockResolver.users["root/user-name"]})

	status, found, err := s.IsAllowed(chain.Ingress, "ns", request)
	require.NoError(t, err)
	require.True(t, found)
	require.Equal(t, chain.AccessDenied, status)
}

Yes from IAM [policy evaluation logic](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_evaluation-logic.html#policy-eval-denyallow) > Remember, an explicit deny in any of these policies overrides the allow. So the following tests should pass (test must be run in `iam` package). In test we have two chain: ```json { "Rules": [{ "Status": 0, "Actions": {"Names": ["PutObject"]}, "Resources": {"Names": ["native:object/*"]}, "Condition": [{ "Op": 0, "Object": 1, "Key": "$Actor:publicKey", "Value": "root/user-name/resolvedValue" }] }] } ``` ```json { "Rules": [{ "Status": 2, "Actions": {"Names": ["*"]}, "Resources": {"Names": ["native:object//test-bucket/resolvedValues/*"]}, "Condition": [{ "Op": 4, "Object": 1, "Key": "$Actor:publicKey", "Value": "*" }] }] } ``` that applied in mentioned order and expectation that we get access denied. The same expectation for one chain with multiple rules: ```json { "Rules": [ { "Status": 0, "Actions": {"Names": ["PutObject"]}, "Resources": {"Names": ["native:object/*"]}, "Condition": [{ "Op": 0, "Object": 1, "Key": "$Actor:publicKey", "Value": "root/user-name/resolvedValue" }] }, { "Status": 2, "Actions": {"Names": ["*"]}, "Resources": {"Names": ["native:object//test-bucket/resolvedValues/*"]}, "Any": false, "Condition": [{ "Op": 4, "Object": 1, "Key": "$Actor:publicKey", "Value": "*" }] } ] } ``` ```golang func TestProcessDenyFirst(t *testing.T) { identityBasedPolicyStr := ` { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Principal": { "AWS": [ "arn:aws:iam::root:user/user-name" ] }, "Action": ["s3:PutObject" ], "Resource": "arn:aws:s3:::*" } ] } ` resourceBasedPolicyStr := ` { "Version": "2012-10-17", "Statement": [ { "Effect": "Deny", "Principal": "*", "Action": "s3:*", "Resource": [ "arn:aws:s3:::test-bucket/*" ] } ] } ` var identityPolicy Policy err := json.Unmarshal([]byte(identityBasedPolicyStr), &identityPolicy) require.NoError(t, err) var resourcePolicy Policy err = json.Unmarshal([]byte(resourceBasedPolicyStr), &resourcePolicy) require.NoError(t, err) mockResolver := newMockUserResolver([]string{"root/user-name"}, []string{"test-bucket"}) identityNativePolicy, err := ConvertToNativeChain(identityPolicy, mockResolver) require.NoError(t, err) resourceNativePolicy, err := ConvertToNativeChain(resourcePolicy, mockResolver) require.NoError(t, err) s := inmemory.NewInMemory() target := engine.NamespaceTarget("ns") err = s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, target, identityNativePolicy) require.NoError(t, err) err = s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, target, resourceNativePolicy) require.NoError(t, err) resource := testutil.NewResource(fmt.Sprintf(native.ResourceFormatRootContainerObjects, mockResolver.buckets["test-bucket"]), nil) request := testutil.NewRequest("PutObject", resource, map[string]string{native.PropertyKeyActorPublicKey: mockResolver.users["root/user-name"]}) status, found, err := s.IsAllowed(chain.Ingress, "ns", request) require.NoError(t, err) require.True(t, found) require.Equal(t, chain.AccessDenied, status) } ```