From 7d5b0be716b9d6d4269afdaae10032bb296d3cdf Mon Sep 17 00:00:00 2001 From: Peter Moody Date: Wed, 8 Jul 2015 15:14:43 -0700 Subject: [PATCH] crypto/ssh: allow identities to be constrained. The ssh-agent protocol allows the usage of keys and certs added to a given agent to be constrained in certain ways. The only constraints currently supported are lifetime (keys expire after some number of seconds) and confirmation (the agent requires user confirmation before performing any operations with the private key). Change-Id: Idba5760db929805bf3da43fdcaca53ae6c479ca4 Reviewed-on: https://go-review.googlesource.com/12260 Reviewed-by: Adam Langley Run-TryBot: Adam Langley Reviewed-by: Peter Moody --- ssh/agent/client.go | 194 +++++++++++++++++++++++------------- ssh/agent/client_test.go | 29 ++++-- ssh/agent/keyring.go | 11 +- ssh/agent/server.go | 2 +- ssh/agent/server_test.go | 4 +- ssh/test/agent_unix_test.go | 11 +- 6 files changed, 161 insertions(+), 90 deletions(-) diff --git a/ssh/agent/client.go b/ssh/agent/client.go index f5527e8..8c856a0 100644 --- a/ssh/agent/client.go +++ b/ssh/agent/client.go @@ -36,9 +36,8 @@ type Agent interface { // in [PROTOCOL.agent] section 2.6.2. Sign(key ssh.PublicKey, data []byte) (*ssh.Signature, error) - // Add adds a private key to the agent. If a certificate - // is given, that certificate is added as public key. - Add(s interface{}, cert *ssh.Certificate, comment string) error + // Add adds a private key to the agent. + Add(key AddedKey) error // Remove removes all identities with the given public key. Remove(key ssh.PublicKey) error @@ -56,6 +55,24 @@ type Agent interface { Signers() ([]ssh.Signer, error) } +// AddedKey describes an SSH key to be added to an Agent. +type AddedKey struct { + // PrivateKey must be a *rsa.PrivateKey, *dsa.PrivateKey or + // *ecdsa.PrivateKey, which will be inserted into the agent. + PrivateKey interface{} + // Certificate, if not nil, is communicated to the agent and will be + // stored with the key. + Certificate *ssh.Certificate + // Comment is an optional, free-form string. + Comment string + // LifetimeSecs, if not zero, is the number of seconds that the + // agent will store the key for. + LifetimeSecs uint32 + // ConfirmBeforeUse, if true, requests that the agent confirm with the + // user before each use of this key. + ConfirmBeforeUse bool +} + // See [PROTOCOL.agent], section 3. const ( agentRequestV1Identities = 1 @@ -368,36 +385,39 @@ func unmarshal(packet []byte) (interface{}, error) { } type rsaKeyMsg struct { - Type string `sshtype:"17"` - N *big.Int - E *big.Int - D *big.Int - Iqmp *big.Int // IQMP = Inverse Q Mod P - P *big.Int - Q *big.Int - Comments string + Type string `sshtype:"17"` + N *big.Int + E *big.Int + D *big.Int + Iqmp *big.Int // IQMP = Inverse Q Mod P + P *big.Int + Q *big.Int + Comments string + Constraints []byte `ssh:"rest"` } type dsaKeyMsg struct { - Type string `sshtype:"17"` - P *big.Int - Q *big.Int - G *big.Int - Y *big.Int - X *big.Int - Comments string + Type string `sshtype:"17"` + P *big.Int + Q *big.Int + G *big.Int + Y *big.Int + X *big.Int + Comments string + Constraints []byte `ssh:"rest"` } type ecdsaKeyMsg struct { - Type string `sshtype:"17"` - Curve string - KeyBytes []byte - D *big.Int - Comments string + Type string `sshtype:"17"` + Curve string + KeyBytes []byte + D *big.Int + Comments string + Constraints []byte `ssh:"rest"` } // Insert adds a private key to the agent. -func (c *client) insertKey(s interface{}, comment string) error { +func (c *client) insertKey(s interface{}, comment string, constraints []byte) error { var req []byte switch k := s.(type) { case *rsa.PrivateKey: @@ -406,37 +426,46 @@ func (c *client) insertKey(s interface{}, comment string) error { } k.Precompute() req = ssh.Marshal(rsaKeyMsg{ - Type: ssh.KeyAlgoRSA, - N: k.N, - E: big.NewInt(int64(k.E)), - D: k.D, - Iqmp: k.Precomputed.Qinv, - P: k.Primes[0], - Q: k.Primes[1], - Comments: comment, + Type: ssh.KeyAlgoRSA, + N: k.N, + E: big.NewInt(int64(k.E)), + D: k.D, + Iqmp: k.Precomputed.Qinv, + P: k.Primes[0], + Q: k.Primes[1], + Comments: comment, + Constraints: constraints, }) case *dsa.PrivateKey: req = ssh.Marshal(dsaKeyMsg{ - Type: ssh.KeyAlgoDSA, - P: k.P, - Q: k.Q, - G: k.G, - Y: k.Y, - X: k.X, - Comments: comment, + Type: ssh.KeyAlgoDSA, + P: k.P, + Q: k.Q, + G: k.G, + Y: k.Y, + X: k.X, + Comments: comment, + Constraints: constraints, }) case *ecdsa.PrivateKey: nistID := fmt.Sprintf("nistp%d", k.Params().BitSize) req = ssh.Marshal(ecdsaKeyMsg{ - Type: "ecdsa-sha2-" + nistID, - Curve: nistID, - KeyBytes: elliptic.Marshal(k.Curve, k.X, k.Y), - D: k.D, - Comments: comment, + Type: "ecdsa-sha2-" + nistID, + Curve: nistID, + KeyBytes: elliptic.Marshal(k.Curve, k.X, k.Y), + D: k.D, + Comments: comment, + Constraints: constraints, }) default: return fmt.Errorf("agent: unsupported key type %T", s) } + + // if constraints are present then the message type needs to be changed. + if len(constraints) != 0 { + req[0] = agentAddIdConstrained + } + resp, err := c.call(req) if err != nil { return err @@ -448,40 +477,57 @@ func (c *client) insertKey(s interface{}, comment string) error { } type rsaCertMsg struct { - Type string `sshtype:"17"` - CertBytes []byte - D *big.Int - Iqmp *big.Int // IQMP = Inverse Q Mod P - P *big.Int - Q *big.Int - Comments string + Type string `sshtype:"17"` + CertBytes []byte + D *big.Int + Iqmp *big.Int // IQMP = Inverse Q Mod P + P *big.Int + Q *big.Int + Comments string + Constraints []byte `ssh:"rest"` } type dsaCertMsg struct { - Type string `sshtype:"17"` - CertBytes []byte - X *big.Int - Comments string + Type string `sshtype:"17"` + CertBytes []byte + X *big.Int + Comments string + Constraints []byte `ssh:"rest"` } type ecdsaCertMsg struct { - Type string `sshtype:"17"` - CertBytes []byte - D *big.Int - Comments string + Type string `sshtype:"17"` + CertBytes []byte + D *big.Int + Comments string + Constraints []byte `ssh:"rest"` } // Insert adds a private key to the agent. If a certificate is given, // that certificate is added instead as public key. -func (c *client) Add(s interface{}, cert *ssh.Certificate, comment string) error { - if cert == nil { - return c.insertKey(s, comment) +func (c *client) Add(key AddedKey) error { + var constraints []byte + + if secs := key.LifetimeSecs; secs != 0 { + constraints = append(constraints, agentConstrainLifetime) + + var secsBytes [4]byte + binary.BigEndian.PutUint32(secsBytes[:], secs) + constraints = append(constraints, secsBytes[:]...) + } + + if key.ConfirmBeforeUse { + constraints = append(constraints, agentConstrainConfirm) + } + + if cert := key.Certificate; cert == nil { + return c.insertKey(key.PrivateKey, key.Comment, constraints) } else { - return c.insertCert(s, cert, comment) + return c.insertCert(key.PrivateKey, cert, key.Comment, constraints) } } -func (c *client) insertCert(s interface{}, cert *ssh.Certificate, comment string) error { +func (c *client) insertCert(s interface{}, cert *ssh.Certificate, comment string, constraints []byte) error { var req []byte switch k := s.(type) { case *rsa.PrivateKey: @@ -490,13 +536,14 @@ func (c *client) insertCert(s interface{}, cert *ssh.Certificate, comment string } k.Precompute() req = ssh.Marshal(rsaCertMsg{ - Type: cert.Type(), - CertBytes: cert.Marshal(), - D: k.D, - Iqmp: k.Precomputed.Qinv, - P: k.Primes[0], - Q: k.Primes[1], - Comments: comment, + Type: cert.Type(), + CertBytes: cert.Marshal(), + D: k.D, + Iqmp: k.Precomputed.Qinv, + P: k.Primes[0], + Q: k.Primes[1], + Comments: comment, + Constraints: constraints, }) case *dsa.PrivateKey: req = ssh.Marshal(dsaCertMsg{ @@ -516,6 +563,11 @@ func (c *client) insertCert(s interface{}, cert *ssh.Certificate, comment string return fmt.Errorf("agent: unsupported key type %T", s) } + // if constraints are present then the message type needs to be changed. + if len(constraints) != 0 { + req[0] = agentAddIdConstrained + } + signer, err := ssh.NewSignerFromKey(s) if err != nil { return err diff --git a/ssh/agent/client_test.go b/ssh/agent/client_test.go index 80e2c2c..ec7198d 100644 --- a/ssh/agent/client_test.go +++ b/ssh/agent/client_test.go @@ -78,14 +78,14 @@ func startAgent(t *testing.T) (client Agent, socket string, cleanup func()) { } } -func testAgent(t *testing.T, key interface{}, cert *ssh.Certificate) { +func testAgent(t *testing.T, key interface{}, cert *ssh.Certificate, lifetimeSecs uint32) { agent, _, cleanup := startAgent(t) defer cleanup() - testAgentInterface(t, agent, key, cert) + testAgentInterface(t, agent, key, cert, lifetimeSecs) } -func testAgentInterface(t *testing.T, agent Agent, key interface{}, cert *ssh.Certificate) { +func testAgentInterface(t *testing.T, agent Agent, key interface{}, cert *ssh.Certificate, lifetimeSecs uint32) { signer, err := ssh.NewSignerFromKey(key) if err != nil { t.Fatalf("NewSignerFromKey(%T): %v", key, err) @@ -100,10 +100,15 @@ func testAgentInterface(t *testing.T, agent Agent, key interface{}, cert *ssh.Ce // Attempt to insert the key, with certificate if specified. var pubKey ssh.PublicKey if cert != nil { - err = agent.Add(key, cert, "comment") + err = agent.Add(AddedKey{ + PrivateKey: key, + Certificate: cert, + Comment: "comment", + LifetimeSecs: lifetimeSecs, + }) pubKey = cert } else { - err = agent.Add(key, nil, "comment") + err = agent.Add(AddedKey{PrivateKey: key, Comment: "comment", LifetimeSecs: lifetimeSecs}) pubKey = signer.PublicKey() } if err != nil { @@ -135,7 +140,7 @@ func testAgentInterface(t *testing.T, agent Agent, key interface{}, cert *ssh.Ce func TestAgent(t *testing.T) { for _, keyType := range []string{"rsa", "dsa", "ecdsa"} { - testAgent(t, testPrivateKeys[keyType], nil) + testAgent(t, testPrivateKeys[keyType], nil, 0) } } @@ -147,7 +152,11 @@ func TestCert(t *testing.T) { } cert.SignCert(rand.Reader, testSigners["ecdsa"]) - testAgent(t, testPrivateKeys["rsa"], cert) + testAgent(t, testPrivateKeys["rsa"], cert, 0) +} + +func TestConstraints(t *testing.T) { + testAgent(t, testPrivateKeys["rsa"], nil, 3600 /* lifetime in seconds */) } // netPipe is analogous to net.Pipe, but it uses a real net.Conn, and @@ -185,7 +194,7 @@ func TestAuth(t *testing.T) { agent, _, cleanup := startAgent(t) defer cleanup() - if err := agent.Add(testPrivateKeys["rsa"], nil, "comment"); err != nil { + if err := agent.Add(AddedKey{PrivateKey: testPrivateKeys["rsa"], Comment: "comment"}); err != nil { t.Errorf("Add: %v", err) } @@ -223,10 +232,10 @@ func TestLockClient(t *testing.T) { } func testLockAgent(agent Agent, t *testing.T) { - if err := agent.Add(testPrivateKeys["rsa"], nil, "comment 1"); err != nil { + if err := agent.Add(AddedKey{PrivateKey: testPrivateKeys["rsa"], Comment: "comment 1"}); err != nil { t.Errorf("Add: %v", err) } - if err := agent.Add(testPrivateKeys["dsa"], nil, "comment dsa"); err != nil { + if err := agent.Add(AddedKey{PrivateKey: testPrivateKeys["dsa"], Comment: "comment dsa"}); err != nil { t.Errorf("Add: %v", err) } if keys, err := agent.List(); err != nil { diff --git a/ssh/agent/keyring.go b/ssh/agent/keyring.go index 8ac2009..11b72df 100644 --- a/ssh/agent/keyring.go +++ b/ssh/agent/keyring.go @@ -125,27 +125,28 @@ func (r *keyring) List() ([]*Key, error) { } // Insert adds a private key to the keyring. If a certificate -// is given, that certificate is added as public key. -func (r *keyring) Add(priv interface{}, cert *ssh.Certificate, comment string) error { +// is given, that certificate is added as public key. Note that +// any constraints given are ignored. +func (r *keyring) Add(key AddedKey) error { r.mu.Lock() defer r.mu.Unlock() if r.locked { return errLocked } - signer, err := ssh.NewSignerFromKey(priv) + signer, err := ssh.NewSignerFromKey(key.PrivateKey) if err != nil { return err } - if cert != nil { + if cert := key.Certificate; cert != nil { signer, err = ssh.NewCertSigner(cert, signer) if err != nil { return err } } - r.keys = append(r.keys, privKey{signer, comment}) + r.keys = append(r.keys, privKey{signer, key.Comment}) return nil } diff --git a/ssh/agent/server.go b/ssh/agent/server.go index be9df0e..b21a201 100644 --- a/ssh/agent/server.go +++ b/ssh/agent/server.go @@ -167,7 +167,7 @@ func (s *server) insertIdentity(req []byte) error { } priv.Precompute() - return s.agent.Add(&priv, nil, k.Comments) + return s.agent.Add(AddedKey{PrivateKey: &priv, Comment: k.Comments}) } return fmt.Errorf("not implemented: %s", record.Type) } diff --git a/ssh/agent/server_test.go b/ssh/agent/server_test.go index def5f8c..ef0ab29 100644 --- a/ssh/agent/server_test.go +++ b/ssh/agent/server_test.go @@ -21,7 +21,7 @@ func TestServer(t *testing.T) { go ServeAgent(NewKeyring(), c2) - testAgentInterface(t, client, testPrivateKeys["rsa"], nil) + testAgentInterface(t, client, testPrivateKeys["rsa"], nil, 0) } func TestLockServer(t *testing.T) { @@ -72,6 +72,6 @@ func TestSetupForwardAgent(t *testing.T) { go ssh.DiscardRequests(reqs) agentClient := NewClient(ch) - testAgentInterface(t, agentClient, testPrivateKeys["rsa"], nil) + testAgentInterface(t, agentClient, testPrivateKeys["rsa"], nil, 0) conn.Close() } diff --git a/ssh/test/agent_unix_test.go b/ssh/test/agent_unix_test.go index 502e24f..f481253 100644 --- a/ssh/test/agent_unix_test.go +++ b/ssh/test/agent_unix_test.go @@ -21,7 +21,16 @@ func TestAgentForward(t *testing.T) { defer conn.Close() keyring := agent.NewKeyring() - keyring.Add(testPrivateKeys["dsa"], nil, "") + if err := keyring.Add(agent.AddedKey{PrivateKey: testPrivateKeys["dsa"]}); err != nil { + t.Fatalf("Error adding key: %s", err) + } + if err := keyring.Add(agent.AddedKey{ + PrivateKey: testPrivateKeys["dsa"], + ConfirmBeforeUse: true, + LifetimeSecs: 3600, + }); err != nil { + t.Fatalf("Error adding key with constraints: %s", err) + } pub := testPublicKeys["dsa"] sess, err := conn.NewSession()