Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disk Encryption Set Module does not work correctly #20995

Closed
1 task done
cshea-msft opened this issue Mar 17, 2023 · 10 comments
Closed
1 task done

Disk Encryption Set Module does not work correctly #20995

cshea-msft opened this issue Mar 17, 2023 · 10 comments

Comments

@cshea-msft
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

1.4.0

AzureRM Provider Version

3.47.0

Affected Resource(s)/Data Source(s)

azurerm_disk_encryption_set

Terraform Configuration Files

provider "azurerm" {
  features {
    key_vault {
      purge_soft_delete_on_destroy = true
    }
  }
}

data "azurerm_client_config" "current" {}

// Create a resource group for the Keyvault
resource "azurerm_resource_group" "rg" {
  name     = "rg-kv-${var.location}-${var.environment}"
  location = var.location
  tags     = var.tags
}

// Create a Keyvault
resource "azurerm_key_vault" "kv" {
  name                        = "kv-${var.prefix}-${var.environment}"
  location                    = var.location
  resource_group_name         = azurerm_resource_group.rg.name
  tenant_id                   = data.azurerm_client_config.current.tenant_id
  sku_name                    = "premium"
  enabled_for_disk_encryption = true
  purge_protection_enabled    = true
  tags                        = var.tags
}

// Create a Keyvault Key
resource "azurerm_key_vault_key" "kv_key" {
  name         = "kv-key-${var.prefix}-${var.location}-${var.environment}"
  key_vault_id = azurerm_key_vault.kv.id
  key_type     = "RSA"
  key_size     = 2048
  depends_on = [
    azurerm_key_vault_access_policy.kv_policy_user
  ]
  key_opts = [
    "decrypt",
    "encrypt",
    "sign",
    "unwrapKey",
    "verify",
  "wrapKey"]
}

// Create a Disk Encryption Set
resource "azurerm_disk_encryption_set" "dse" {
  name                = "dse-${var.prefix}-${var.environment}"
  resource_group_name = azurerm_resource_group.rg.name
  location            = var.location
  key_vault_key_id    = azurerm_key_vault_key.kv_key.id
  encryption_type     = "EncryptionAtRestWithCustomerKey"
  tags                = var.tags
  identity {
    type = "SystemAssigned"
  }
}

// Create a Keyvault Access Policy for the Disk
resource "azurerm_key_vault_access_policy" "kv_policy_disk" {
  key_vault_id    = azurerm_key_vault.kv.id
  tenant_id       = azurerm_disk_encryption_set.dse.identity.0.tenant_id
  object_id       = azurerm_disk_encryption_set.dse.identity.0.principal_id
  key_permissions = ["Create", "Delete", "Get", "List", "Update", "Purge", "Recover", "Decrypt", "Sign"]
}

// Create a Keyvault Access Policy for the User
resource "azurerm_key_vault_access_policy" "kv_policy_user" {
  key_vault_id    = azurerm_key_vault.kv.id
  tenant_id       = data.azurerm_client_config.current.tenant_id
  object_id       = data.azurerm_client_config.current.object_id
  key_permissions = ["Get", "Create", "Delete", "Purge", "Recover", "Update", "List", "Decrypt", "Sign", "GetRotationPolicy"]
}

// Create a role assignment for the Keyvault
resource "azurerm_role_assignment" "kv_role" {
  scope                = azurerm_key_vault.kv.id
  role_definition_name = "Key Vault Crypto Service Encryption User"
  principal_id         = azurerm_disk_encryption_set.dse.identity.0.principal_id
}

Debug Output/Panic Output

will attach

Expected Behaviour

should deploy out key vault, key vault key, key vault policy, disk encryption set.

Actual Behaviour

│ Error: current client lacks permissions to read Key Rotation Policy for Key "kv-key-chashea-eastus-dev" ("Vault: (Name "kv-chashea-dev" / Resource Group "rg-kv-eastus-dev")", Vault url: "https://kv-chashea-dev.vault.azure.net/"), please update this as described here: https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/key_vault_key#example-usage : keyvault.BaseClient#GetKeyRotationPolicy: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="The user, group or application 'appid=04b07795-8ddb-461a-bbee-02f9e1bf7b46;oid=b6ea9d10-396e-4760-80f4-4336b0b1a990;numgroups=1;iss=https://sts.windows.net/5c5e1a56-251f-44b1-8f67-c97243f9e7cb/' does not have keys getrotationpolicy permission on key vault 'kv-chashea-dev;location=eastus'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"ForbiddenByPolicy"}

│ with module.dse.azurerm_key_vault_key.kv_key,
│ on Modules\dse\kv.tf line 31, in resource "azurerm_key_vault_key" "kv_key":
│ 31: resource "azurerm_key_vault_key" "kv_key" {

Steps to Reproduce

terraform init
terraform plan

Important Factoids

No response

References

following this doc here. https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/disk_encryption_set

@cshea-msft cshea-msft added the bug label Mar 17, 2023
@github-actions github-actions bot removed the bug label Mar 17, 2023
@cshea-msft
Copy link
Author

terraform.log

@cshea-msft
Copy link
Author

Follow exactly what the doc shows so not sure why it is giving me errors.

@meizenga
Copy link

meizenga commented Mar 17, 2023

Can you try this?

resource "azurerm_key_vault_access_policy" "kv_policy_disk" {
  key_vault_id    = azurerm_key_vault.kv.id
  tenant_id       = azurerm_disk_encryption_set.dse.identity.0.tenant_id
  object_id       = azurerm_disk_encryption_set.dse.identity.0.principal_id
  key_permissions = ["Create", "Delete", "Get", "List", "Update", "Purge", "Recover", "Decrypt", "Sign", "WrapKey", "UnwrapKey"]
}

Adding WrapKey and UnwrapKey fixed the access issue for me.

@cshea-msft
Copy link
Author

Thanks @meizenga, i also had to add Verify permission as well for the VM i was deploying. Can you make changes to the module doc that adds wrapkey, unwrap key and verify to those key permissions?

@meizenga
Copy link

meizenga commented Mar 17, 2023

@cshea15 you can add the Label Documentation to this issue. Hopefully someone picks it up.

btw. I used v3.48 and I didn't require the Verify permission.

@myc2h6o
Copy link
Contributor

myc2h6o commented Mar 20, 2023

This seems to be caused by https://github.com/hashicorp/terraform-provider-azurerm/pull/19113/files#diff-a0ea8d81165c29e329a938b613536ff9cd587c0130c040c927ff7094c984c0f3R590-R595 which requires the new permission from the client. It is not possible to update a previously created azurerm_key_vault_access_policy and azurerm_key_vault_key because the plan will trigger a Read for azurerm_key_vault_key and this will fail due to the fact that client is lacking the GetRotationPolicy permission. I think we need to take into consideration the update scenario here, and as a temporary mitigation, @cshea15 you can use the old provider versions before v3.45.0 to update the newly required permissions, then it shall work fine in the new version.

@myc2h6o
Copy link
Contributor

myc2h6o commented Mar 20, 2023

There was a discussion about the breaking the existing resources in #19113 (comment)
@aristosvo @katbyte shall we ignore the 403 read failure or move it behind the 4.0 flag so that it won't break the existing resources? The previously created azurerm_key_vault_access_policy and azurerm_key_vault_key would fail at plan phase due to the lack of new GetRotationPolicy permission.

@cshea-msft
Copy link
Author

@myc2h6o I was able to get it to work when I added the GetRotationPolicy and Veriy policy.

@tombuildsstuff
Copy link
Contributor

@myc2h6o

@aristosvo @katbyte shall we ignore the 403 read failure or move it behind the 4.0 flag so that it won't break the existing resources? The previously created azurerm_key_vault_access_policy and azurerm_key_vault_key would fail at plan phase due to the lack of new GetRotationPolicy permission.

That would introduce a regression, since we then wouldn't be tracking this field, which would be misleading to users - instead this can be fixed by adding the relevant permission as @cshea15 has done.

In retrospect this permission probably wanted documenting under the upgrade notes for this version of the Provider - however unfortunately the release notes are immutable so that's not something we can change at this time.

However since it appears that the issue has been resolved by adding the new permission, I'm going to close this for the moment.

Thanks!

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants