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

azure key vault plugin #4458

Merged
merged 3 commits into from
Sep 4, 2023
Merged

Conversation

moe-omar
Copy link
Contributor

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.

@amartinezfayo
Copy link
Member

Thank you @moe-omar for this contribution!

I've added a commit on top of the original contribution to:

  • Change the implementation to create a new key when we need to generate a new one instead of adding a new version to an existing key. Once the key is created, the old one is deleted. Since key versions can't be deleted in Key Vault, this allows us to delete rotated keys.
  • Avoid disposing keys that belong to the current server.
  • Rename the plugin to azure_key_vault.
  • Fix tests and linter.

| Key | Type | Required | Description | Default |
|-------------------|---------|---------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------|---------|
| key_vault_uri | string | Yes | The Key Vault URI where the keys managed by this plugin reside. | "" |
| key_metadata_file | string | yes | A file path location where key metadata used by the plugin will be persisted. See "[Management of keys](#management-of-keys)" for more information. | "" |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you keep the same order that in server_full.conf?

}

// Skip keys that belong to this server
if p.serverID == *key.Tags[tagNameServerID] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add more comments about why are we removing the keys that belongs to another server?
and no only taking care of own keys?

moe-omar and others added 3 commits September 4, 2023 10:58
Signed-off-by: mo omar <momar@confluent.io>
- Change the implementation to create a new key when we need to generate a new one instead of adding a new version to an existing key. Once the key is created, the old one is deleted. Since key versions can't be deleted in Key Vault, this allows us to delete rotated keys.
- Avoid disposing keys that belong to the current server.
- Rename the plugin to azure_key_vault.
- Fix tests and linter.

Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
@MarcosDY MarcosDY merged commit 6b22b00 into spiffe:main Sep 4, 2023
33 checks passed
@MarcosDY MarcosDY added this to the 1.8.0 milestone Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants