Merge pull request #1007 from weaveworks/issue/1002-ro-key-volume

Generate keys in a separate tmpfs volume

cluster/kubernetes/sshkeyring.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"sync"
 
+	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/typed/core/v1"
 
@@ -30,14 +31,14 @@ type SSHKeyRingConfig struct {
 	SecretDataKey         string // e.g. "identity"
 	KeyBits               ssh.OptionalValue
 	KeyType               ssh.OptionalValue
+	KeyGenDir             string // a tmpfs mount; e.g., /var/fluxd/ssh
 }
 
 type sshKeyRing struct {
 	sync.RWMutex
 	SSHKeyRingConfig
-	publicKey              ssh.PublicKey
-	expectedPrivateKeyPath string
-	realPrivateKeyPath     string
+	publicKey      ssh.PublicKey
+	privateKeyPath string
 }
 
 // NewSSHKeyRing constructs an sshKeyRing backed by a kubernetes secret
@@ -46,28 +47,32 @@ type sshKeyRing struct {
 // generated key if none was found.
 func NewSSHKeyRing(config SSHKeyRingConfig) (*sshKeyRing, error) {
 	skr := &sshKeyRing{SSHKeyRingConfig: config}
-	skr.expectedPrivateKeyPath = filepath.Join(skr.SecretVolumeMountPath, skr.SecretDataKey)
+	mountedPrivateKeyPath := filepath.Join(skr.SecretVolumeMountPath, skr.SecretDataKey)
 
-	fileInfo, err := os.Stat(skr.expectedPrivateKeyPath)
+	fileInfo, err := os.Stat(mountedPrivateKeyPath)
 	switch {
 	case os.IsNotExist(err):
+		// The key is not mounted from the secret, so generate one.
 		if err := skr.Regenerate(); err != nil {
 			return nil, err
 		}
-		skr.publicKey, skr.realPrivateKeyPath = skr.KeyPair()
 	case err != nil:
-		return nil, err
+		// There's some other problem with that bit of filesystem
+		return nil, errors.Wrap(err, "checking for mounted secret")
 	case fileInfo.Mode() != privateKeyFileMode:
-		if err := os.Chmod(skr.expectedPrivateKeyPath, privateKeyFileMode); err != nil {
-			return nil, err
+		// The key is mounted, but not the right permissions; since
+		// it's likely to be read-only, we may not be able to rectify
+		// this, but let's try.
+		if err := os.Chmod(mountedPrivateKeyPath, privateKeyFileMode); err != nil {
+			return nil, errors.Wrap(err, "failed to chmod identity file")
 		}
 		fallthrough
 	default:
-		publicKey, err := ssh.ExtractPublicKey(skr.expectedPrivateKeyPath)
+		skr.privateKeyPath = mountedPrivateKeyPath
+		publicKey, err := ssh.ExtractPublicKey(skr.privateKeyPath)
 		if err != nil {
-			return nil, err
+			return nil, errors.Wrap(err, "extracting public key")
 		}
-		skr.realPrivateKeyPath = skr.expectedPrivateKeyPath
 		skr.publicKey = publicKey
 	}
 
@@ -77,16 +82,16 @@ func NewSSHKeyRing(config SSHKeyRingConfig) (*sshKeyRing, error) {
 // KeyPair returns the current public key and the path to its corresponding
 // private key. The private key file is guaranteed to exist for the lifetime of
 // the process, however as the returned pair can be discarded from the keyring
-// at any time by use of the regenerate() method it is inadvisable to cache the
+// at any time by use of the Regenerate() method it is inadvisable to cache the
 // results for long periods; instead request the key pair from the ring
 // immediately prior to each use.
 func (skr *sshKeyRing) KeyPair() (publicKey ssh.PublicKey, privateKeyPath string) {
 	skr.RLock()
 	defer skr.RUnlock()
-	return skr.publicKey, skr.expectedPrivateKeyPath
+	return skr.publicKey, skr.privateKeyPath
 }
 
-// regenerate creates a new keypair in the configured SecretVolumeMountPath and
+// Regenerate creates a new keypair in the configured SecretVolumeMountPath and
 // updates the kubernetes secret resource with the private key so that it will
 // be available to the keyring after restart. If this operation is successful
 // the keyPair() method will return the new pair; if it fails for any reason,
@@ -97,14 +102,14 @@ func (skr *sshKeyRing) KeyPair() (publicKey ssh.PublicKey, privateKeyPath string
 // syscall.Mlockall(MCL_FUTURE) in conjunction with an appropriate ulimit to
 // ensure the private key isn't unintentionally written to persistent storage.
 func (skr *sshKeyRing) Regenerate() error {
-	privateKeyPath, privateKey, publicKey, err := ssh.KeyGen(skr.KeyBits, skr.KeyType, skr.SecretVolumeMountPath)
+	tmpPrivateKeyPath, privateKey, publicKey, err := ssh.KeyGen(skr.KeyBits, skr.KeyType, skr.KeyGenDir)
 	if err != nil {
 		return err
 	}
 
 	// Prepare a symlink pointing at the new key, to be moved later.
-	tmpSymlinkPath := filepath.Join(filepath.Dir(privateKeyPath), "tmp-identity")
-	if err = os.Symlink(privateKeyPath, tmpSymlinkPath); err != nil {
+	tmpSymlinkPath := filepath.Join(filepath.Dir(tmpPrivateKeyPath), "tmp-identity")
+	if err = os.Symlink(tmpPrivateKeyPath, tmpSymlinkPath); err != nil {
 		return err
 	}
 	if err = os.Chmod(tmpSymlinkPath, privateKeyFileMode); err != nil {
@@ -130,13 +135,14 @@ func (skr *sshKeyRing) Regenerate() error {
 	// The secret is updated, and Kubernetes will eventually make sure
 	// it's mounted and that `identity` points at it. In the meantime,
 	// change the symlink to point to our copy of it.
-	if err = os.Rename(tmpSymlinkPath, skr.expectedPrivateKeyPath); err != nil {
+	generatedPrivateKeyPath := filepath.Join(skr.KeyGenDir, skr.SecretDataKey)
+	if err = os.Rename(tmpSymlinkPath, generatedPrivateKeyPath); err != nil {
 		os.Remove(tmpSymlinkPath)
 		return err
 	}
 
 	skr.Lock()
-	skr.realPrivateKeyPath = privateKeyPath
+	skr.privateKeyPath = generatedPrivateKeyPath
 	skr.publicKey = publicKey
 	skr.Unlock()
 

cmd/fluxd/main.go

@@ -101,8 +101,9 @@ func main() {
 		k8sSecretVolumeMountPath = fs.String("k8s-secret-volume-mount-path", "/etc/fluxd/ssh", "Mount location of the k8s secret storing the private SSH key")
 		k8sSecretDataKey         = fs.String("k8s-secret-data-key", "identity", "Data key holding the private SSH key within the k8s secret")
 		// SSH key generation
-		sshKeyBits = optionalVar(fs, &ssh.KeyBitsValue{}, "ssh-keygen-bits", "-b argument to ssh-keygen (default unspecified)")
-		sshKeyType = optionalVar(fs, &ssh.KeyTypeValue{}, "ssh-keygen-type", "-t argument to ssh-keygen (default unspecified)")
+		sshKeyBits   = optionalVar(fs, &ssh.KeyBitsValue{}, "ssh-keygen-bits", "-b argument to ssh-keygen (default unspecified)")
+		sshKeyType   = optionalVar(fs, &ssh.KeyTypeValue{}, "ssh-keygen-type", "-t argument to ssh-keygen (default unspecified)")
+		sshKeygenDir = fs.String("ssh-keygen-dir", "", "directory, ideally on a tmpfs volume, in which to generate new SSH keys when necessary")
 
 		upstreamURL = fs.String("connect", "", "Connect to an upstream service e.g., Weave Cloud, at this base address")
 		token       = fs.String("token", "", "Authentication token for upstream service")
@@ -132,6 +133,8 @@ func main() {
 	}
 	logger.Log("started", true)
 
+	// Argument validation
+
 	// Sort out values for the git tag and notes ref. There are
 	// running deployments that assume the defaults as given, so don't
 	// mess with those unless explicitly told.
@@ -150,6 +153,11 @@ func main() {
 		os.Exit(1)
 	}
 
+	if *sshKeygenDir == "" {
+		logger.Log("info", fmt.Sprintf("SSH keygen dir (--ssh-keygen-dir) not provided, so using the deploy key volume (--k8s-secret-volume-mount-path=%s); this may cause problems if the deploy key volume is mounted read-only", *k8sSecretVolumeMountPath))
+		*sshKeygenDir = *k8sSecretVolumeMountPath
+	}
+
 	// Cluster component.
 	var clusterVersion string
 	var sshKeyRing ssh.KeyRing
@@ -192,6 +200,7 @@ func main() {
 			SecretDataKey:         *k8sSecretDataKey,
 			KeyBits:               sshKeyBits,
 			KeyType:               sshKeyType,
+			KeyGenDir:             *sshKeygenDir,
 		})
 		if err != nil {
 			logger.Log("err", err)

deploy/flux-deployment.yaml

@@ -15,8 +15,17 @@ spec:
       serviceAccount: flux
       volumes:
       - name: git-key
+        defaultMode: 0400 # when mounted read-only, we won't be able to chmod
         secret:
           secretName: flux-git-deploy
+
+      # This is a tmpfs used for generating SSH keys. In K8s >= 1.10,
+      # mounted secrets are read-only, so we need a separate volume we
+      # can write to.
+      - name: git-keygen
+        emptyDir:
+          medium: Memory
+
       containers:
       - name: flux
         # There are no ":latest" images for flux. Find the most recent
@@ -28,7 +37,10 @@ spec:
         - containerPort: 3030 # informational
         volumeMounts:
         - name: git-key
-          mountPath: /etc/fluxd/ssh
+          mountPath: /etc/fluxd/ssh # to match image's ~/.ssh/config
+          readOnly: true # this will be the case perforce in K8s >=1.10
+        - name: git-keygen
+          mountPath: /var/fluxd/keygen # to match image's ~/.ssh/config
         args:
 
         # if you deployed memcached in a different namespace to flux,
@@ -37,9 +49,14 @@ spec:
         # - --memcached-hostname=memcached.default.svc.cluster.local
         # - --memcached-service=memcached
 
+        # this must be supplied, and be in the tmpfs (emptyDir)
+        # mounted above, for K8s >= 1.10
+        - --ssh-keygen-dir=/var/fluxd/keygen
+
         # replace (at least) the following URL
         - --git-url=git@github.com:weaveworks/flux-example
         - --git-branch=master
+
         # include these next two to connect to an "upstream" service
         # (e.g., Weave Cloud). The token is particular to the service.
         # - --connect=wss://cloud.weave.works/api/flux

docker/ssh_config

@@ -1,4 +1,5 @@
 Host *
 StrictHostKeyChecking yes
 IdentityFile /etc/fluxd/ssh/identity
+IdentityFile /var/fluxd/keygen/identity
 LogLevel error

ssh/keygen.go

@@ -2,6 +2,7 @@ package ssh
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os/exec"
@@ -156,9 +157,9 @@ type PublicKey struct {
 // ExtractPublicKey extracts and returns the public key from the specified
 // private key, along with its fingerprint hashes.
 func ExtractPublicKey(privateKeyPath string) (PublicKey, error) {
-	keyBytes, err := exec.Command("ssh-keygen", "-y", "-f", privateKeyPath).Output()
+	keyBytes, err := exec.Command("ssh-keygen", "-y", "-f", privateKeyPath).CombinedOutput()
 	if err != nil {
-		return PublicKey{}, err
+		return PublicKey{}, errors.New(string(keyBytes))
 	}
 
 	md5Print, err := ExtractFingerprint(privateKeyPath, "md5")