Commit 8f5916cf authored by JP Sugarbroad's avatar JP Sugarbroad

go.crypto/ssh: fix certificate parsing/marshaling.

The change to add the PublicKey interface accidentally caused certificate handling to expect an extra copy of the private key algorithm name in the binary representation. This change adapts a suitable parsing API and adds a test to ensure that cert handling isn't easily broken in the future.

R=agl, hanwen, jmpittman
CC=golang-dev
https://codereview.appspot.com/13272055

Committer: Adam Langley <agl@golang.org>
parent 9140d54a
......@@ -100,7 +100,7 @@ func (ak *AgentKey) String() string {
// Key returns an agent's public key as one of the supported key or certificate types.
func (ak *AgentKey) Key() (PublicKey, error) {
if key, _, ok := parsePubKey(ak.blob); ok {
if key, _, ok := ParsePublicKey(ak.blob); ok {
return key, nil
}
return nil, errors.New("ssh: failed to parse key blob")
......
......@@ -60,6 +60,17 @@ var certAlgoNames = map[string]string{
KeyAlgoECDSA521: CertAlgoECDSA521v01,
}
// certToPrivAlgo returns the underlying algorithm for a certificate algorithm.
// Panics if a non-certificate algorithm is passed.
func certToPrivAlgo(algo string) string {
for privAlgo, pubAlgo := range certAlgoNames {
if pubAlgo == algo {
return privAlgo
}
}
panic("unknown cert algorithm")
}
func (c *OpenSSHCertV01) PublicKeyAlgo() string {
algo, ok := certAlgoNames[c.Key.PublicKeyAlgo()]
if !ok {
......@@ -83,12 +94,14 @@ func parseOpenSSHCertV01(in []byte, algo string) (out *OpenSSHCertV01, rest []by
return
}
cert.Key, in, ok = ParsePublicKey(in)
privAlgo := certToPrivAlgo(algo)
cert.Key, in, ok = parsePubKey(in, privAlgo)
if !ok {
return
}
if cert.Key.PrivateKeyAlgo() != algo {
// We test PublicKeyAlgo to make sure we don't use some weird sub-cert.
if cert.Key.PublicKeyAlgo() != privAlgo {
ok = false
return
}
......@@ -139,7 +152,7 @@ func parseOpenSSHCertV01(in []byte, algo string) (out *OpenSSHCertV01, rest []by
if !ok {
return
}
if cert.SignatureKey, _, ok = parsePubKey(sigKey); !ok {
if cert.SignatureKey, _, ok = ParsePublicKey(sigKey); !ok {
return
}
......@@ -152,8 +165,7 @@ func parseOpenSSHCertV01(in []byte, algo string) (out *OpenSSHCertV01, rest []by
}
func (cert *OpenSSHCertV01) Marshal() []byte {
pubKey := MarshalPublicKey(cert.Key)
pubKey := cert.Key.Marshal()
sigKey := MarshalPublicKey(cert.SignatureKey)
length := stringLength(len(cert.Nonce))
......
......@@ -201,7 +201,7 @@ func serializeSignature(name string, sig []byte) []byte {
// generating an authorized_keys or host_keys file.
func MarshalPublicKey(key PublicKey) []byte {
// See also RFC 4253 6.6.
algoname := key.PrivateKeyAlgo()
algoname := key.PublicKeyAlgo()
blob := key.Marshal()
length := stringLength(len(algoname))
......
......@@ -31,14 +31,10 @@ const (
KeyAlgoECDSA521 = "ecdsa-sha2-nistp521"
)
// parsePubKey parses a public key according to RFC 4253, section 6.6.
func parsePubKey(in []byte) (pubKey PublicKey, rest []byte, ok bool) {
algo, in, ok := parseString(in)
if !ok {
return
}
switch string(algo) {
// parsePubKey parses a public key of the given algorithm.
// Use ParsePublicKey for keys with prepended algorithm.
func parsePubKey(in []byte, algo string) (pubKey PublicKey, rest []byte, ok bool) {
switch algo {
case KeyAlgoRSA:
return parseRSA(in)
case KeyAlgoDSA:
......@@ -46,7 +42,7 @@ func parsePubKey(in []byte) (pubKey PublicKey, rest []byte, ok bool) {
case KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521:
return parseECDSA(in)
case CertAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01, CertAlgoECDSA384v01, CertAlgoECDSA521v01:
return parseOpenSSHCertV01(in, string(algo))
return parseOpenSSHCertV01(in, algo)
}
return nil, nil, false
}
......@@ -54,7 +50,7 @@ func parsePubKey(in []byte) (pubKey PublicKey, rest []byte, ok bool) {
// parseAuthorizedKey parses a public key in OpenSSH authorized_keys format
// (see sshd(8) manual page) once the options and key type fields have been
// removed.
func parseAuthorizedKey(in []byte) (out interface{}, comment string, ok bool) {
func parseAuthorizedKey(in []byte) (out PublicKey, comment string, ok bool) {
in = bytes.TrimSpace(in)
i := bytes.IndexAny(in, " \t")
......@@ -69,7 +65,7 @@ func parseAuthorizedKey(in []byte) (out interface{}, comment string, ok bool) {
return
}
key = key[:n]
out, _, ok = parsePubKey(key)
out, _, ok = ParsePublicKey(key)
if !ok {
return nil, "", false
}
......@@ -79,7 +75,7 @@ func parseAuthorizedKey(in []byte) (out interface{}, comment string, ok bool) {
// ParseAuthorizedKeys parses a public key from an authorized_keys
// file used in OpenSSH according to the sshd(8) manual page.
func ParseAuthorizedKey(in []byte) (out interface{}, comment string, options []string, rest []byte, ok bool) {
func ParseAuthorizedKey(in []byte) (out PublicKey, comment string, options []string, rest []byte, ok bool) {
for len(in) > 0 {
end := bytes.IndexByte(in, '\n')
if end != -1 {
......@@ -179,9 +175,14 @@ func ParseAuthorizedKey(in []byte) (out interface{}, comment string, options []s
}
// ParsePublicKey parses an SSH public key formatted for use in
// the SSH wire protocol.
// the SSH wire protocol according to RFC 4253, section 6.6.
func ParsePublicKey(in []byte) (out PublicKey, rest []byte, ok bool) {
return parsePubKey(in)
algo, in, ok := parseString(in)
if !ok {
return
}
return parsePubKey(in, string(algo))
}
// MarshalAuthorizedKey returns a byte stream suitable for inclusion
......
package ssh
import (
"bytes"
"crypto/dsa"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"crypto/rsa"
"encoding/base64"
"reflect"
"strings"
"testing"
......@@ -163,6 +165,33 @@ func TestParseDSA(t *testing.T) {
}
}
func TestParseCert(t *testing.T) {
// Cert generated by ssh-keygen 6.0p1 Debian-4.
// % ssh-keygen -s ca-key -I test user-key
b64data := "AAAAHHNzaC1yc2EtY2VydC12MDFAb3BlbnNzaC5jb20AAAAgb1srW/W3ZDjYAO45xLYAwzHBDLsJ4Ux6ICFIkTjb1LEAAAADAQABAAAAYQCkoR51poH0wE8w72cqSB8Sszx+vAhzcMdCO0wqHTj7UNENHWEXGrU0E0UQekD7U+yhkhtoyjbPOVIP7hNa6aRk/ezdh/iUnCIt4Jt1v3Z1h1P+hA4QuYFMHNB+rmjPwAcAAAAAAAAAAAAAAAEAAAAEdGVzdAAAAAAAAAAAAAAAAP//////////AAAAAAAAAIIAAAAVcGVybWl0LVgxMS1mb3J3YXJkaW5nAAAAAAAAABdwZXJtaXQtYWdlbnQtZm9yd2FyZGluZwAAAAAAAAAWcGVybWl0LXBvcnQtZm9yd2FyZGluZwAAAAAAAAAKcGVybWl0LXB0eQAAAAAAAAAOcGVybWl0LXVzZXItcmMAAAAAAAAAAAAAAHcAAAAHc3NoLXJzYQAAAAMBAAEAAABhANFS2kaktpSGc+CcmEKPyw9mJC4nZKxHKTgLVZeaGbFZOvJTNzBspQHdy7Q1uKSfktxpgjZnksiu/tFF9ngyY2KFoc+U88ya95IZUycBGCUbBQ8+bhDtw/icdDGQD5WnUwAAAG8AAAAHc3NoLXJzYQAAAGC8Y9Z2LQKhIhxf52773XaWrXdxP0t3GBVo4A10vUWiYoAGepr6rQIoGGXFxT4B9Gp+nEBJjOwKDXPrAevow0T9ca8gZN+0ykbhSrXLE5Ao48rqr3zP4O1/9P7e6gp0gw8="
data, err := base64.StdEncoding.DecodeString(b64data)
if err != nil {
t.Fatal("base64.StdEncoding.DecodeString: ", err)
}
key, rest, ok := ParsePublicKey(data)
if !ok {
t.Fatalf("could not parse certificate")
}
if len(rest) > 0 {
t.Errorf("rest: got %q, want empty", rest)
}
_, ok = key.(*OpenSSHCertV01)
if !ok {
t.Fatalf("got %#v, want *OpenSSHCertV01", key)
}
marshaled := MarshalPublicKey(key)
if !bytes.Equal(data, marshaled) {
t.Errorf("marshaled certificate does not match original: got %q, want %q", marshaled, data)
}
}
func init() {
raw, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
ecdsaKey, _ = NewSignerFromKey(raw)
......
......@@ -406,7 +406,7 @@ userAuthLoop:
break
}
signedData := buildDataSignedForAuth(H, userAuthReq, algoBytes, pubKey)
key, _, ok := parsePubKey(pubKey)
key, _, ok := ParsePublicKey(pubKey)
if !ok {
return ParseError{msgUserAuthRequest}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment