Azure KMS (Key Vault) Server KeyManager plugin

[moe-omar]

This PR adds a server KMS plugin that uses Azure's Key Vault.

Pull Request check list

• Commit conforms to CONTRIBUTING.md?
• Proper tests/regressions included?
• Documentation updated?

Affected functionality
No existing functionality affected. New server plugin added

Description of change
A new server keymanager plugin that uses Azure's Key Vault.

What/How was this new change tested?

• Tested using all the supported key types (rsa-2048, rsa-4096, ec-p256, and ec-p384) for both JWT keys and CA keys.
• Tested key rotation, refresh and keys cleanup for stale/unused keys belonging to the trust domain the server belongs to.
• Tested authentication using Azure client/application credentials as well as using MSI in an Azure VM.
• This involved running the server, registering agents and workloads and making sure SVID's are issued (signed by Key Vault keys) and can be validated using the bundle.
• Added unit tests.

[MarcosDY]

Looks great!!!

[MarcosDY]

you can use require.EqualError() that validate error is not nil

[moe-omar]

These are strings we're comparing here though not errors

[amartinezfayo]

Thank you very much @moe-omar for this contribution! We really appreciate the effort put in this.

I have a couple of concerns:

1. With the current logic, it seems that key versions will be kept in the Key Vault forever, unless the server starts with a different server ID (e.g. the metadata file is lost).
   This may not be super critical from the costing perspective, since the pricing model of the service is based on the number of transactions performed rather than the number of objects maintained. Still, it's concerning to me to keep keys that have been rotated and shouldn't be usable anymore.

2. It doesn't seem that is possible to customize the access control of the keys created by SPIRE. This has been proved to be problematic in the past with other providers (see Make the KMS Key manager to create keys using a role-based policy when possible #2424 for details). Could we support influencing the access control of keys managed by the plugin?

[amartinezfayo]

I think that it may be problematic to keep key versions even if we know that they have been rotated.
Keys will only be deleted if the metadata file is lost, otherwise the server will continue to keep current keys active and never delete any of the old versions.

[moe-omar]

Yes that's correct, when a key is rotated, the older versions are still active but they won't be used by the SPIRE server (the plugin will only use the current version). So even though the older versions are active, they are not usable in the SPIRE context.

[moe-omar]

Thank you very much @moe-omar for this contribution! We really appreciate the effort put in this.

I have a couple of concerns:

1. With the current logic, it seems that key versions will be kept in the Key Vault forever, unless the server starts with a different server ID (e.g. the metadata file is lost).
   This may not be super critical from the costing perspective, since the pricing model of the service is based on the number of transactions performed rather than the number of objects maintained. Still, it's concerning to me to keep keys that have been rotated and shouldn't be usable anymore.
2. It doesn't seem that is possible to customize the access control of the keys created by SPIRE. This has been proved to be problematic in the past with other providers (see Make the KMS Key manager to create keys using a role-based policy when possible #2424 for details). Could we support influencing the access control of keys managed by the plugin?

Thanks @amartinezfayo !

For 1, Key Vault doesn't support deleting specific key versions unfortunately, but I think it's possible to "disable" a key version (it can be reenabled though). You think that would be good enough?

Regarding 2, I looked at the original issue that motivated adding policies to AWS KMS keys, I think it's a little different with Key Vault. Policies are set at the Key Vault level not individual keys like in AWS KMS and I also don't think that you get a default policy when you create the Key Vault itself (I had to create a policy to give SPIRE access to keys in the Key Vault). So the way I think this would work is that a Key Vault + access policies to give SPIRE the appropriate access are provisioned separately (via Terraform for example) and the plugin is then configured to use the Key Vault.

Let me know if you think I'm missing something.

[amartinezfayo]

Key Vault doesn't support deleting specific key versions unfortunately, but I think it's possible to "disable" a key version (it can be reenabled though). You think that would be good enough?

I think that we should try to avoid having the plugin creating key versions indefinitely. Key Vault does not restrict the number of versions on a key, but storing a large number of versions may have impact in the performance of operations, and may be confusing to still have them showing in the console.
I would rather prefer that we delete the rotated keys. They will still be recoverable for a retention period but will not be showed by default in the console (you will only see them if you choose to manage the deleted keys).
We would create a new key when we need to generate a new one, and once it's created, we can delete the old one. What do you think?

Policies are set at the Key Vault level not individual keys like in AWS KMS and I also don't think that you get a default policy when you create the Key Vault itself (I had to create a policy to give SPIRE access to keys in the Key Vault). So the way I think this would work is that a Key Vault + access policies to give SPIRE the appropriate access are provisioned separately (via Terraform for example) and the plugin is then configured to use the Key Vault.

While I believe that the access control in Key Vault can also be defined at the key level (that's the resource scope in the case of Key Vault - https://learn.microsoft.com/en-us/azure/role-based-access-control/role-assignments-portal#step-1-identify-the-needed-scope) I agree that since there is no default policy applied, the situation is different than the other KMS plugins so it seems that we should be good here.

[amartinezfayo]

@moe-omar are you still willing to finalize this implementation?
The SPIRE project has now access to Azure resources, which leaves us in a better position to pick up this work if needed.

Something that I haven't realized before is that the commits are not signed off, which we will need in order to be able to merge this. Please refer to https://github.com/apps/dco for more information about signing-off your commits.

Could you please have the commits signed-off and let us know how would you like to proceed with this work? Thanks!

[amartinezfayo]

Closing in favor of #4458. Thank you @moe-omar for this contribution!