x/crypto/ssh/agent: honor constraints on keys in the keyring.

If a key is added to an agent keyring with constraints, honor them.
This will remove keys when they've been on the keyring for
LifetimeSecs seconds or longer and will ask the user to confirm a
signing operation if ConfirmBeforeUse is set.

Change-Id: I633713c5f78b13a628a5d752f11b306b6e16a2ef
Reviewed-on: https://go-review.googlesource.com/28956
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh/agent/client_test.go

@@ -86,6 +86,11 @@ func testAgent(t *testing.T, key interface{}, cert *ssh.Certificate, lifetimeSec
 	testAgentInterface(t, agent, key, cert, lifetimeSecs)
 }
 
+func testKeyring(t *testing.T, key interface{}, cert *ssh.Certificate, lifetimeSecs uint32) {
+	a := NewKeyring()
+	testAgentInterface(t, a, key, cert, lifetimeSecs)
+}
+
 func testAgentInterface(t *testing.T, agent Agent, key interface{}, cert *ssh.Certificate, lifetimeSecs uint32) {
 	signer, err := ssh.NewSignerFromKey(key)
 	if err != nil {
@@ -137,11 +142,25 @@ func testAgentInterface(t *testing.T, agent Agent, key interface{}, cert *ssh.Ce
 	if err := pubKey.Verify(data, sig); err != nil {
 		t.Fatalf("Verify(%s): %v", pubKey.Type(), err)
 	}
+
+	// If the key has a lifetime, is it removed when it should be?
+	if lifetimeSecs > 0 {
+		time.Sleep(time.Second*time.Duration(lifetimeSecs) + 100*time.Millisecond)
+		keys, err := agent.List()
+		if err != nil {
+			t.Fatalf("List: %v", err)
+		}
+		if len(keys) > 0 {
+			t.Fatalf("key not expired")
+		}
+	}
+
 }
 
 func TestAgent(t *testing.T) {
 	for _, keyType := range []string{"rsa", "dsa", "ecdsa", "ed25519"} {
 		testAgent(t, testPrivateKeys[keyType], nil, 0)
+		testKeyring(t, testPrivateKeys[keyType], nil, 1)
 	}
 }
 
@@ -154,10 +173,7 @@ func TestCert(t *testing.T) {
 	cert.SignCert(rand.Reader, testSigners["ecdsa"])
 
 	testAgent(t, testPrivateKeys["rsa"], cert, 0)
-}
-
-func TestConstraints(t *testing.T) {
-	testAgent(t, testPrivateKeys["rsa"], nil, 3600 /* lifetime in seconds */)
+	testKeyring(t, testPrivateKeys["rsa"], cert, 1)
 }
 
 // netPipe is analogous to net.Pipe, but it uses a real net.Conn, and

ssh/agent/keyring.go

@@ -11,6 +11,7 @@ import (
 	"errors"
 	"fmt"
 	"sync"
+	"time"
 
 	"golang.org/x/crypto/ssh"
 )
@@ -18,6 +19,7 @@ import (
 type privKey struct {
 	signer  ssh.Signer
 	comment string
+	expire  *time.Time
 }
 
 type keyring struct {
@@ -48,15 +50,9 @@ func (r *keyring) RemoveAll() error {
 	return nil
 }
 
-// Remove removes all identities with the given public key.
-func (r *keyring) Remove(key ssh.PublicKey) error {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-	if r.locked {
-		return errLocked
-	}
-
-	want := key.Marshal()
+// removeLocked does the actual key removal. The caller must already be holding the
+// keyring mutex.
+func (r *keyring) removeLocked(want []byte) error {
 	found := false
 	for i := 0; i < len(r.keys); {
 		if bytes.Equal(r.keys[i].signer.PublicKey().Marshal(), want) {
@@ -75,7 +71,18 @@ func (r *keyring) Remove(key ssh.PublicKey) error {
 	return nil
 }
 
-// Lock locks the agent. Sign and Remove will fail, and List will empty an empty list.
+// Remove removes all identities with the given public key.
+func (r *keyring) Remove(key ssh.PublicKey) error {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	if r.locked {
+		return errLocked
+	}
+
+	return r.removeLocked(key.Marshal())
+}
+
+// Lock locks the agent. Sign and Remove will fail, and List will return an empty list.
 func (r *keyring) Lock(passphrase []byte) error {
 	r.mu.Lock()
 	defer r.mu.Unlock()
@@ -104,6 +111,17 @@ func (r *keyring) Unlock(passphrase []byte) error {
 	return nil
 }
 
+// expireKeysLocked removes expired keys from the keyring. If a key was added
+// with a lifetimesecs contraint and seconds >= lifetimesecs seconds have
+// ellapsed, it is removed. The caller *must* be holding the keyring mutex.
+func (r *keyring) expireKeysLocked() {
+	for _, k := range r.keys {
+		if k.expire != nil && time.Now().After(*k.expire) {
+			r.removeLocked(k.signer.PublicKey().Marshal())
+		}
+	}
+}
+
 // List returns the identities known to the agent.
 func (r *keyring) List() ([]*Key, error) {
 	r.mu.Lock()
@@ -113,6 +131,7 @@ func (r *keyring) List() ([]*Key, error) {
 		return nil, nil
 	}
 
+	r.expireKeysLocked()
 	var ids []*Key
 	for _, k := range r.keys {
 		pub := k.signer.PublicKey()
@@ -146,7 +165,17 @@ func (r *keyring) Add(key AddedKey) error {
 		}
 	}
 
-	r.keys = append(r.keys, privKey{signer, key.Comment})
+	p := privKey{
+		signer:  signer,
+		comment: key.Comment,
+	}
+
+	if key.LifetimeSecs > 0 {
+		t := time.Now().Add(time.Duration(key.LifetimeSecs) * time.Second)
+		p.expire = &t
+	}
+
+	r.keys = append(r.keys, p)
 
 	return nil
 }
@@ -159,6 +188,7 @@ func (r *keyring) Sign(key ssh.PublicKey, data []byte) (*ssh.Signature, error) {
 		return nil, errLocked
 	}
 
+	r.expireKeysLocked()
 	wanted := key.Marshal()
 	for _, k := range r.keys {
 		if bytes.Equal(k.signer.PublicKey().Marshal(), wanted) {
@@ -176,6 +206,7 @@ func (r *keyring) Signers() ([]ssh.Signer, error) {
 		return nil, errLocked
 	}
 
+	r.expireKeysLocked()
 	s := make([]ssh.Signer, 0, len(r.keys))
 	for _, k := range r.keys {
 		s = append(s, k.signer)

ssh/agent/keyring_test.go

@@ -4,9 +4,7 @@
 
 package agent
 
-import (
-	"testing"
-)
+import "testing"
 
 func addTestKey(t *testing.T, a Agent, keyName string) {
 	err := a.Add(AddedKey{