Add extra coverage in sshagentkms

kms/sshagentkms/sshagentkms.go

@@ -83,11 +83,14 @@ func (s *WrappedSSHSigner) Public() crypto.PublicKey {
 }
 
 // Sign signs the given digest using the ssh agent and returns the signature.
-func (s *WrappedSSHSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) (signature []byte, err error) {
+// Note that because of the way an SSH agent and x509.CreateCertificate works,
+// this signer can only properly sign X509 certificates if the key type is
+// Ed25519.
+func (s *WrappedSSHSigner) Sign(rand io.Reader, data []byte, opts crypto.SignerOpts) (signature []byte, err error) {
 	if signer, ok := s.Signer.(interface {
 		SignWithOpts(io.Reader, []byte, crypto.SignerOpts) (*ssh.Signature, error)
 	}); ok {
-		sig, err := signer.SignWithOpts(rand, digest, opts)
+		sig, err := signer.SignWithOpts(rand, data, opts)
 		if err != nil {
 			return nil, err
 		}
@@ -97,7 +100,7 @@ func (s *WrappedSSHSigner) Sign(rand io.Reader, digest []byte, opts crypto.Signe
 		return sig.Blob, nil
 	}
 
-	sig, err := s.Signer.Sign(rand, digest)
+	sig, err := s.Signer.Sign(rand, data)
 	if err != nil {
 		return nil, err
 	}
@@ -131,7 +134,9 @@ func (k *SSHAgentKMS) findKey(signingKey string) (target int, err error) {
 	return -1, errors.Errorf("SSHAgentKMS couldn't find %s", signingKey)
 }
 
-// CreateSigner returns a new signer configured with the given signing key.
+// CreateSigner returns a new signer configured with the given signing key. Note
+// that because of the way an SSH agent and x509.CreateCertificate works, this
+// signer can only properly sign X509 certificates if the key type is Ed25519.
 func (k *SSHAgentKMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, error) {
 	if req.Signer != nil {
 		return req.Signer, nil

kms/sshagentkms/sshagentkms_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"crypto"
 	"crypto/ecdsa"
+	"crypto/ed25519"
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/x509"
@@ -19,11 +20,10 @@ import (
 	"testing"
 
 	"go.step.sm/crypto/kms/apiv1"
-
+	"go.step.sm/crypto/pemutil"
+	"go.step.sm/crypto/randutil"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
-
-	"go.step.sm/crypto/pemutil"
 )
 
 // Some helpers with inspiration from crypto/ssh/agent/client_test.go
@@ -607,3 +607,72 @@ func TestSSHAgentKMS_CreateKey(t *testing.T) {
 		})
 	}
 }
+
+func TestWrappedSSHSigner(t *testing.T) {
+	pub, priv, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	sshSigner, err := ssh.NewSignerFromSigner(priv)
+	if err != nil {
+		t.Fatal(err)
+	}
+	message, err := randutil.Salt(128)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ws := NewWrappedSignerFromSSHSigner(sshSigner)
+	if !reflect.DeepEqual(ws.Public(), sshSigner.PublicKey()) {
+		t.Errorf("WrappedSigner.Public() = %v, want %v", ws.Public(), sshSigner.PublicKey())
+	}
+
+	sig, err := ws.Sign(rand.Reader, message, crypto.Hash(0))
+	if err != nil {
+		t.Errorf("WrappedSigner.Public() error = %v", err)
+	}
+	if !ed25519.Verify(pub, message, sig) {
+		t.Error("ed25519.Verify() = false, want true")
+	}
+	sshSig := ws.(*WrappedSSHSigner).LastSignature()
+	if err := sshSigner.PublicKey().Verify(message, sshSig); err != nil {
+		t.Errorf("ssh.PublicKey.Verify() error = %v", err)
+	}
+}
+
+func TestWrappedSSHSigner_agent(t *testing.T) {
+	pub, priv, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	sshSigner, err := ssh.NewSignerFromSigner(priv)
+	if err != nil {
+		t.Fatal(err)
+	}
+	message, err := randutil.Salt(128)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	sshAgent, err := NewFromAgent(context.Background(), apiv1.Options{}, startTestKeyringAgent(t, agent.AddedKey{PrivateKey: priv, Comment: "go-test-key"}))
+	if err != nil {
+		t.Errorf("NewFromAgent() error = %v", err)
+	}
+	signer, err := sshAgent.CreateSigner(&apiv1.CreateSignerRequest{
+		SigningKey: "sshagentkms:go-test-key",
+	})
+	if err != nil {
+		t.Errorf("SSHAgentKMS.CreateSigner() error = %v", err)
+	}
+	sig, err := signer.Sign(rand.Reader, message, crypto.Hash(0))
+	if err != nil {
+		t.Errorf("WrappedSigner.Public() error = %v", err)
+	}
+	if !ed25519.Verify(pub, message, sig) {
+		t.Error("ed25519.Verify() = false, want true")
+	}
+	sshSig := signer.(*WrappedSSHSigner).LastSignature()
+	if err := sshSigner.PublicKey().Verify(message, sshSig); err != nil {
+		t.Errorf("ssh.PublicKey.Verify() error = %v", err)
+	}
+}