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.

Sign up for GitHub

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

Jump to bottom

kms: Root key rotation for Vault KMS #1457

Merged
merged 2 commits into from
Nov 11, 2024
Merged

kms: Root key rotation for Vault KMS #1457

merged 2 commits into from
Nov 11, 2024

Conversation

iPraveenParihar
Copy link
Contributor

This PR adds support for key Rotation for Vault KMS.

@iPraveenParihar iPraveenParihar marked this pull request as ready for review October 7, 2024 11:30
@nimrod-becker
Copy link
Contributor

@dannyzaken @jackyalbo @romayalon can you PTAL?

@iPraveenParihar
Copy link
Contributor Author

Testing Results -

Configured Vault KMS (vault implementing VersionSingleSecret)

  • secret created and stored in vault
    KMS-Status updated to Sync
  - lastHeartbeatTime: "2024-10-08T08:43:05Z"
    lastTransitionTime: "2024-10-08T08:38:35Z"
    status: vault
    type: KMS-Type
  - lastHeartbeatTime: "2024-10-08T08:43:05Z"
    lastTransitionTime: "2024-10-08T08:38:35Z"
    status: Sync
    type: KMS-Status
  • From vault KMS , secret stored under path NOOBAA_ROOT_SECRET_PATH with key name rootkeyb64-<noobaa-UID>
$ vault kv list vault-test-fff0175734684927973ef4796eab7/NOOBAA_ROOT_SECRET_PATH/                                              
Keys
----
rootkeyb64-f5f3dcfa-69a0-4f57-bcf6-6d75f823fc01
$ vault kv get vault-test-fff0175734684927973ef4796eab7/NOOBAA_ROOT_SECRET_PATH/rootkeyb64-f5f3dcfa-69a0-4f57-bcf6-6d75f823fc01
==================================================== Secret Path ====================================================
vault-test-fff0175734684927973ef4796eab7/data/NOOBAA_ROOT_SECRET_PATH/rootkeyb64-f5f3dcfa-69a0-4f57-bcf6-6d75f823fc01

======= Metadata =======
Key                Value
---                -----
created_time       2024-10-08T08:38:35.105804254Z
custom_metadata    <nil>
deletion_time      n/a
destroyed          false
version            1

========================= Data =========================
Key                                                Value
---                                                -----
rootkeyb64-f5f3dcfa-69a0-4f57-bcf6-6d75f823fc01    7fzTiXYYZg99M4nMNhGMO095Qy0UIyddDieP1hjqXYI=
  • After changing the NooBaa operator image (vault implementing VersionRotatingSecret)
    Now, the secret is stored under root path with key name noobaa-root-master-key-backend
$ vault kv list vault-test-fff0175734684927973ef4796eab7/                                                                      
Keys
----
noobaa-root-master-key-backend

$ vault kv get vault-test-fff0175734684927973ef4796eab7/noobaa-root-master-key-backend                                         
================================ Secret Path ================================
vault-test-fff0175734684927973ef4796eab7/data/noobaa-root-master-key-backend

======= Metadata =======
Key                Value
---                -----
created_time       2024-10-08T08:53:04.215345339Z
custom_metadata    <nil>
deletion_time      n/a
destroyed          false
version            1

============= Data =============
Key                        Value
---                        -----
active_root_key            key-1728377584212169051
key-1728377584212169051    7fzTiXYYZg99M4nMNhGMO095Qy0UIyddDieP1hjqXYI=

The same stored in k8s secrets and mounted to endpoint pod

$ k get secrets noobaa-root-master-key-volume -oyaml
apiVersion: v1
data:
  active_root_key: a2V5LTE3MjgzNzc1ODQyMTIxNjkwNTE=
  key-1728377584212169051: N2Z6VGlYWVlaZzk5TTRuTU5oR01PMDk1UXkwVUl5ZGREaWVQMWhqcVhZST0=
kind: Secret
metadata:
  creationTimestamp: "2024-10-08T08:53:38Z"
  labels:
    app: noobaa
  name: noobaa-root-master-key-volume
  namespace: openshift-storage
  • Enabling Key rotation

operator logs

time="2024-10-08T09:05:01Z" level=info msg="Authenticated to Vault with token\n"
time="2024-10-08T09:05:01Z" level=info msg="setKMSConditionType vault" sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="ReconcileObject: Done - updated Secret noobaa-root-master-key-volume " sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="setKMSConditionStatus Sync" sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="ReconcileKeyRotation, KMS Starting" sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="ReconcileKeyRotation, KMS rotation nextSchedule: 2024-10-08 09:05:00 +0000 UTC" sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="Key rotation started at 2024-10-08 09:05:01.533742832 +0000 UTC m=+716.201718414" sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="Update event detected for noobaa-root-master-key-volume (openshift-storage), queuing Reconcile"
time="2024-10-08T09:05:01Z" level=info msg="ReconcileObject: Done - updated Secret noobaa-root-master-key-volume " sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="setKMSConditionStatus KeyRotate" sys=openshift-storage/noobaa
time="2024-10-08T09:05:01Z" level=info msg="ReconcileKeyRotation, KMS Updating Last Key Rotate time: 2024-10-08 09:05:01.533724531 +0000 UTC m=+716.201700113" sys=openshift-storage/noobaa
  • Update in Vault KMS as version 2
$ vault kv get vault-test-fff0175734684927973ef4796eab7/noobaa-root-master-key-backend
================================ Secret Path ================================
vault-test-fff0175734684927973ef4796eab7/data/noobaa-root-master-key-backend

======= Metadata =======
Key                Value
---                -----
created_time       2024-10-08T09:05:01.821321812Z
custom_metadata    <nil>
deletion_time      n/a
destroyed          false
version            2                     // NOTE: Version 2 is created

============= Data =============
Key                        Value
---                        -----
active_root_key            key-1728378301818071119
key-1728378301818071119    X6GuUG8i+Nf/aEXoido98j1u6lHKIt2pyk4LYocjFpY=
  • Updated in k8s secret
$ k get secrets noobaa-root-master-key-volume -oyaml                                  
apiVersion: v1
data:
  active_root_key: a2V5LTE3MjgzNzgzMDE4MTgwNzExMTk=.    //  key-1728378301818071119
  key-1728377584212169051: N2Z6VGlYWVlaZzk5TTRuTU5oR01PMDk1UXkwVUl5ZGREaWVQMWhqcVhZST0=
  key-1728378301818071119: WDZHdVVHOGkrTmYvYUVYb2lkbzk4ajF1NmxIS0l0MnB5azRMWW9jakZwWT0=
kind: Secret
metadata:
  creationTimestamp: "2024-10-08T08:53:38Z"
  labels:
    app: noobaa
  name: noobaa-root-master-key-volume
  namespace: openshift-storage

@jackyalbo
Copy link
Contributor

jackyalbo commented Oct 15, 2024 •
edited
Loading

Not a Vault expert, will let @dannyzaken review also. But I think we want both versions in the Vault no? Can we still access version 1 after the rotation? I guess we can't rely only on the mounted secret to have all of the key's history in case of some catastrophic issues during the key rotation in core side.

@iPraveenParihar
Copy link
Contributor Author

Not a Vault expert, will let @dannyzaken review also. But I think we want both versions in the Vault no? Can we still access version 1 after the rotation? I guess we can't rely only on the mounted secret to have all of the key's history in case of some catastrophic issues during the key rotation in core side.

@jackyalbo You mean in the vault, we need something like this? with previous secrets?

============= Data =============
Key                        Value
---                        -----
active_root_key            key-1728378301818071119
key-1728378301818071119    X6GuUG8i+Nf/aEXoido98j1u6lHKIt2pyk4LYocjFpY=
key-1728378301818071119    WDZHdVVHOGkrTmYvYUVYb2lkbzk4ajF1NmxIS0l0MnB5azRMWW9jakZwWT0=

@iPraveenParihar
Copy link
Contributor Author

pushed changes and tested. Now the vault will have the previous keys as well

Initial

============= Data =============
Key                        Value
---                        -----
active_root_key            key-1729062082494730928
key-1729062082494730928    FejE+iKr2st/JHRNgR9PB5VFJ5XYLSly8Z186aIS46c=

After KeyRotation

============= Data =============
Key                        Value
---                        -----
active_root_key            key-1729062124774666630
key-1729062082494730928    FejE+iKr2st/JHRNgR9PB5VFJ5XYLSly8Z186aIS46c=
key-1729062124774666630    vOHgHCYi2q5XJ0SGiHhbGe3pxMfMwoCHa1HbOOcxDMo=

Also, tagging @baum as key rotation for k8s (#1071) was done by him.

baum
baum approved these changes Oct 20, 2024
View reviewed changes
Copy link
Contributor

@baum baum left a comment

Choose a reason for hiding this comment

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

lgtm 🖖

@iPraveenParihar
Copy link
Contributor Author

@dannyzaken, can you PTAL?

@iPraveenParihar
Copy link
Contributor Author

Hey @jackyalbo, do we need one more approval ✅ to get PR merged?

@iPraveenParihar
Copy link
Contributor Author

rebased PR

@jackyalbo
Copy link
Contributor

Hey @jackyalbo, do we need one more approval ✅ to get PR merged?

We don't; we just need the tests to pass. I re-ran the failing test.

@jackyalbo jackyalbo merged commit 87b1023 into noobaa:master Nov 11, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackyalbo jackyalbo jackyalbo approved these changes

@dannyzaken dannyzaken dannyzaken approved these changes

@baum baum baum approved these changes

@romayalon romayalon Awaiting requested review from romayalon

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@iPraveenParihar @nimrod-becker @jackyalbo @baum @dannyzaken