feat: verify vault cluster identity by Raven-182 · Pull Request #33 · openstack-charmers/vaultlocker

Raven-182 · 2026-09-27T23:19:06Z

Depends on feat: make Vaultlocker device operations safe to repeat #32. This branch will be rebased when that PR merges.

Summary

After a successful AppRole login, Vaultlocker reads the cluster ID. On the first run, it saves the ID beside the configuration file. On later runs, it compares the current ID with the saved one.

If the IDs differ, encrypt and enroll stop before changing a device or storing a key. decrypt logs a warning and tries to unlock using the vault connection if it finds a key. This also applies to boot-time unlocking.

Why

A Vault URL can change or point to a different cluster. New device keys should not be stored there while existing keys may still be in the original cluster.

Decrypting uses an existing key. If that key is available and unlocks the device, a cluster ID change should not prevent boot-time unlocking.

Changes

Authenticate to vault, then read and validate the cluster ID from Vault’s seal status.
Save one cluster ID per configuration file as <config-path>.cluster-id.
Stop encrypt and enroll when the cluster ID differs from the saved ID.
Warn and attempt decrypt on a mismatch.
Add unit and Vault-backed functional tests for pinning and mismatches.
Configure coverage reports to exclude tests and require full coverage of production code.

Read the cluster ID from Vault's seal status and reject missing, empty, non-string, or non-object responses with an error. Assisted-by: OpenCode (DeepSeek v4.1 Flash) Co-authored-by: Raven Kaur <raven.kaur@canonical.com> Co-authored-by: Lucas Payne <lucas.payne@canonical.com> Co-authored-by: Heather Lanigan <heather.lanigan@canonical.com> Signed-off-by: Billy Olsen <billy.olsen@canonical.com> Signed-off-by: Raven Kaur <raven.kaur@canonical.com>

LucasAPayne · 2026-10-01T20:17:35Z

+        _login_to_vault(client, config)
+        try:
+            _verify_cluster_identity(client, args.config)
+        except exceptions.ClusterIdentityMismatchError as mismatch_error:
+            if fail_on_cluster_mismatch:
+                raise
+            logger.warning('Cluster identity mismatch: %s', mismatch_error)


suggestion: According to the hvac docs, read_seal_status() reads from an unauthenticated endpoint, so the login should not be necessary since that's what _verify_cluster_identity() calls. If login is needed for other actions, it can be moved after verifying the cluster's identity.

So on the first run we create the cluster id pin. If we verify the cluster before authenticating, we could end up pinning a vault that we were never able to authenticate to. If login fails, we should not pin a Vault that these credentials could not access. I will add a comment to clarify this order.

Thanks for clarifying

LucasAPayne · 2026-10-01T20:29:43Z

+class ClusterIdentityMismatchError(ClusterIdentityError):
+    """The connected Vault cluster does not match the saved cluster."""
+
+    def __init__(self, expected, actual):
+        super().__init__(
+            "Vault cluster identity mismatch: pinned cluster_id={}, "
+            "observed cluster_id={}".format(expected, actual)
+        )


todo: A user may not be aware of the sidecar file used to verify the cluster ID. We should add remediation steps to this message. For example: "If this change is expected, remove {pin_path} and re-run."

todo: In addition to suggesting how to correct the issue in this message, we should also add some documentation (in the README for now) of steps to take when migrating Vault.

I don't think we should suggest removing the pin. A mismatch can mean the original Vault still holds keys for devices we already manage. If we remove the pin, new operations could start using a different Vault without those keys being migrated. Vault migration is currently out of scope but it may be worth documenting this in the README.

That makes sense, thanks. My primary concern was having some way to advertise that the pin file exists and what it's for.

LucasAPayne · 2026-10-01T20:48:37Z

+        _luks_format.assert_not_called()
+        _boot_unlock.register.assert_not_called()


question: Why are these functions being asserted to not be called? They seem unrelated to decryption, but I'm unsure if I'm missing something or if there are some extraneous asserts in some of the tests.

Thanks for catching, these assertions are stale, I will remove them

LucasAPayne · 2026-10-01T20:53:34Z

+    def test_cluster_mismatch_warns_and_attempts_decrypts(
+            self, _luks_open, _luks_format, _boot_unlock,
+            _udevadm_rescan, _udevadm_settle):


nit: Remove the extra "s" at the end of _decrypts

LucasAPayne · 2026-10-02T19:39:09Z

+After authentication, vaultlocker saves the Vault cluster ID in ``<config-path>.cluster_id``.
+If the ID later changes, ``encrypt`` and ``enroll`` operations will fail with an error.
+``decrypt`` will log a warning and try to find the key in the observed Vault cluster. If
+that Vault does not have the key, vaultlocker cannot unlock the device.
+
+If the change is unexpected, check the Vault url and restore the connection to
+the original cluster.  Deleting the pin does not move existing keys.
+Vaultlocker does not support automatically migrating keys between clusters.
+


Perfect, thanks!

hemanthnakkina

minor nit on readme, otherwise LGTM

hemanthnakkina · 2026-10-05T23:44:33Z

 that a CIDR based ACL is in use to only allow permitted systems within the
 Data Center to login and retrieve secrets from Vault.

+After authentication, vaultlocker saves the Vault cluster ID in ``<config-path>.cluster_id``.


<config-path>.cluster-id hyphen instead of underscore

Updated, thanks

hmlanigan

Looks good once the small change is made.

Create one cluster pin beside each configuration file on first use. Authenticate with AppRole then verify the pin before starting a device operation. Assisted-by: OpenCode (DeepSeek v4.1 Flash) Co-authored-by: Billy Olsen <billy.olsen@canonical.com> Co-authored-by: Lucas Payne <lucas.payne@canonical.com> Co-authored-by: Heather Lanigan <heather.lanigan@canonical.com> Signed-off-by: Billy Olsen <billy.olsen@canonical.com> Signed-off-by: Raven Kaur <raven.kaur@canonical.com>

Require 90% line and branch coverage for production modules while excluding the test packages. Assisted-by: OpenCode (DeepSeek v4.1 Flash) Co-authored-by: Raven Kaur <raven.kaur@canonical.com> Co-authored-by: Lucas Payne <lucas.payne@canonical.com> Co-authored-by: Heather Lanigan <heather.lanigan@canonical.com> Signed-off-by: Billy Olsen <billy.olsen@canonical.com> Signed-off-by: Raven Kaur <raven.kaur@canonical.com>

Raven-182 force-pushed the vault-cluster-identity branch 2 times, most recently from 0cf28e3 to 57a3bf7 Compare September 29, 2026 15:03

Raven-182 requested review from LucasAPayne and hmlanigan and removed request for LucasAPayne September 29, 2026 15:22

Raven-182 force-pushed the vault-cluster-identity branch from 57a3bf7 to fc3efac Compare September 30, 2026 16:00

Raven-182 marked this pull request as ready for review October 1, 2026 18:08

Raven-182 force-pushed the vault-cluster-identity branch 2 times, most recently from 13e2808 to 2b31f00 Compare October 1, 2026 18:36

LucasAPayne reviewed Oct 1, 2026

View reviewed changes

Raven-182 force-pushed the vault-cluster-identity branch from 2b31f00 to ba32eef Compare October 2, 2026 14:37

Raven-182 requested a review from LucasAPayne October 2, 2026 14:38

LucasAPayne approved these changes Oct 2, 2026

View reviewed changes

Raven-182 requested review from freyes, gboutry and hemanthnakkina October 5, 2026 18:57

hemanthnakkina reviewed Oct 5, 2026

View reviewed changes

hmlanigan reviewed Oct 6, 2026

View reviewed changes

Raven-182 and others added 2 commits October 6, 2026 14:07

Raven-182 force-pushed the vault-cluster-identity branch from ba32eef to 063d320 Compare October 6, 2026 18:08

		_luks_format.assert_not_called()
		_boot_unlock.register.assert_not_called()

Conversation

Raven-182 commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hemanthnakkina left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hmlanigan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Raven-182 commented Sep 27, 2026 •

edited

Loading