Skip to content

Commit

Permalink
Force name and namespace from --merge-into target
Browse files Browse the repository at this point in the history
A frequent operation is to take an existing sealed secrets and update a single entry in it.

We even have a fancy command now that helps this tedious operation: `--merge-into` (documented in the README).

The rationale for implementing that command was not only to help lazy users to merge strings into the right place
into a YAML file but also to address one of the most common and confusing user errors:

```bash
$ echo -n foo | kubectl create secret generic goodname --dry-run  --from-file=bar=/dev/stdin -o json \
  | kubeseal >sealed.yml
$ echo -n bar | kubectl create secret generic baadname --dry-run  --from-file=bar=/dev/stdin -o json \
  | kubeseal --merge-into sealed.yml
```

It's even more subtle when the namespace is concerned, since it's often taken from the current kubeconfig context.

This PR turns `--merge-into` into what it was intended: a helper that helps, leaning more on the "do what I mean"
rather than blindly "do what I told you":

It treats the state of the existing sealed secret the user is merging new values into as the home for the sensible
parameters of the sealing process.

A subsequent PR will do something similar for the "scope", since that also affects how the entries are encrypted and has subtle consequences when you get it wrong (and it's harder to deal with in one-liners, since you cannot set an annotation during `kubectl create secret...`).

---

Enhances the tests for the mergeInto command so that they also decrypt the values.
This resulted in a noisy refactoring due to the fact that until now the kubeseal code only encrypted values
using a dummy public key for which we had no private key.
  • Loading branch information
Marko Mikulicic committed Oct 2, 2019
1 parent 40ddea6 commit 515b6cf
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 92 deletions.
48 changes: 2 additions & 46 deletions cmd/controller/keys.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
package main

import (
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"errors"
"io"
"math/big"
"time"

"github.com/bitnami-labs/sealed-secrets/pkg/crypto"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand All @@ -27,16 +23,7 @@ var (
)

func generatePrivateKeyAndCert(keySize int) (*rsa.PrivateKey, *x509.Certificate, error) {
r := rand.Reader
privKey, err := rsa.GenerateKey(r, keySize)
if err != nil {
return nil, nil, err
}
cert, err := signKey(r, privKey)
if err != nil {
return nil, nil, err
}
return privKey, cert, nil
return crypto.GeneratePrivateKeyAndCert(keySize, *validFor, *myCN)
}

func readKey(secret v1.Secret) (*rsa.PrivateKey, []*x509.Certificate, error) {
Expand Down Expand Up @@ -95,34 +82,3 @@ func writeKey(client kubernetes.Interface, key *rsa.PrivateKey, certs []*x509.Ce
}
return createdSecret.Name, nil
}

func signKey(r io.Reader, key *rsa.PrivateKey) (*x509.Certificate, error) {
// TODO: use certificates API to get this signed by the cluster root CA
// See https://kubernetes.io/docs/tasks/tls/managing-tls-in-a-cluster/

notBefore := time.Now()

serialNo, err := rand.Int(r, new(big.Int).Lsh(big.NewInt(1), 128))
if err != nil {
return nil, err
}

cert := x509.Certificate{
SerialNumber: serialNo,
KeyUsage: x509.KeyUsageEncipherOnly,
NotBefore: notBefore.UTC(),
NotAfter: notBefore.Add(*validFor).UTC(),
Subject: pkix.Name{
CommonName: *myCN,
},
BasicConstraintsValid: true,
IsCA: true,
}

data, err := x509.CreateCertificate(r, &cert, &cert, &key.PublicKey, key)
if err != nil {
return nil, err
}

return x509.ParseCertificate(data)
}
24 changes: 6 additions & 18 deletions cmd/controller/keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
mathrand "math/rand"
"reflect"
"testing"
"time"

"github.com/bitnami-labs/sealed-secrets/pkg/crypto"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
Expand All @@ -21,6 +23,10 @@ func testRand() io.Reader {
return mathrand.New(mathrand.NewSource(42))
}

func signKey(r io.Reader, key *rsa.PrivateKey) (*x509.Certificate, error) {
return crypto.SignKey(r, key, time.Hour, "testcn")
}

func TestReadKey(t *testing.T) {
rand := testRand()

Expand Down Expand Up @@ -87,21 +93,3 @@ func TestWriteKey(t *testing.T) {
t.Errorf("writeKey() created key in wrong namespace!")
}
}

func TestSignKey(t *testing.T) {
rand := testRand()

key, err := rsa.GenerateKey(rand, 512)
if err != nil {
t.Fatalf("Failed to generate test key: %v", err)
}

cert, err := signKey(rand, key)
if err != nil {
t.Errorf("signKey() returned error: %v", err)
}

if !reflect.DeepEqual(cert.PublicKey, &key.PublicKey) {
t.Errorf("cert pubkey != original pubkey")
}
}
27 changes: 19 additions & 8 deletions cmd/kubeseal/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,10 @@ func openCert(certFile string) (io.ReadCloser, error) {
return openCertHTTP(restClient, *controllerNs, *controllerName)
}

func seal(in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, pubKey *rsa.PublicKey) error {
// Seal reads a k8s Secret resource parsed from an input reader by a given codec, encrypts all its secrets
// with a given public key, using the name and namespace found in the input secret, unless explicitly overridden
// by the overrideName and overrideNamespace arguments.
func seal(in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, pubKey *rsa.PublicKey, overrideName, overrideNamespace string) error {
secret, err := readSecret(codecs.UniversalDecoder(), in)
if err != nil {
return err
Expand All @@ -193,10 +196,18 @@ func seal(in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, pu
return fmt.Errorf("Secret.data is empty in input Secret, assuming this is an error and aborting")
}

if overrideName != "" {
secret.Name = overrideName
}

if secret.GetName() == "" {
return fmt.Errorf("Missing metadata.name in input Secret")
}

if overrideNamespace != "" {
secret.Namespace = overrideNamespace
}

if secret.GetNamespace() == "" {
ns, _, err := clientConfig.Namespace()
if err != nil {
Expand Down Expand Up @@ -337,22 +348,22 @@ func decodeSealedSecret(codecs runtimeserializer.CodecFactory, b []byte) (*ssv1a
}

func sealMergingInto(in io.Reader, filename string, codecs runtimeserializer.CodecFactory, pubKey *rsa.PublicKey) error {
var buf bytes.Buffer
if err := seal(in, &buf, codecs, pubKey); err != nil {
b, err := ioutil.ReadFile(filename)
if err != nil {
return err
}

update, err := decodeSealedSecret(codecs, buf.Bytes())
orig, err := decodeSealedSecret(codecs, b)
if err != nil {
return err
}

b, err := ioutil.ReadFile(filename)
if err != nil {
var buf bytes.Buffer
if err := seal(in, &buf, codecs, pubKey, orig.Name, orig.Namespace); err != nil {
return err
}

orig, err := decodeSealedSecret(codecs, b)
update, err := decodeSealedSecret(codecs, buf.Bytes())
if err != nil {
return err
}
Expand Down Expand Up @@ -467,7 +478,7 @@ func run(w io.Writer, secretName, controllerNs, controllerName, certFile string,
return encryptSecretItem(w, secretName, ns, data, sealingScope, pubKey)
}

return seal(os.Stdin, os.Stdout, scheme.Codecs, pubKey)
return seal(os.Stdin, os.Stdout, scheme.Codecs, pubKey, secretName, "")
}

func main() {
Expand Down
96 changes: 76 additions & 20 deletions cmd/kubeseal/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ import (
"os"
"strings"
"testing"
"time"

ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"
"github.com/bitnami-labs/sealed-secrets/pkg/crypto"
"github.com/spf13/pflag"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"

ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"
"github.com/spf13/pflag"
)

const testCert = `
Expand Down Expand Up @@ -156,7 +157,7 @@ func TestSeal(t *testing.T) {
t.Logf("input is: %s", string(inbuf.Bytes()))

outbuf := bytes.Buffer{}
if err := seal(&inbuf, &outbuf, scheme.Codecs, key); err != nil {
if err := seal(&inbuf, &outbuf, scheme.Codecs, key, "", ""); err != nil {
t.Fatalf("seal() returned error: %v", err)
}

Expand Down Expand Up @@ -184,11 +185,37 @@ func TestSeal(t *testing.T) {
// NB: See sealedsecret_test.go for e2e crypto test
}

func mkTestSecret(t *testing.T, key, value string) []byte {
type mkTestSecretOpt func(*mkTestSecretOpts)
type mkTestSecretOpts struct {
secretName string
secretNamespace string
}

func withSecretName(n string) mkTestSecretOpt {
return func(o *mkTestSecretOpts) {
o.secretName = n
}
}

func withSecretNamespace(n string) mkTestSecretOpt {
return func(o *mkTestSecretOpts) {
o.secretNamespace = n
}
}

func mkTestSecret(t *testing.T, key, value string, opts ...mkTestSecretOpt) []byte {
o := mkTestSecretOpts{
secretName: "testname",
secretNamespace: "testns",
}
for _, opt := range opts {
opt(&o)
}

secret := v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "testsecret",
Namespace: "testns",
Name: o.secretName,
Namespace: o.secretNamespace,
Annotations: map[string]string{
key: value, // putting secret here just to have a simple way to test annotation merges
},
Expand All @@ -213,23 +240,36 @@ func mkTestSecret(t *testing.T, key, value string) []byte {
return inbuf.Bytes()
}

func mkTestSealedSecret(t *testing.T, pubKey *rsa.PublicKey, key, value string) []byte {
inbuf := bytes.NewBuffer(mkTestSecret(t, key, value))
func mkTestSealedSecret(t *testing.T, pubKey *rsa.PublicKey, key, value string, opts ...mkTestSecretOpt) []byte {
inbuf := bytes.NewBuffer(mkTestSecret(t, key, value, opts...))
var outbuf bytes.Buffer
if err := seal(inbuf, &outbuf, scheme.Codecs, pubKey); err != nil {
if err := seal(inbuf, &outbuf, scheme.Codecs, pubKey, "", ""); err != nil {
t.Fatalf("seal() returned error: %v", err)
}

return outbuf.Bytes()
}

func TestMergeInto(t *testing.T) {
pubKey, err := parseKey(strings.NewReader(testCert))
func newTestKeyPair(t *testing.T) (*rsa.PublicKey, map[string]*rsa.PrivateKey) {
privKey, _, err := crypto.GeneratePrivateKeyAndCert(2048, time.Hour, "testcn")
if err != nil {
t.Fatalf("Failed to parse test key: %v", err)
t.Fatal(err)
}
pubKey := &privKey.PublicKey

fp, err := crypto.PublicKeyFingerprint(pubKey)
if err != nil {
t.Fatal(err)
}
privKeys := map[string]*rsa.PrivateKey{fp: privKey}

return pubKey, privKeys
}

func TestMergeInto(t *testing.T) {
pubKey, privKeys := newTestKeyPair(t)

merge := func(newSecret, oldSealedSecret []byte) *ssv1alpha1.SealedSecret {
merge := func(t *testing.T, newSecret, oldSealedSecret []byte) *ssv1alpha1.SealedSecret {
f, err := ioutil.TempFile("", "*.json")
if err != nil {
t.Fatal(err)
Expand All @@ -253,11 +293,17 @@ func TestMergeInto(t *testing.T) {
if err != nil {
t.Fatal(err)
}

_, err = merged.Unseal(scheme.Codecs, privKeys)
if err != nil {
t.Fatal(err)
}

return merged
}

{
merged := merge(
t.Run("added", func(t *testing.T) {
merged := merge(t,
mkTestSecret(t, "foo", "secret1"),
mkTestSealedSecret(t, pubKey, "bar", "secret2"),
)
Expand All @@ -279,16 +325,16 @@ func TestMergeInto(t *testing.T) {
checkAdded(merged.Spec.EncryptedData, "foo", "bar")
checkAdded(merged.Spec.Template.Annotations, "foo", "bar")
checkAdded(merged.Spec.Template.Labels, "foo", "bar")
}
})

{
t.Run("updated", func(t *testing.T) {
origSrc := mkTestSealedSecret(t, pubKey, "foo", "secret2")
orig, err := decodeSealedSecret(scheme.Codecs, origSrc)
if err != nil {
t.Fatal(err)
}

merged := merge(
merged := merge(t,
mkTestSecret(t, "foo", "secret1"),
origSrc,
)
Expand All @@ -306,7 +352,17 @@ func TestMergeInto(t *testing.T) {
checkUpdated(orig.Spec.EncryptedData, merged.Spec.EncryptedData, "foo")
checkUpdated(orig.Spec.Template.Annotations, merged.Spec.Template.Annotations, "foo")
checkUpdated(orig.Spec.Template.Labels, merged.Spec.Template.Labels, "foo")
}
})

t.Run("bad name", func(t *testing.T) {
// should not fail even if input has a bad secret name because the name in existing existing sealed secret
// should win (same for namespace).
// TODO(mkm): test for case with scope mismatch too.
merge(t,
mkTestSecret(t, "foo", "secret1", withSecretName("badname"), withSecretNamespace("badns")),
mkTestSealedSecret(t, pubKey, "bar", "secret2"),
)
})
}

func TestVersion(t *testing.T) {
Expand Down
Loading

0 comments on commit 515b6cf

Please sign in to comment.