Skip to content

Commit

Permalink
Add new logic for looking up Strongbox Secrets
Browse files Browse the repository at this point in the history
We are 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.
  • Loading branch information
george-angel committed Nov 18, 2024
1 parent a69db60 commit 7f49292
Show file tree
Hide file tree
Showing 10 changed files with 191 additions and 66 deletions.
5 changes: 2 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down Expand Up @@ -238,7 +239,6 @@ subjects:
- kind: ServiceAccount
name: argocd-repo-server
namespace: sys-argocd
```

### Plugin Configuration
Expand All @@ -258,7 +258,6 @@ 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_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 |
9 changes: 6 additions & 3 deletions decrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,18 @@ var (
func ensureDecryption(ctx context.Context, cwd string, app applicationInfo) error {
keyringData, identityData, err := secretData(ctx, app.destinationNamespace, app.keyringSecret)
if err != nil {
if errors.Is(err, errNotFound) {
return nil
}
return err
}
if keyringData == nil && identityData == nil {
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
}
Expand All @@ -61,7 +64,7 @@ func ensureDecryption(ctx context.Context, cwd string, app applicationInfo) erro
}

func secretData(ctx context.Context, destinationNamespace string, si secretInfo) ([]byte, []byte, error) {
secret, err := getSecret(ctx, destinationNamespace, si)
secret, err := secret(ctx, destinationNamespace, si)
if err != nil {
return nil, nil, err
}
Expand Down
83 changes: 61 additions & 22 deletions generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}

Expand All @@ -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{}

Expand Down Expand Up @@ -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
Expand All @@ -109,10 +118,9 @@ 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

Expand All @@ -123,31 +131,62 @@ func runKustomizeBuild(ctx context.Context, cwd string, env []string) (string, e
k.Stderr = &stderr

if err := k.Start(); err != nil {
return "", fmt.Errorf("unable to start kustomize cmd err:%s", err)
return nil, fmt.Errorf("unable to start kustomize cmd: err=%s", err)
}

if err := k.Wait(); err != nil {
return "", fmt.Errorf("error running kustomize err:%s", strings.TrimSpace(stderr.String()))
return nil, fmt.Errorf("error running kustomize: err=%s", stderr.String())
}

if err := checkSecrets(stdout.Bytes()); err != nil {
return nil, err
}

return stdout.String(), nil
return stdout.Bytes(), 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 {
// Unmarshaling will fail for Secret like object, like ConfigMap
continue
}

// 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: secret=%s key=%s", secret.Name, key)
}
}
}
}
return nil
}
88 changes: 87 additions & 1 deletion generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package main

import "testing"

func Test_hasSSHRemoteBaseURL(t *testing.T) {
func TestHasSSHRemoteBaseURL(t *testing.T) {
type args struct {
cwd string
}
Expand Down Expand Up @@ -33,3 +33,89 @@ 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: ServiceAccount
metadata:
name: test
---
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: ConfigMap
metadata:
name: test
data:
key: value
key1: value1
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: test
---
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: ServiceAccount
metadata:
name: test
---
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")
}
})
}
4 changes: 2 additions & 2 deletions git-ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -49,7 +49,7 @@ func setupGitSSH(ctx context.Context, cwd, globalKeyPath, globalKnownHostFile st

// Using own SSH key
if app.gitSSHSecret.name != "" {
sec, err := getSecret(ctx, app.destinationNamespace, app.gitSSHSecret)
sec, err := secret(ctx, app.destinationNamespace, app.gitSSHSecret)
if err != nil {
return "", err
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
Loading

0 comments on commit 7f49292

Please sign in to comment.