From bc9fb2e21b2d820dc97f83acb3503efbadb54d10 Mon Sep 17 00:00:00 2001 From: George Angel Date: Tue, 12 Nov 2024 21:30:56 +1000 Subject: [PATCH] Add new logic for looking up Strongbox Secrets We now try to deduce if we need to lookup Strongbox keyring/identity Secret. We do this by checking for `filter=strongbox` in `.gitattributes` in a given Namespace. This should cover majority of cases and allow use default settings. For those who **only** have Strongbox files in remote bases that are loaded via Kustomize, need to enable Strongbox functionality via `STRONGBOX_FORCE="true"`. We are also adding a safeguard, we now check `kustomize build` output and check Secret data values for Strongbox headers. Plugin will fail if it finds a Strongbox header in Secret data. --- README.md | 12 +++++-- decrypt.go | 54 +++++++++++++++++++++++++++-- decrypt_test.go | 73 +++++++++++++++++++++++++++++++++++++-- generate.go | 90 ++++++++++++++++++++++++++++++++---------------- generate_test.go | 65 +++++++++++++++++++++++++++++++++- git-ssh.go | 2 +- go.mod | 1 + go.sum | 2 ++ main.go | 37 +++++++++----------- 9 files changed, 276 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index d0420ac..8f005c5 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,8 @@ resources: ### Strongbox envvars -Secret name containing Strongbox keyring/identity file MUST be `argocd-voodoobox-strongbox-keyring`. +Secret name containing Strongbox keyring/identity file MUST be +`argocd-voodoobox-strongbox-keyring`. Key name for keyring MUST be `.strongbox_keyring` @@ -52,6 +53,11 @@ the Secret should have an annotation called "argocd.voodoobox.plugin.io/allowed- Since ArgoCD Application can be used to create a namespace, wild card is not supported in the allow list. It is an exact match. If this env is not specified then it defaults to the same namespace as the app's destination NS. +`STRONGBOX_FORCE` Plugin will try to determine if it needs to setup Strongbox +or not based on presence of `filter=strongbox` in `.gitattributes`. If you ONLY +have Strongbox files in remote Kustomize base - you need to tell plugin to +setup Strongbox explicitly by setting this to "true". + ```yaml # secret example the following secret can be used by namespaces "ns-a", "ns-b" and "ns-c": @@ -91,6 +97,8 @@ spec: env: - name: STRONGBOX_SECRET_NAMESPACE value: team-a + - name: STRONGBOX_FORCE + value: "true" ``` ### Git SSH Keys envvars @@ -258,7 +266,7 @@ subjects: |-|-|-| | ARGOCD_APP_NAME | set by argocd | name of application | | ARGOCD_APP_NAMESPACE | set by argocd | application's destination namespace | -| STRONGBOX_ENABLED | "true" | Enable Strongbox for decryption | +| STRONGBOX_FORCE | "false" | If your Namespace **only** has Strongbox Secrets in remote Kustomize bases - you need to force Strongbox functionality | | STRONGBOX_SECRET_NAMESPACE | | the name of a namespace where secret resource containing strongbox keyring is located, defaults to current | | GIT_SSH_CUSTOM_KEY_ENABLED | "false" | Enable Git SSH building using custom (non global) key | | GIT_SSH_SECRET_NAMESPACE | | the value should be the name of a namespace where secret resource containing ssh keys are located, defaults to current | diff --git a/decrypt.go b/decrypt.go index 1e42296..a17c12f 100644 --- a/decrypt.go +++ b/decrypt.go @@ -1,6 +1,7 @@ package main import ( + "bufio" "bytes" "context" "errors" @@ -26,7 +27,17 @@ var ( errEncryptedFileFound = errors.New("encrypted file found") ) -func ensureDecryption(ctx context.Context, cwd string, app applicationInfo) error { +func ensureDecryption(force bool, ctx context.Context, cwd string, app applicationInfo) error { + hsf, err := hasStrongboxFilter(ctx, cwd) + if err != nil { + return err + } + + // If we are not forcing and we can't find a filter - return + if !force && !hsf { + return nil + } + keyringData, identityData, err := secretData(ctx, app.destinationNamespace, app.keyringSecret) if err != nil { return err @@ -35,9 +46,9 @@ func ensureDecryption(ctx context.Context, cwd string, app applicationInfo) erro return nil } - // create strongbox keyRing file + // create Strongbox keyRing file if keyringData != nil { - keyRingPath := filepath.Join(cwd, strongboxKeyRingFile) + keyRingPath := filepath.Join(cwd, strongboxKeyringFilename) if err := os.WriteFile(keyRingPath, keyringData, 0644); err != nil { return err } @@ -129,3 +140,40 @@ func strongboxAgeRecursiveDecrypt(ctx context.Context, cwd string, identityData return file.Truncate(n) }) } + +func hasStrongboxFilter(ctx context.Context, dir string) (bool, error) { + var found bool + err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() || filepath.Base(path) != ".gitattributes" { + return nil + } + + file, err := os.Open(path) + if err != nil { + return err + } + defer file.Close() + + scanner := bufio.NewScanner(file) + for scanner.Scan() { + select { + case <-ctx.Done(): + return ctx.Err() + default: + if strings.Contains(scanner.Text(), "filter=strongbox") { + found = true + return filepath.SkipDir // Stop search at current directory level + } + } + } + return scanner.Err() + }) + + if err != nil && err != filepath.SkipDir { + return false, err + } + return found, nil +} diff --git a/decrypt_test.go b/decrypt_test.go index b4b84b7..0e82b05 100644 --- a/decrypt_test.go +++ b/decrypt_test.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "reflect" "testing" @@ -184,7 +185,7 @@ func Test_ensureDecryption(t *testing.T) { }, } t.Run("no-encrypted-files-with-secret", func(t *testing.T) { - err = ensureDecryption(context.Background(), withRemoteBaseTestDir, bar2) + err = ensureDecryption(true, context.Background(), withRemoteBaseTestDir, bar2) if err != nil { t.Fatal(err) } @@ -204,7 +205,7 @@ func Test_ensureDecryption(t *testing.T) { }, } t.Run("encrypted-files-with-secret", func(t *testing.T) { - err = ensureDecryption(context.Background(), encryptedTestDir1, foo) + err = ensureDecryption(true, context.Background(), encryptedTestDir1, foo) if err != nil { t.Fatal(err) } @@ -238,7 +239,7 @@ func Test_ensureDecryption(t *testing.T) { }, } t.Run("encrypted-files-with-secret-from-diff-ns", func(t *testing.T) { - err = ensureDecryption(context.Background(), encryptedTestDir2, baz) + err = ensureDecryption(true, context.Background(), encryptedTestDir2, baz) if err != nil { t.Fatal(err) } @@ -262,3 +263,69 @@ func Test_ensureDecryption(t *testing.T) { }) } + +func TestHasStrongboxFilter(t *testing.T) { + ctx := context.Background() + + // Case 1: Pre-existing .gitattributes at the root with "filter=strongbox" + withStrongboxPath := filepath.Join("testData", "app-with-secrets") + strongboxFile := filepath.Join(withStrongboxPath, ".gitattributes") + + // Verify that the checked-in .gitattributes is detected correctly + found, err := hasStrongboxFilter(ctx, withStrongboxPath) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !found { + t.Errorf("expected to find 'filter=strongbox' in %s, but got false", withStrongboxPath) + } else { + t.Logf("Successfully found 'filter=strongbox' in %s", withStrongboxPath) + } + + // Temporarily rename the top-level .gitattributes to test the nested case + tempRename := strongboxFile + ".bak" + if err := os.Rename(strongboxFile, tempRename); err != nil { + t.Fatalf("failed to rename .gitattributes file: %v", err) + } + defer os.Rename(tempRename, strongboxFile) // Restore after test + + // Case 2: .gitattributes one level deeper with "filter=strongbox" + nestedPath := filepath.Join(withStrongboxPath, "app") + nestedStrongboxFile := filepath.Join(nestedPath, ".gitattributes") + + t.Logf("Creating nested .gitattributes with strongbox filter at: %s", nestedStrongboxFile) + if err := os.WriteFile(nestedStrongboxFile, []byte("*.json filter=strongbox\n"), 0644); err != nil { + t.Fatalf("failed to create nested .gitattributes file: %v", err) + } + defer os.Remove(nestedStrongboxFile) + + found, err = hasStrongboxFilter(ctx, withStrongboxPath) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !found { + t.Errorf("expected to find 'filter=strongbox' in nested %s, but got false", nestedPath) + } else { + t.Logf("Successfully found 'filter=strongbox' in nested %s", nestedPath) + } + + // Case 3: .gitattributes file without "filter=strongbox" + withoutStrongboxPath := filepath.Join("testData", "app-with-remote-base") + noStrongboxFile := filepath.Join(withoutStrongboxPath, ".gitattributes") + + t.Logf("Creating .gitattributes without strongbox filter at: %s", noStrongboxFile) + if err := os.WriteFile(noStrongboxFile, []byte("*.yaml filter=anotherFilter\n"), 0644); err != nil { + t.Fatalf("failed to create .gitattributes file: %v", err) + } + defer os.Remove(noStrongboxFile) + + found, err = hasStrongboxFilter(ctx, withoutStrongboxPath) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if found { + t.Errorf("expected not to find 'filter=strongbox' in %s, but got true", withoutStrongboxPath) + } else { + t.Logf("Correctly did not find 'filter=strongbox' in %s", withoutStrongboxPath) + } +} diff --git a/generate.go b/generate.go index 722aae0..3f482f2 100644 --- a/generate.go +++ b/generate.go @@ -9,18 +9,22 @@ import ( "os/exec" "path/filepath" "strings" + + "filippo.io/age/armor" + "github.com/ghodss/yaml" + v1 "k8s.io/api/core/v1" ) -func ensureBuild(ctx context.Context, cwd, globalKeyPath, globalKnownHostFile string, app applicationInfo) (string, error) { +func ensureBuild(ctx context.Context, cwd, globalKeyPath, globalKnownHostFile string, app applicationInfo) ([]byte, error) { // Even when there is no git SSH secret defined, we still override the - // git ssh command (pointing the key to /dev/null) in order to avoid - // using ssh keys in default system locations and to surface the error - // if bases over ssh have been configured. + // Git SSH command (pointing the key to /dev/null) in order to avoid + // using SSH keys in default system locations and to surface the error + // if bases over SSH have been configured. sshCmdEnv := `GIT_SSH_COMMAND=ssh -q -F none -o IdentitiesOnly=yes -o IdentityFile=/dev/null -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no` kFiles, err := findKustomizeFiles(cwd) if err != nil { - return "", fmt.Errorf("unable to ge kustomize files paths err:%s", err) + return nil, fmt.Errorf("unable to get Kustomize files paths err:%s", err) } if len(kFiles) == 0 { @@ -29,13 +33,13 @@ func ensureBuild(ctx context.Context, cwd, globalKeyPath, globalKnownHostFile st hasRemoteBase, err := hasSSHRemoteBaseURL(kFiles) if err != nil { - return "", fmt.Errorf("unable to look for ssh protocol err:%s", err) + return nil, fmt.Errorf("unable to look for SSH protocol err:%s", err) } if hasRemoteBase { sshCmdEnv, err = setupGitSSH(ctx, cwd, globalKeyPath, globalKnownHostFile, app) if err != nil { - return "", err + return nil, err } } @@ -50,20 +54,25 @@ func ensureBuild(ctx context.Context, cwd, globalKeyPath, globalKnownHostFile st env = append(env, sshCmdEnv) - // setup git config if .strongbox_keyring exits - if _, err = os.Stat(filepath.Join(cwd, strongboxKeyRingFile)); err == nil { + // setup Git config if .strongbox_keyring or .strongbox_identity exits + if fileExists(filepath.Join(cwd, strongboxKeyringFilename)) || fileExists(filepath.Join(cwd, strongboxIdentityFilename)) { // setup SB home for kustomize run env = append(env, fmt.Sprintf("STRONGBOX_HOME=%s", cwd)) - // getup git config via `strongbox -git-config` + // setup git config via `strongbox -git-config` if err := setupGitConfigForSB(ctx, cwd, env); err != nil { - return "", fmt.Errorf("unable setup git config for strongbox err:%s", err) + return nil, fmt.Errorf("unable setup git config for strongbox err:%s", err) } } return runKustomizeBuild(ctx, cwd, env) } +func fileExists(filepath string) bool { + _, err := os.Stat(filepath) + return err == nil +} + func findKustomizeFiles(cwd string) ([]string, error) { kFiles := []string{} @@ -95,7 +104,7 @@ func hasSSHRemoteBaseURL(kFiles []string) (bool, error) { return false, nil } -// setupGitConfigForSB will setup git filters to run strongbox +// setupGitConfigForSB will setup git filters to run Strongbox func setupGitConfigForSB(ctx context.Context, cwd string, env []string) error { s := exec.CommandContext(ctx, "strongbox", "-git-config") s.Dir = cwd @@ -109,45 +118,66 @@ func setupGitConfigForSB(ctx context.Context, cwd string, env []string) error { return nil } -// runKustomizeBuild will run `kustomize build` cmd and return generated yaml or error -func runKustomizeBuild(ctx context.Context, cwd string, env []string) (string, error) { +// runKustomizeBuild runs `kustomize build` and returns the generated YAML or an error. +func runKustomizeBuild(ctx context.Context, cwd string, env []string) ([]byte, error) { k := exec.CommandContext(ctx, "kustomize", "build", ".") - k.Dir = cwd k.Env = env - var stdout bytes.Buffer - var stderr bytes.Buffer - - k.Stdout = &stdout - k.Stderr = &stderr - - if err := k.Start(); err != nil { - return "", fmt.Errorf("unable to start kustomize cmd err:%s", err) + output, err := k.CombinedOutput() + if err != nil { + return nil, fmt.Errorf("error running kustomize: %s", strings.TrimSpace(string(output))) } - if err := k.Wait(); err != nil { - return "", fmt.Errorf("error running kustomize err:%s", strings.TrimSpace(stderr.String())) + checkSecrets(output) + if err != nil { + return nil, err } - return stdout.String(), nil + return output, nil } -func findAndReadYamlFiles(cwd string) (string, error) { - var content string +func findAndReadYamlFiles(cwd string) ([]byte, error) { + var content []byte err := filepath.WalkDir(cwd, func(path string, info fs.DirEntry, err error) error { if filepath.Ext(path) == ".yaml" || filepath.Base(path) == ".yml" { data, err := os.ReadFile(path) if err != nil { return fmt.Errorf("unable to read file %s err:%s", path, err) } - content += fmt.Sprintf("%s\n---\n", data) + content = append(content, []byte(fmt.Sprintf("%s\n---\n", data))...) } return nil }) if err != nil { - return "", err + return nil, err } return content, nil } + +func checkSecrets(yamlData []byte) error { + // Split input YAML into multiple documents by "---" + docs := bytes.Split(yamlData, []byte("\n---\n")) + + for _, doc := range docs { + if len(bytes.TrimSpace(doc)) == 0 { + continue // Skip empty documents + } + + var secret v1.Secret + if err := yaml.Unmarshal(doc, &secret); err != nil { + return fmt.Errorf("failed to decode YAML document: %v", err) + } + + // Check if the decoded document is a Secret + if secret.Kind == "Secret" { + for key, val := range secret.Data { + if bytes.HasPrefix(val, encryptedFilePrefix) || strings.HasPrefix(string(val), armor.Header) { + return fmt.Errorf("found ciphertext in Secret %s, key %s", secret.Name, key) + } + } + } + } + return nil +} diff --git a/generate_test.go b/generate_test.go index 2d67aa6..25bbef7 100644 --- a/generate_test.go +++ b/generate_test.go @@ -2,7 +2,7 @@ package main import "testing" -func Test_hasSSHRemoteBaseURL(t *testing.T) { +func TestHasSSHRemoteBaseURL(t *testing.T) { type args struct { cwd string } @@ -33,3 +33,66 @@ func Test_hasSSHRemoteBaseURL(t *testing.T) { }) } } + +func TestCiphertextSecrets(t *testing.T) { + t.Run("Error on STRONGBOX header in Secret data", func(t *testing.T) { + yamlData := ` +apiVersion: v1 +kind: Secret +metadata: + name: my-secret +data: + key1: | + c2VjcmV0ZGF0YQ== + key2: | + IyBTVFJPTkdCT1ggRU5DUllQVEVEIFJFU09VUkNFIDsgU2VlIGh0dHBzOi8vZ2l0aHViLmNvbS91 + dy1sYWJzL3N0cm9uZ2JveApiZlY0ZWZnVjNwTVVJUmRwV0VzbDFCdnJTNUo0QXZHcnd1eWNpZ0Y4 + eXZtUWVGUGNMNktFZGxRbjROOEtzVDhWNHJiUm45TVlIWXFUCmtoQ1d2bEMxWjh2QXJGcVhRdkhz + UGF4M2lRPT0K +` + err := checkSecrets([]byte(yamlData)) + if err == nil { + t.Error("Expected error due to STRONGBOX header, but got nil") + } + }) + + t.Run("Success with no STRONGBOX header in Secret data", func(t *testing.T) { + yamlData := ` +apiVersion: v1 +kind: Secret +metadata: + name: my-secret +data: + key1: | + c2VjcmV0ZGF0YQ== +` + err := checkSecrets([]byte(yamlData)) + if err != nil { + t.Errorf("Expected success, but got error: %v", err) + } + }) + + t.Run("Error on AGE header in Secret data", func(t *testing.T) { + yamlData := ` +apiVersion: v1 +kind: Secret +metadata: + name: my-secret +data: + key1: | + c2VjcmV0ZGF0YQ== + key2: | + LS0tLS1CRUdJTiBBR0UgRU5DUllQVEVEIEZJTEUtLS0tLQpZV2RsTFdWdVkzSjVjSFJwYjI0dWIz + Sm5MM1l4Q2kwK0lGZ3lOVFV4T1NCMVNHbHdXRkJMT0VwbVpHOWlUM1JTClMzQk5aREZPYm5Ndllr + c3pkMVpwTUVsTldXSXpXRVEyWmtRMENqUnJPRTVuUVV3dlVrNWpZWFZTV1RaalNUVXoKUzBOdWRu + RXpWWE5WVFhBeVpFcHZaMjl2V0ZwSVN6Z0tMUzB0SUROWVIweFpkM2ROVG5admF6QkRjM2RJWm1G + SQpZMDQ1Ukc1WVlsWnJUMmREWVdZek1GRTVhVk5RYVRRS05DYmE3QzU1S01FWFp2MjU4bFU2WjFD + M1c4UUF0WklGClJxZXFQSXZKYTljRTU0YUFDQT09Ci0tLS0tRU5EIEFHRSBFTkNSWVBURUQgRklM + RS0tLS0tCg== +` + err := checkSecrets([]byte(yamlData)) + if err == nil { + t.Error("Expected error due to AGE header, but got nil") + } + }) +} diff --git a/git-ssh.go b/git-ssh.go index 382628a..e175320 100644 --- a/git-ssh.go +++ b/git-ssh.go @@ -35,7 +35,7 @@ var ( func setupGitSSH(ctx context.Context, cwd, globalKeyPath, globalKnownHostFile string, app applicationInfo) (string, error) { knownHostsFragment := `-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no` - sshDir := filepath.Join(cwd, SSHDirName) + sshDir := filepath.Join(cwd, ".ssh") if err := os.Mkdir(sshDir, 0700); err != nil { return "", fmt.Errorf("unable to create ssh config dir err:%s", err) } diff --git a/go.mod b/go.mod index b4226e2..ebcc037 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ toolchain go1.22.2 require ( filippo.io/age v1.2.0 + github.com/ghodss/yaml v1.0.0 github.com/google/go-cmp v0.6.0 github.com/hashicorp/go-hclog v1.6.3 github.com/urfave/cli/v2 v2.27.5 diff --git a/go.sum b/go.sum index 74dbbf4..0230cf6 100644 --- a/go.sum +++ b/go.sum @@ -15,6 +15,8 @@ github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E= github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= +github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= +github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-openapi/jsonpointer v0.21.0 h1:YgdVicSA9vH5RiHs9TZW5oyafXZFc6+2Vc1rr/O9oNQ= diff --git a/main.go b/main.go index 048b406..9b040a1 100644 --- a/main.go +++ b/main.go @@ -14,9 +14,6 @@ import ( const ( // argocd adds `ARGOCD_ENV_` prefix to all plugin envs configured in Applications argocdAppEnvPrefix = "ARGOCD_ENV_" - - strongboxKeyRingFile = ".strongbox_keyring" - SSHDirName = ".ssh" ) var ( @@ -84,12 +81,13 @@ to get comma-separated list of all the namespaces that are allowed to use it`, }, &cli.BoolFlag{ - Name: "app-strongbox-enabled", - EnvVars: []string{argocdAppEnvPrefix + "STRONGBOX_ENABLED"}, - Usage: `set 'STRONGBOX_ENABLED' in argocd application as plugin - ENV. If set to "true" (default) will use default values to lookup the - Strongbox secret and use it.`, - Value: true, + Name: "app-force-strongbox", + EnvVars: []string{argocdAppEnvPrefix + "FORCE_STRONGBOX"}, + Usage: `set 'STRONGBOX_FORCE' in ArgoCD application as plugin + ENV. If set to "true" (default "false") will force the plugin + to try and look up "argocd-voodoobox-strongbox-keyring" Secret + in target Namespace.`, + Value: false, }, // following envs comes from argocd application resource // strongbox secrets flags @@ -167,14 +165,13 @@ func main() { } } - if c.Bool("app-strongbox-enabled") { - app.keyringSecret = secretInfo{ - name: c.String("app-strongbox-secret-name"), - namespace: c.String("app-strongbox-secret-namespace"), - } - if err := ensureDecryption(c.Context, cwd, app); err != nil { - return err - } + // Always try to decrypt + app.keyringSecret = secretInfo{ + name: c.String("app-strongbox-secret-name"), + namespace: c.String("app-strongbox-secret-namespace"), + } + if err := ensureDecryption(c.Bool("app-git-ssh-enabled"), c.Context, cwd, app); err != nil { + return err } manifests, err := ensureBuild(c.Context, cwd, globalKeyPath, globalKnownHostFile, app) @@ -185,10 +182,10 @@ func main() { // argocd creates a temp folder of plugin which gets deleted // once plugin is existed still clean up secrets manually // in case this behavior changes - os.Remove(filepath.Join(cwd, strongboxKeyRingFile)) - os.RemoveAll(filepath.Join(cwd, SSHDirName)) + os.Remove(filepath.Join(cwd, strongboxKeyringFilename)) + os.RemoveAll(filepath.Join(cwd, ".ssh")) - fmt.Printf("%s\n---\n", manifests) + fmt.Printf("%s", manifests) return nil }, },