From 21102e65cd2264eb4a181e759a43362228b86b22 Mon Sep 17 00:00:00 2001 From: Hayden B Date: Tue, 26 Sep 2023 05:29:57 -0700 Subject: [PATCH] Fix panic when parsing SSH SK pubkeys (#1712) This is because these pubkey types do not implement ssh.CryptoPublicKey, which causes a panic when we try to do a type assertion. Also realized we weren't handling SSH certs, so now we extract the pubkey from the cert before trying to parse it. Had to reimplement parsing the SK pubkeys because there is no other way to extract the raw pubkey from it. After https://github.com/golang/go/issues/62518, this will get cleaned up. Signed-off-by: Hayden Blauzvern --- pkg/pki/ssh/ssh.go | 92 ++++++++++++++++++++++++++++++++++++++++- pkg/pki/ssh/ssh_test.go | 26 ++++++++++++ 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/pkg/pki/ssh/ssh.go b/pkg/pki/ssh/ssh.go index f2caa835f..d96a83343 100644 --- a/pkg/pki/ssh/ssh.go +++ b/pkg/pki/ssh/ssh.go @@ -16,6 +16,12 @@ package ssh import ( + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "encoding/binary" + "errors" "fmt" "io" "net/http" @@ -122,8 +128,73 @@ func (k PublicKey) Subjects() []string { // Identities implements the pki.PublicKey interface func (k PublicKey) Identities() ([]identity.Identity, error) { - key := k.key.(ssh.CryptoPublicKey).CryptoPublicKey() - pkixKey, err := cryptoutils.MarshalPublicKeyToDER(key) + // extract key from SSH certificate if present + var sshKey ssh.PublicKey + switch v := k.key.(type) { + case *ssh.Certificate: + sshKey = v.Key + default: + sshKey = k.key + } + + // Extract crypto.PublicKey from SSH key + // Handle sk public keys which do not implement ssh.CryptoPublicKey + // Inspired by x/ssh/keys.go + // TODO: Simplify after https://github.com/golang/go/issues/62518 + var cryptoPubKey crypto.PublicKey + if v, ok := sshKey.(ssh.CryptoPublicKey); ok { + cryptoPubKey = v.CryptoPublicKey() + } else { + switch sshKey.Type() { + case ssh.KeyAlgoSKECDSA256: + var w struct { + Curve string + KeyBytes []byte + Application string + Rest []byte `ssh:"rest"` + } + _, k, ok := parseString(sshKey.Marshal()) + if !ok { + return nil, fmt.Errorf("error parsing ssh.KeyAlgoSKED25519 key") + } + if err := ssh.Unmarshal(k, &w); err != nil { + return nil, err + } + if w.Curve != "nistp256" { + return nil, errors.New("ssh: unsupported curve") + } + ecdsaPubKey := new(ecdsa.PublicKey) + ecdsaPubKey.Curve = elliptic.P256() + //nolint:staticcheck // ignore SA1019 for old code + ecdsaPubKey.X, ecdsaPubKey.Y = elliptic.Unmarshal(ecdsaPubKey.Curve, w.KeyBytes) + if ecdsaPubKey.X == nil || ecdsaPubKey.Y == nil { + return nil, errors.New("ssh: invalid curve point") + } + cryptoPubKey = ecdsaPubKey + case ssh.KeyAlgoSKED25519: + var w struct { + KeyBytes []byte + Application string + Rest []byte `ssh:"rest"` + } + _, k, ok := parseString(sshKey.Marshal()) + if !ok { + return nil, fmt.Errorf("error parsing ssh.KeyAlgoSKED25519 key") + } + if err := ssh.Unmarshal(k, &w); err != nil { + return nil, err + } + if l := len(w.KeyBytes); l != ed25519.PublicKeySize { + return nil, fmt.Errorf("invalid size %d for Ed25519 public key", l) + } + cryptoPubKey = ed25519.PublicKey(w.KeyBytes) + default: + // Should not occur, as rsa, dsa, ecdsa, and ed25519 all implement ssh.CryptoPublicKey + return nil, fmt.Errorf("unknown key type: %T", sshKey) + } + } + + pkixKey, err := cryptoutils.MarshalPublicKeyToDER(cryptoPubKey) if err != nil { return nil, err } @@ -134,3 +205,20 @@ func (k PublicKey) Identities() ([]identity.Identity, error) { Fingerprint: fp, }}, nil } + +// Copied by x/ssh/keys.go +// TODO: Remove after https://github.com/golang/go/issues/62518 +func parseString(in []byte) (out, rest []byte, ok bool) { + if len(in) < 4 { + return + } + length := binary.BigEndian.Uint32(in) + in = in[4:] + if uint32(len(in)) < length { + return + } + out = in[:length] + rest = in[length:] + ok = true + return +} diff --git a/pkg/pki/ssh/ssh_test.go b/pkg/pki/ssh/ssh_test.go index f06d1e3fd..56d02ce43 100644 --- a/pkg/pki/ssh/ssh_test.go +++ b/pkg/pki/ssh/ssh_test.go @@ -67,6 +67,32 @@ func TestIdentities(t *testing.T) { } } +func TestIdentitiesParsesAllKeyTypes(t *testing.T) { + for _, k := range []string{ + // DSA is not supported + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDXofkiahE7uavjWvxnwkUF27qMgz7pdTwzSv0XzVG6EtirOv3PDWct4YKoXE9c0EqbxnIfYEKwEextdvB7zkgwczdJSHxf/18jQumLn/FuoCmugVSk1H5Qli/qzwBpaTnOk3WuakGuoYUl8ZAokKKgOKLA0aZJ1WRQ2ZCZggA3EkwNZiY17y9Q6HqdgQcH6XN8aAMADNVJdMAJb33hSRJjjsAPTmzBTishP8lYDoGRSsSE7/8XRBCEV5E4I8mI9GElcZwV/1KJx98mpH8QvMzXM1idFcwPRtt1NTAOshwgUU0Fu1x8lU5RQIa6ZKW36qNQLvLxy/BscC7B/mdLptoDs/ot9NimUXZcgCR1a2Q3o7Wi6jIgcgJcyV10Nba81ol4RdN4qPHnVZIzuo+dBkqwG3CMtB4Rj84+Qi+7zyU01hIPreoxQDXaayiGPBUUIiAlW9gsiuRWJzNnu3cvuWDLVfQIkjh7Wug58z+v2NOJ7IMdyERillhzDcvVHaq14+U= test@rekor.dev", + "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNLCu01+wpXe3xB5olXCN4SqU2rQu0qjSRKJO4Bg+JRCPU+ENcgdA5srTU8xYDz/GEa4dzK5ldPw4J/gZgSXCMs=", + "ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBLSsu/HNKiaALhQ26UDv+N0AFdMb26fMVrOKe866CGu6ajSf9HUOhJFdjhseihB2rTalMPr8MrcXNLufii4mL8u4l9fUQXFgwnM/ZpiVPSs6C+8i4u/ZDg7Nx2NXybNIgQ==", + "ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACB4WgRgGBRo6Uk+cRgg8tJPCbEtGURRWlUA7PDDerXR+P9O6mm3L99Etxsyh5XNYqXyaMNtH5c51ooMajrFwcayAHIhPPb8X3CsTwEfIUQ96aDyHQMotbRfnkn6uefeUTRrSNcqeAndUtVyAqBdqbsq2mgJYXHrz2NUKlPFYgauQi+WQ==", + "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIXffBYeYL+WVzVru8npl5JHt2cjlr4ornFTWzoij9sx", + "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGRNqlFgED/pf4zXz8IzqA6CALNwYcwgd4MQDmIS1GOtn1SySFObiuyJaOlpqkV5FeEifhxfIC2ejKKtNyO4CysAAAAEc3NoOg== user@host", + "sk-ssh-ed25519@openssh.com AAAAGnNrLXNzaC1lZDI1NTE5QG9wZW5zc2guY29tAAAAIJjzc2a20RjCvN/0ibH6UpGuN9F9hDvD7x182bOesNhHAAAABHNzaDo= user@host", + "ssh-rsa-cert-v01@openssh.com AAAAHHNzaC1yc2EtY2VydC12MDFAb3BlbnNzaC5jb20AAAAgb1srW/W3ZDjYAO45xLYAwzHBDLsJ4Ux6ICFIkTjb1LEAAAADAQABAAAAYQCkoR51poH0wE8w72cqSB8Sszx+vAhzcMdCO0wqHTj7UNENHWEXGrU0E0UQekD7U+yhkhtoyjbPOVIP7hNa6aRk/ezdh/iUnCIt4Jt1v3Z1h1P+hA4QuYFMHNB+rmjPwAcAAAAAAAAAAAAAAAEAAAAEdGVzdAAAAAAAAAAAAAAAAP//////////AAAAAAAAAIIAAAAVcGVybWl0LVgxMS1mb3J3YXJkaW5nAAAAAAAAABdwZXJtaXQtYWdlbnQtZm9yd2FyZGluZwAAAAAAAAAWcGVybWl0LXBvcnQtZm9yd2FyZGluZwAAAAAAAAAKcGVybWl0LXB0eQAAAAAAAAAOcGVybWl0LXVzZXItcmMAAAAAAAAAAAAAAHcAAAAHc3NoLXJzYQAAAAMBAAEAAABhANFS2kaktpSGc+CcmEKPyw9mJC4nZKxHKTgLVZeaGbFZOvJTNzBspQHdy7Q1uKSfktxpgjZnksiu/tFF9ngyY2KFoc+U88ya95IZUycBGCUbBQ8+bhDtw/icdDGQD5WnUwAAAG8AAAAHc3NoLXJzYQAAAGC8Y9Z2LQKhIhxf52773XaWrXdxP0t3GBVo4A10vUWiYoAGepr6rQIoGGXFxT4B9Gp+nEBJjOwKDXPrAevow0T9ca8gZN+0ykbhSrXLE5Ao48rqr3zP4O1/9P7e6gp0gw8=", + } { + pub, err := NewPublicKey(strings.NewReader(k)) + if err != nil { + t.Fatal(err) + } + ids, err := pub.Identities() + if err != nil { + t.Fatal(err) + } + if len(ids) == 0 { + t.Fatal("expected identities to be found") + } + } +} + func randomSuffix(n int) string { const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"