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: Use decryptSecrets from Kubernetes #242

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 3 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,10 @@ setup-e2e: kustomize ## Setup test environment in the K8s cluster specified in ~
run-operator:
# Clean start
killall goreman || true
mkdir -p /tmp/paas-e2e/secrets/priv && chmod 0700 /tmp/paas-e2e/secrets/priv
mkdir -p /tmp/paas-e2e/secrets/pub && chmod 0700 /tmp/paas-e2e/secrets/pub
cp -r ./test/e2e/fixtures/crypt/priv* /tmp/paas-e2e/secrets/priv
cp -r ./test/e2e/fixtures/crypt/pub/* /tmp/paas-e2e/secrets/pub
kubectl create namespace paas-system
kubectl apply -f manifests/config/example-keys.yaml
goreman -f $(PAAS_PROCFILE) start
rm -rf /tmp/paas-e2e
kubectl delete namespace paas-system

##@ Build

Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha1/paasconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ type PaasConfigSpec struct {
// +kubebuilder:validation:Required
DecryptKeyPaths []string `json:"decryptKeyPaths"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This one can go, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats a breaking change. We are allowed to do that in an alpha api. If we agree, let's do that. Else, we need to support both which makes this feature more complex. Let me know what you think is appropriate.


// DecryptKeysSecretName is a reference to the secret containing the DecryptKeys
// +kubebuilder:validation:Required
DecryptKeysSecret NamespacedName `json:"decryptKeySecret"`

// Enable debug information generation or not
// +kubebuilder:default:=false
// +kubebuilder:validation:Optional
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cmd/crypttool/check_paas.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func checkPaasFiles(privateKeyFiles string, files []string) error {
}

paasName := paas.ObjectMeta.Name
srcCrypt, err := crypt.NewCrypt([]string{privateKeyFiles}, "", paasName)
srcCrypt, err := crypt.NewCryptFromFiles([]string{privateKeyFiles}, "", paasName)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/crypttool/reencrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ func reencryptFiles(privateKeyFiles string, publicKeyFile string, outputFormat s
}

paasName := paas.ObjectMeta.Name
srcCrypt, err := crypt.NewCrypt([]string{privateKeyFiles}, "", paasName)
srcCrypt, err := crypt.NewCryptFromFiles([]string{privateKeyFiles}, "", paasName)
if err != nil {
return err
}

dstCrypt, err := crypt.NewCrypt([]string{}, publicKeyFile, paasName)
dstCrypt, err := crypt.NewCryptFromFiles([]string{}, publicKeyFile, paasName)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/webservice/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func getRsa(paas string) *crypt.Crypt {
return c
}

c, err := crypt.NewCrypt([]string{config.PrivateKeyPath}, config.PublicKeyPath, paas)
c, err := crypt.NewCryptFromFiles([]string{config.PrivateKeyPath}, config.PublicKeyPath, paas)
if err != nil {
panic(fmt.Errorf("unable to create a crypt: %w", err))
}
Expand Down
72 changes: 53 additions & 19 deletions internal/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ package controller

import (
"context"
"crypto/rsa"
"crypto/x509"
"encoding/pem"
"fmt"
"sync"

v1 "k8s.io/api/core/v1"

"github.com/belastingdienst/opr-paas/api/v1alpha1"
"github.com/belastingdienst/opr-paas/internal/crypt"

Expand All @@ -30,37 +35,66 @@ type PaasConfigStore struct {
}

var (
_cnf = &PaasConfigStore{}
_crypt map[string]*crypt.Crypt
debugComponents []string
cnf = &PaasConfigStore{}
// crypts contains a maps of crypt against a Paas name
crypts map[string]*crypt.Crypt
currentDecryptSecretGeneration int64
decryptSecretPrivateKeys []rsa.PrivateKey
debugComponents []string
)

// GetConfig retrieves the current configuration
func GetConfig() v1alpha1.PaasConfigSpec {
_cnf.mutex.RLock()
defer _cnf.mutex.RUnlock()
return _cnf.currentConfig
cnf.mutex.RLock()
defer cnf.mutex.RUnlock()
return cnf.currentConfig
}

// SetConfig updates the current configuration
func SetConfig(newConfig v1alpha1.PaasConfig) {
_cnf.mutex.Lock()
defer _cnf.mutex.Unlock()
_cnf.currentConfig = newConfig.Spec
cnf.mutex.Lock()
defer cnf.mutex.Unlock()
cnf.currentConfig = newConfig.Spec
}

// resetCrypts removes all crypts, decryptSecretPrivateKeys and resets the currentDecryptSecretGeneration
func resetCrypts() {
crypts = nil
decryptSecretPrivateKeys = nil
currentDecryptSecretGeneration = 0
}

func getRsa(paas string) *crypt.Crypt {
config := GetConfig()
if _crypt == nil {
_crypt = make(map[string]*crypt.Crypt)
// getRsa returns a crypt.Crypt for a specified paasName
func getRsa(paasName string, secret v1.Secret) (*crypt.Crypt, error) {
if crypts == nil {
crypts = make(map[string]*crypt.Crypt)
}
if c, exists := _crypt[paas]; exists {
return c
} else if c, err := crypt.NewCrypt(config.DecryptKeyPaths, "", paas); err != nil {
panic(fmt.Errorf("could not get a crypt: %w", err))
// Load secrets
// If one error occurs, all is invalid
if decryptSecretPrivateKeys == nil {
tmpKeys := make([]rsa.PrivateKey, 0)
for _, value := range secret.Data {
if privateKeyBlock, _ := pem.Decode(secret.Data[string(value)]); privateKeyBlock == nil {
return nil, fmt.Errorf("cannot decode private key")
} else if privateKey, err := x509.ParsePKCS1PrivateKey(privateKeyBlock.Bytes); err != nil {
return nil, fmt.Errorf("private key invalid: %w", err)
} else {
tmpKeys = append(tmpKeys, *privateKey)
}
}
decryptSecretPrivateKeys = tmpKeys
currentDecryptSecretGeneration = secret.Generation
}

if c, exists := crypts[paasName]; exists {
return c, nil
} else {
_crypt[paas] = c
return c
if c, err := crypt.NewCryptFromKeys(decryptSecretPrivateKeys, "", paasName); err != nil {
return nil, err
} else {
crypts[paasName] = c
return c, nil
}
}
}

Expand Down
6 changes: 5 additions & 1 deletion internal/controller/paasconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (pcr *PaasConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request
logger.Err(err).Msg("failed to update PaasConfig status")
return errResult, nil
}
// don't reconcile this one again as that won't change anything.. I guess.
// don't reconcile this one again as that won't change anything.
return ctrl.Result{}, nil
}
}
Expand All @@ -169,6 +169,10 @@ func (pcr *PaasConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request
}

logger.Info().Msg("configuration has changed")
// TODO(portly-halicore-76) is this the correct place and time to change?
if !reflect.DeepEqual(config.Spec.DecryptKeysSecret, GetConfig().DecryptKeysSecret) {
resetCrypts()
}
// Update the shared configuration store
SetConfig(*config)
logger.Debug().Msg("set active PaasConfig successfully")
Expand Down
30 changes: 28 additions & 2 deletions internal/controller/secret_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"maps"
"strings"

"github.com/belastingdienst/opr-paas/internal/crypt"

"github.com/belastingdienst/opr-paas/api/v1alpha1"

"github.com/rs/zerolog/log"
Expand Down Expand Up @@ -115,12 +117,34 @@ func (r *PaasNSReconciler) backendSecret(
return s, nil
}

// getSecrets returns a list of Secrets which are desired based on the Paas(Ns) spec
func (r *PaasNSReconciler) getSecrets(
ctx context.Context,
paas *v1alpha1.Paas,
paasns *v1alpha1.PaasNS,
encryptedSecrets map[string]string,
) (secrets []*corev1.Secret, err error) {
// Only do something when secrets are required
if len(encryptedSecrets) > 0 {
return nil, nil
}
// Get the configured decryptSecret
decryptSecret := &corev1.Secret{}
err = r.Get(ctx, types.NamespacedName{Name: GetConfig().DecryptKeysSecret.Name, Namespace: GetConfig().DecryptKeysSecret.Namespace}, decryptSecret)
if err != nil {
return nil, fmt.Errorf("Unable to get decryptSecret from kubernetes, contact system administrator: %w", err)
}
// If the generation is changed, the secret has changed, reset Crypts.
if decryptSecret.Generation != currentDecryptSecretGeneration {
resetCrypts()
}

var rsa *crypt.Crypt
rsa, err = getRsa(paas.Name, *decryptSecret)
if err != nil {
return nil, err
}

for url, encryptedSecretData := range encryptedSecrets {
namespacedName := types.NamespacedName{
Namespace: paasns.NamespaceName(),
Expand All @@ -130,16 +154,18 @@ func (r *PaasNSReconciler) getSecrets(
if err != nil {
return nil, err
}
if decrypted, err := getRsa(paasns.Spec.Paas).Decrypt(encryptedSecretData); err != nil {
if decrypted, err := rsa.Decrypt(encryptedSecretData); err != nil {
return nil, fmt.Errorf("failed to decrypt secret %s: %s", secret, err.Error())
} else {
secret.Data["sshPrivateKey"] = decrypted
secrets = append(secrets, secret)
}
}
return
return secrets, nil
}

// BackendSecrets returns a list of kubernetes Secrets which are desired based on the Paas(Ns) spec.
// It returns an error when the secrets cannot be determined.
func (r *PaasNSReconciler) BackendSecrets(
ctx context.Context,
paasns *v1alpha1.PaasNS,
Expand Down
49 changes: 44 additions & 5 deletions internal/crypt/crypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ type cryptPrivateKey struct {
privateKey *rsa.PrivateKey
}

func NewPrivateKey(privateKeyPath string) (*cryptPrivateKey, error) {
type cryptPrivateKeys []cryptPrivateKey

// NewPrivateKeyFromFile returns a cryptPrivateKey from a privateKeyFilePath
func NewPrivateKeyFromFile(privateKeyPath string) (*cryptPrivateKey, error) {
if privateKeyPath == "" {
return nil, fmt.Errorf("cannot get private key without a specified path")
}
Expand All @@ -52,6 +55,15 @@ func NewPrivateKey(privateKeyPath string) (*cryptPrivateKey, error) {
}
}

// NewPrivateKeyFromRsa returns a cryptPrivateKey from a rsa.PrivateKey
func NewPrivateKeyFromRsa(privateKey rsa.PrivateKey) *cryptPrivateKey {
return &cryptPrivateKey{
"",
nil,
&privateKey,
}
}

func (pk *cryptPrivateKey) writePrivateKey() error {
if pk.privateKeyPath == "" {
return fmt.Errorf("cannot write private key without a specified path")
Expand All @@ -69,17 +81,23 @@ func (pk *cryptPrivateKey) writePrivateKey() error {
return nil
}

// getPrivateKey returns the rsa.PrivateKey from the provided cryptPrivateKey. If it is not set yet, it will
// try to load it from the specified filePath. It also checks whether it is a valid PrivateKey.
func (pk cryptPrivateKey) getPrivateKey() (*rsa.PrivateKey, error) {
// if privateKey is already loaded, return it from the cryptPrivateKey
if pk.privateKey != nil {
return pk.privateKey, nil
}
// if it is not loaded on the pk, we must load it, in this case from a specific path which hopefully is set
if pk.privateKeyPath == "" {
return nil, fmt.Errorf("cannot get private key without a specified path")
}
// if set, read file from set path and try to decode
if privateKeyPEM, err := os.ReadFile(pk.privateKeyPath); err != nil {
panic(err)
} else if privateKeyBlock, _ := pem.Decode(privateKeyPEM); privateKeyBlock == nil {
return nil, fmt.Errorf("cannot decode private key")
// sanity check if the privatekey is a valid one
} else if privateRsaKey, err := x509.ParsePKCS1PrivateKey(privateKeyBlock.Bytes); err != nil {
return nil, fmt.Errorf("private key invalid: %w", err)
} else {
Expand All @@ -88,16 +106,15 @@ func (pk cryptPrivateKey) getPrivateKey() (*rsa.PrivateKey, error) {
return pk.privateKey, nil
}

type cryptPrivateKeys []cryptPrivateKey

func NewCrypt(privateKeyPaths []string, publicKeyPath string, encryptionContext string) (*Crypt, error) {
// NewCryptFromFiles returns a Crypt based on the provided privateKeyPaths and publicKeyPath using the encryptionContext
func NewCryptFromFiles(privateKeyPaths []string, publicKeyPath string, encryptionContext string) (*Crypt, error) {
var privateKeys cryptPrivateKeys

if files, err := utils.PathToFileList(privateKeyPaths); err != nil {
return nil, fmt.Errorf("could not find files in '%v': %w", privateKeyPaths, err)
} else {
for _, file := range files {
if pk, err := NewPrivateKey(file); err != nil {
if pk, err := NewPrivateKeyFromFile(file); err != nil {
return nil, fmt.Errorf("invalid private key file %s", file)
} else {
privateKeys = append(privateKeys, *pk)
Expand All @@ -119,6 +136,28 @@ func NewCrypt(privateKeyPaths []string, publicKeyPath string, encryptionContext
}, nil
}

// NewCryptFromKeys returns a Crypt based on the provided privateKeys and publicKey using the encryptionContext
func NewCryptFromKeys(privateKeys []rsa.PrivateKey, publicKeyPath string, encryptionContext string) (*Crypt, error) {
var cryptPrivateKeys cryptPrivateKeys

for _, key := range privateKeys {
cryptPrivateKeys = append(cryptPrivateKeys, *NewPrivateKeyFromRsa(key))
}

if publicKeyPath != "" {
publicKeyPaths := []string{publicKeyPath}
if _, err := utils.PathToFileList(publicKeyPaths); err != nil {
return nil, fmt.Errorf("could not find files in '%v': %w", publicKeyPaths, err)
}
}

return &Crypt{
privateKeys: cryptPrivateKeys,
publicKeyPath: publicKeyPath,
encryptionContext: []byte(encryptionContext),
}, nil
}

func NewGeneratedCrypt(privateKeyPath string, publicKeyPath string) (*Crypt, error) {
var c Crypt
if privateKey, err := rsa.GenerateKey(rand.Reader, 4096); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/crypt/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

func encrypt(publicKey string, paasName string, data []byte) error {
if c, err := NewCrypt([]string{}, publicKey, paasName); err != nil {
if c, err := NewCryptFromFiles([]string{}, publicKey, paasName); err != nil {
return err
} else if encrypted, err := c.Encrypt(data); err != nil {
return fmt.Errorf("failed to encrypt: %w", err)
Expand All @@ -27,7 +27,7 @@ func DecryptFromStdin(privateKeys []string, paasName string) error {
if data, err := io.ReadAll(os.Stdin); err != nil {
return err
} else {
if c, err := NewCrypt(privateKeys, "", paasName); err != nil {
if c, err := NewCryptFromFiles(privateKeys, "", paasName); err != nil {
return err
} else if encrypted, err := c.Decrypt(string(data)); err != nil {
return fmt.Errorf("failed to decrypt: %w", err)
Expand Down
13 changes: 13 additions & 0 deletions manifests/crds/cpet.belastingdienst.nl_paasconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,18 @@ spec:
type: string
minItems: 1
type: array
decryptKeySecret:
description: DecryptKeysSecretName is a reference to the secret containing
the DecryptKeys
properties:
name:
type: string
namespace:
type: string
required:
- name
- namespace
type: object
exclude_appset_name:
description: |-
Deprecated: ArgoCD specific code will be removed from the operator
Expand Down Expand Up @@ -233,6 +245,7 @@ spec:
required:
- clusterwide_argocd_namespace
- decryptKeyPaths
- decryptKeySecret
- exclude_appset_name
- groupsynclist
type: object
Expand Down
Loading
Loading