Skip to content

Commit

Permalink
Add new logic for looking up Strongbox Secrets (#112)
Browse files Browse the repository at this point in the history
We now reverted to the original logic to always look up the Strongbox
Secret and "fail open" (skip decrypting) when we do not find it.

We are adding a safeguard, we now check `kustomize build` output for
Strongbox headers in Secret data and will fail if found.
  • Loading branch information
george-angel authored Nov 19, 2024
1 parent a69db60 commit 6c2b8dd
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 6c2b8dd

Please sign in to comment.