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

✨ feat(VSecM Safe): #460 Manual Root Key Updates K8s Secret #725

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/safe/internal/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func CreateCryptoKey(id *string, updatedSecret chan<- bool) {

log.InfoLn(id, "Generated public key, private key, and aes seed")

if err = persistKeys(privateKey, publicKey, aesSeed); err != nil {
if err = PersistKeys(privateKey, publicKey, aesSeed); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment here?

Something like:

// Save the key to VSecM Safe's memory, and also save them to 
// VSecM Safe's root key Kubernetes Secret.
// If the root key input mode is not manual, VSecM Safe will
// use a trusted backing store to retrieve the key in case of
// Pod crash or eviction. As of Mar,17, 2024 the only trusted
// backing store is the VSecM root key Kubernetes Secret; however
// this will change in the future.
if err = PersistKeys(privateKey, publicKey, aesSeed); err != nil {

Copy link
Contributor

Choose a reason for hiding this comment

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

Reasoning: I always forget why we didn’t do a "IsManualRootKeySavedAsK8sSecret()" check here; so it’s reminder to me, and also anyone who thinks the same too :) .

log.FatalLn(id, "Failed to persist keys", err.Error())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"k8s.io/client-go/rest"
)

func persistKeys(privateKey, publicKey, aesSeed string) error {
func PersistKeys(privateKey, publicKey, aesSeed string) error {
config, err := rest.InClusterConfig()
if err != nil {
return errors.Wrap(err, "Error creating client config")
Expand Down
8 changes: 8 additions & 0 deletions app/safe/internal/server/route/receive/receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
package receive

import (
"github.com/vmware-tanzu/secrets-manager/app/safe/internal/bootstrap"
"github.com/vmware-tanzu/secrets-manager/core/env"
"io"
"net/http"
"strings"
Expand Down Expand Up @@ -85,6 +87,12 @@ func Keys(cid string, w http.ResponseWriter, r *http.Request, spiffeid string) {
keysCombined := agePrivateKey + "\n" + agePublicKey + "\n" + aesCipherKey
state.SetRootKey(keysCombined)

if env.RootKeyInputModeManual() && env.ManualRootKeyUpdatesKubernetesSecret() {
if err := bootstrap.PersistKeys(agePrivateKey, agePublicKey, aesCipherKey); err != nil {
log.ErrorLn(&cid, "Keys: Problem persisting keys", err.Error())
}
}
Comment on lines +90 to +94
Copy link
Contributor

Choose a reason for hiding this comment

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

We don’t need the checks in the first if.

The user can change the key (at their own risk) if they want to.

"manual mode" is only for the bootstrapping flow.

Can we replace it with this, please?

	if env.ManualRootKeyUpdatesKubernetesSecret() {
 		if err := bootstrap.PersistKeys(agePrivateKey, agePublicKey, aesCipherKey); err != nil {
 			log.ErrorLn(&cid, "Keys: Problem persisting keys", err.Error())
 		}
 	}


log.DebugLn(&cid, "Keys: before response")

_, err := io.WriteString(w, "OK")
Expand Down
16 changes: 16 additions & 0 deletions core/env/safe.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,22 @@ func RootKeyInputModeManual() bool {
return false
}

// ManualRootKeyUpdatesK8sSecret returns a boolean indicating whether to
// update the Kubernetes secret when the root key is provided manually to VSecM Safe.
// If the environment variable is not set or its value is not "true", the function
// returns false. Otherwise, the function returns true.
func ManualRootKeyUpdatesK8sSecret() bool {
p := os.Getenv("VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET")
if p == "" {
return false
}
if strings.ToLower(p) == "true" {
return true
}
return false

}

// DataPathForSafe returns the path to the safe data directory.
// The path is determined by the VSECM_SAFE_DATA_PATH environment variable.
// If the environment variable is not set, the default path "/data" is returned.
Expand Down
63 changes: 63 additions & 0 deletions core/env/safe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,69 @@ func TestRootKeyInputMode(t *testing.T) {
}
}

func TestManualRootKeyUpdatesK8sSecrets(t *testing.T) {
tests := []struct {
name string
setup func() error
cleanup func() error
want bool
}{
{
name: "default_safe_manual_root_key_updates_k8s_secret",
want: false,
},
{
name: "safe_manual_root_key_updates_k8s_secret_from_env_true",
setup: func() error {
return os.Setenv("VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET", "true")
},
cleanup: func() error {
return os.Unsetenv("VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET")
},
want: true,
},
{
name: "safe_manual_root_key_updates_k8s_secret_from_env_false",
setup: func() error {
return os.Setenv("VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET", "false")
},
cleanup: func() error {
return os.Unsetenv("VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET")
},
want: false,
},
{
name: "invalid_safe_manual_root_key_updates_k8s_secret_from_env",
setup: func() error {
return os.Setenv("VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET", "test")
},
cleanup: func() error {
return os.Unsetenv("VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET")
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.setup != nil {
if err := tt.setup(); err != nil {
t.Errorf("ManualRootKeyUpdatesK8sSecret() = failed to setup, with error: %+v", err)
}
}
defer func() {
if tt.cleanup != nil {
if err := tt.cleanup(); err != nil {
t.Errorf("ManualRootKeyUpdatesK8sSecret() = failed to cleanup, with error: %+v", err)
}
}
}()
if got := ManualRootKeyUpdatesK8sSecret(); got != tt.want {
t.Errorf("ManualRootKeyUpdatesK8sSecret() = %v, want %v", got, tt.want)
}
})
}
}

func TestSafeDataPath(t *testing.T) {
tests := []struct {
name string
Expand Down
28 changes: 15 additions & 13 deletions docs/_pages/0110-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,22 +337,24 @@ Using a Kubernetes `Secret` to store the *root key* is still secure,
especially if you encrypt your `etcd` and establish a **tight RBAC** over the
Kubernetes `Secret`that stores the *root key*.

> **Root Key Storage in Manual Input Mode**
>
> As of *v0.21.5* of **VMware Secrets Manager**, the *root key* is not stored
> in a Kubernetes `Secret` if you use the manual key input approach. This
> behavior is subject to change in the future, depending on a configuration
> environment variable, the *root key* might be stored in a Kubernetes `Secret`
> even if you use the manual key input approach.
>
> However, this means that if you use the manual key input approach, you will
> have to re-enter the *root key* every time you restart **VSecM Safe** or
> every time the pod is evicted by the scheduler.
{: .block-warning}

Also note that when this variable is set to `"true"`, **VSecM Safe** will **not**
respond to API requests until a *root key* is provided, using **VSecM Sentinel**.

### VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET

**Used By**: *VSecM Safe*.

`VSECM_MANUAL_ROOT_KEY_UPDATES_K8S_SECRET` is a boolean indicating whether to update
the Kubernetes secret when the root key is provided manually to **VSecM Safe**.

If the environment variable is not set or its value is not `"true"`, **VSecM Safe**
will **not** update the Kubernetes secret when the root key is provided manually.

If this variable is set to `"true"` and **VSecM Safe** is in manual root key input
mode (see: [VSECM_ROOT_KEY_INPUT_MODE_MANUAL](#vsecm_root_key_input_mode_manual)),
then **VSecM Safe** will update the Kubernetes secret with the new root key
when the root key is provided manually.

### VSECM_SAFE_SECRET_BACKUP_COUNT

**Used By**: *VSecM Safe*.
Expand Down