go.crypto/ssh: some cleanup

Simplify MarshalAuthorizedKey by using the algoName func.
Make the algoName func be very specific about supported key types in openssh certs.
Generalize some of the commentary that previously mentioned specific key types.

R=agl, dave
CC=golang-dev
https://golang.org/cl/6938067

ssh/agent.go

@@ -98,8 +98,7 @@ func (ak *AgentKey) String() string {
 	return s
 }
 
-// Key returns an agent's public key as a *rsa.PublicKey, *dsa.PublicKey, or
-// *OpenSSHCertV01.
+// Key returns an agent's public key as one of the supported key or certificate types.
 func (ak *AgentKey) Key() (interface{}, error) {
 	if key, _, ok := parsePubKey(ak.blob); ok {
 		return key, nil
@@ -204,8 +203,7 @@ func (ac *AgentClient) RequestIdentities() ([]*AgentKey, error) {
 }
 
 // SignRequest requests the signing of data by the agent using a protocol 2 key
-// as defined in [PROTOCOL.agent] section 2.6.2.  Supported key types include
-// *rsa.PublicKey, *dsa.PublicKey, *OpenSSHCertV01.
+// as defined in [PROTOCOL.agent] section 2.6.2.
 func (ac *AgentClient) SignRequest(key interface{}, data []byte) ([]byte, error) {
 	req := marshal(agentSignRequest, signRequestAgentMsg{
 		KeyBlob: serializePublickey(key),

ssh/certs.go

@@ -11,7 +11,8 @@ import (
 	"time"
 )
 
-// String constants in [PROTOCOL.certkeys] for certificate algorithm names.
+// These constants from [PROTOCOL.certkeys] represent the algorithm names
+// for certificate types supported by this package.
 const (
 	CertAlgoRSAv01      = "ssh-rsa-cert-v01@openssh.com"
 	CertAlgoDSAv01      = "ssh-dss-cert-v01@openssh.com"

ssh/common.go

@@ -255,7 +255,21 @@ func algoName(key interface{}) string {
 			return KeyAlgoECDSA521
 		}
 	case *OpenSSHCertV01:
-		return algoName(key.(*OpenSSHCertV01).Key) + "-cert-v01@openssh.com"
+		switch key.(*OpenSSHCertV01).Key.(type) {
+		case *rsa.PublicKey:
+			return CertAlgoRSAv01
+		case *dsa.PublicKey:
+			return CertAlgoDSAv01
+		case *ecdsa.PublicKey:
+			switch key.(*OpenSSHCertV01).Key.(*ecdsa.PublicKey).Params().BitSize {
+			case 256:
+				return CertAlgoECDSA256v01
+			case 384:
+				return CertAlgoECDSA384v01
+			case 521:
+				return CertAlgoECDSA521v01
+			}
+		}
 	}
 	panic("unexpected key type")
 }

ssh/common_test.go

@@ -5,6 +5,11 @@
 package ssh
 
 import (
+	"crypto/dsa"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rsa"
+	"errors"
 	"testing"
 )
 
@@ -24,3 +29,49 @@ func TestSafeString(t *testing.T) {
 		}
 	}
 }
+
+func TestAlgoNameSupported(t *testing.T) {
+	supported := map[string]interface{}{
+		KeyAlgoRSA:          new(rsa.PublicKey),
+		KeyAlgoDSA:          new(dsa.PublicKey),
+		KeyAlgoECDSA256:     &ecdsa.PublicKey{Curve: elliptic.P256()},
+		KeyAlgoECDSA384:     &ecdsa.PublicKey{Curve: elliptic.P384()},
+		KeyAlgoECDSA521:     &ecdsa.PublicKey{Curve: elliptic.P521()},
+		CertAlgoRSAv01:      &OpenSSHCertV01{Key: new(rsa.PublicKey)},
+		CertAlgoDSAv01:      &OpenSSHCertV01{Key: new(dsa.PublicKey)},
+		CertAlgoECDSA256v01: &OpenSSHCertV01{Key: &ecdsa.PublicKey{Curve: elliptic.P256()}},
+		CertAlgoECDSA384v01: &OpenSSHCertV01{Key: &ecdsa.PublicKey{Curve: elliptic.P384()}},
+		CertAlgoECDSA521v01: &OpenSSHCertV01{Key: &ecdsa.PublicKey{Curve: elliptic.P521()}},
+	}
+
+	for expected, key := range supported {
+		actual := algoName(key)
+		if expected != actual {
+			t.Errorf("expected: %s, actual: %s", expected, actual)
+		}
+	}
+
+}
+
+func TestAlgoNameNotSupported(t *testing.T) {
+	notSupported := []interface{}{
+		&ecdsa.PublicKey{Curve: elliptic.P224()},
+		&OpenSSHCertV01{Key: &ecdsa.PublicKey{Curve: elliptic.P224()}},
+	}
+
+	panicTest := func(key interface{}) (algo string, err error) {
+		defer func() {
+			if r := recover(); r != nil {
+				err = errors.New(r.(string))
+			}
+		}()
+		algo = algoName(key)
+		return
+	}
+
+	for _, unsupportedKey := range notSupported {
+		if algo, err := panicTest(unsupportedKey); err == nil {
+			t.Errorf("Expected a panic, Got: %s (for type %T)", algo, unsupportedKey)
+		}
+	}
+}

ssh/keys.go

@@ -14,7 +14,8 @@ import (
 	"math/big"
 )
 
-// Key types supported by OpenSSH 5.9
+// These constants represent the algorithm names for key types supported by this
+// package.
 const (
 	KeyAlgoRSA      = "ssh-rsa"
 	KeyAlgoDSA      = "ssh-dss"
@@ -330,46 +331,7 @@ func ParsePublicKey(in []byte) (out interface{}, rest []byte, ok bool) {
 // in the sshd(8) manual page.
 func MarshalAuthorizedKey(key interface{}) []byte {
 	b := &bytes.Buffer{}
-	switch keyType := key.(type) {
-	case *rsa.PublicKey:
-		b.WriteString(KeyAlgoRSA)
-	case *dsa.PublicKey:
-		b.WriteString(KeyAlgoDSA)
-	case *ecdsa.PublicKey:
-		switch keyType.Params().BitSize {
-		case 256:
-			b.WriteString(KeyAlgoECDSA256)
-		case 384:
-			b.WriteString(KeyAlgoECDSA384)
-		case 521:
-			b.WriteString(KeyAlgoECDSA521)
-		default:
-			panic("unexpected key type")
-		}
-	case *OpenSSHCertV01:
-		switch keyType.Key.(type) {
-		case *rsa.PublicKey:
-			b.WriteString(CertAlgoRSAv01)
-		case *dsa.PublicKey:
-			b.WriteString(CertAlgoDSAv01)
-		case *ecdsa.PublicKey:
-			switch keyType.Key.(*ecdsa.PublicKey).Params().BitSize {
-			case 256:
-				b.WriteString(CertAlgoECDSA256v01)
-			case 384:
-				b.WriteString(CertAlgoECDSA384v01)
-			case 521:
-				b.WriteString(CertAlgoECDSA521v01)
-			default:
-				panic("unexpected key type")
-			}
-		default:
-			panic("unexpected key type")
-		}
-	default:
-		panic("unexpected key type")
-	}
-
+	b.WriteString(algoName(key))
 	b.WriteByte(' ')
 	e := base64.NewEncoder(base64.StdEncoding, b)
 	e.Write(serializePublickey(key))
@@ -378,10 +340,10 @@ func MarshalAuthorizedKey(key interface{}) []byte {
 	return b.Bytes()
 }
 
-// MarshalPublicKey serializes a *rsa.PublicKey, *dsa.PublicKey or
-// *OpenSSHCertV01 for use in the SSH wire protocol. It can be used for
-// comparison with the pubkey argument of ServerConfig's PublicKeyCallback as
-// well as for generating an authorized_keys or host_keys file.
+// MarshalPublicKey serializes a supported key or certificate for use by the
+// SSH wire protocol. It can be used for comparison with the pubkey argument
+// of ServerConfig's PublicKeyCallback as well as for generating an
+// authorized_keys or host_keys file.
 func MarshalPublicKey(key interface{}) []byte {
 	return serializePublickey(key)
 }