auth/aws: Make disallow_reauthentication and allow_instance_migration mutually exclusive (#3291)

CHANGELOG.md

@@ -159,7 +159,7 @@ IMPROVEMENTS:
 
  * audit/file: Allow specifying `stdout` as the `file_path` to log to standard
    output [GH-3235]
- * auth/aws: Allow wildcards in `bound_iam_principal_id` [GH-3213]
+ * auth/aws: Allow wildcards in `bound_iam_principal_arn` [GH-3213]
  * auth/okta: Compare groups case-insensitively since Okta is only
    case-preserving [GH-3240]
  * auth/okta: Standardize Okta configuration APIs across backends [GH-3245]

builtin/credential/aws/path_role.go

@@ -663,6 +663,10 @@ func (b *backend) pathRoleCreateUpdate(
 		roleEntry.AllowInstanceMigration = data.Get("allow_instance_migration").(bool)
 	}
 
+	if roleEntry.AllowInstanceMigration && roleEntry.DisallowReauthentication {
+		return logical.ErrorResponse("cannot specify both disallow_reauthentication=true and allow_instance_migration=true"), nil
+	}
+
 	var resp logical.Response
 
 	ttlRaw, ok := data.GetOk("ttl")

builtin/credential/aws/path_role_tag.go

@@ -124,6 +124,10 @@ func (b *backend) pathRoleTagUpdate(
 		resp.AddWarning("Role does not allow instance migration. Login will not be allowed with this tag unless the role value is updated.")
 	}
 
+	if disallowReauthentication && allowInstanceMigration {
+		return logical.ErrorResponse("cannot set both disallow_reauthentication and allow_instance_migration"), nil
+	}
+
 	// max_ttl for the role tag should be less than the max_ttl set on the role.
 	maxTTL := time.Duration(data.Get("max_ttl").(int)) * time.Second
 

builtin/credential/aws/path_role_test.go

@@ -66,12 +66,25 @@ func TestBackend_pathRoleEc2(t *testing.T) {
 		Data:      data,
 		Storage:   storage,
 	})
-	if resp != nil && resp.IsError() {
-		t.Fatalf("failed to create role: %s", resp.Data["error"])
-	}
 	if err != nil {
 		t.Fatal(err)
 	}
+	if resp == nil || !resp.IsError() {
+		t.Fatalf("expected failure to create role with both allow_instance_migration true and disallow_reauthentication true")
+	}
+	data["disallow_reauthentication"] = false
+	resp, err = b.HandleRequest(&logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "role/ami-abcd123",
+		Data:      data,
+		Storage:   storage,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp != nil && resp.IsError() {
+		t.Fatalf("failure to update role: %v", resp.Data["error"])
+	}
 	resp, err = b.HandleRequest(&logical.Request{
 		Operation: logical.ReadOperation,
 		Path:      "role/ami-abcd123",
@@ -80,8 +93,12 @@ func TestBackend_pathRoleEc2(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !resp.Data["allow_instance_migration"].(bool) || !resp.Data["disallow_reauthentication"].(bool) {
-		t.Fatal("bad: expected:true got:false\n")
+	if !resp.Data["allow_instance_migration"].(bool) {
+		t.Fatal("bad: expected allow_instance_migration:true got:false\n")
+	}
+
+	if resp.Data["disallow_reauthentication"].(bool) {
+		t.Fatal("bad: expected disallow_reauthentication: false got:true\n")
 	}
 
 	// add another entry, to test listing of role entries
@@ -529,7 +546,7 @@ func TestAwsEc2_RoleCrud(t *testing.T) {
 		"ttl":                       "10m",
 		"max_ttl":                   "20m",
 		"policies":                  "testpolicy1,testpolicy2",
-		"disallow_reauthentication": true,
+		"disallow_reauthentication": false,
 		"hmac_key":                  "testhmackey",
 		"period":                    "1m",
 	}
@@ -567,7 +584,7 @@ func TestAwsEc2_RoleCrud(t *testing.T) {
 		"ttl":                       time.Duration(600),
 		"max_ttl":                   time.Duration(1200),
 		"policies":                  []string{"testpolicy1", "testpolicy2"},
-		"disallow_reauthentication": true,
+		"disallow_reauthentication": false,
 		"period":                    time.Duration(60),
 	}
 

website/source/api/auth/aws/index.html.md

@@ -651,12 +651,14 @@ inferencing configuration of that role.
   the metadata document, so essentially, this disables the client nonce check
   whenever the instance is migrated to a new host and pendingTime is newer than
   the previously-remembered time. Use with caution. This only applies to
-  authentications via the ec2 auth method.
+  authentications via the ec2 auth method. This is mutually exclusive with
+  `disallow_reauthentication`.
 - `disallow_reauthentication` `(bool: false)` - If set, only allows a single
   token to be granted per instance ID. In order to perform a fresh login, the
   entry in whitelist for the instance ID needs to be cleared using
   'auth/aws/identity-whitelist/<instance_id>' endpoint. Defaults to 'false'.
-  This only applies to authentications via the ec2 auth method.
+  This only applies to authentications via the ec2 auth method. This is mutually
+  exclusive with `allow_instance_migration`.
 
 ### Sample Payload
 
@@ -812,9 +814,11 @@ given instance can be allowed to gain in a worst-case scenario.
   the metadata document, so essentially, this disables the client nonce check
   whenever the instance is migrated to a new host and pendingTime is newer than
   the previously-remembered time. Use with caution. Defaults to 'false'.
+  Mutually exclusive with `disallow_reauthentication`.
 - `disallow_reauthentication` `(bool: false)` - If set, only allows a single
   token to be granted per instance ID. This can be cleared with the
-  auth/aws/identity-whitelist endpoint. Defaults to 'false'.
+  auth/aws/identity-whitelist endpoint. Defaults to 'false'. Mutually exclusive
+  with `allow_instance_migration`.
 
 ### Sample Payload
 

website/source/docs/auth/aws.html.md

@@ -313,10 +313,10 @@ in which Vault might make an AWS API call, but rather illustrative of why these
 are needed.
 
 * `ec2:DescribeInstances` is necessary when you are using the `ec2` auth method
-  or when you are inferring an `ec2_instance` entity type to validate the EC2
-  instance meets binding requirements of the role
+  or when you are inferring an `ec2_instance` entity type to validate that the
+  EC2 instance meets binding requirements of the role
 * `iam:GetInstanceProfile` is used when you have a `bound_iam_role_arn` in the
-  ec2 auth method. Vault needs determine which IAM role is attached to the
+  `ec2` auth method. Vault needs to determine which IAM role is attached to the
   instance profile.
 * `iam:GetUser` and `iam:GetRole` are used when using the iam auth method and
   binding to an IAM user or role principal to determine the unique AWS user ID