From 71e2725e11d87edc3f64ec3da7d11157616a7822 Mon Sep 17 00:00:00 2001 From: Dariusz Koryto Date: Mon, 28 Sep 2026 16:14:36 +0200 Subject: [PATCH] Recognize certificates held by ssh-agent Keys listed by the agent client come back as *agent.Key whatever their type, so the type assertion on *ssh.Certificate in loadIDs never matched. Certificate identities from the agent were treated as plain keys, and with IdentitiesOnly they were dropped because their fingerprint doesn't match the configured key. Connections to servers that require a certificate then failed with 'no key files found'. asCert now parses the key from its wire format before checking the type. certify uses it as well, for the same reason. Tested with a unit test for asCert, an e2e test that authenticates with a certificate only available through the agent, and manually with OpenSSH's ssh-agent against an sshd using TrustedUserCAKeys. --- internal/ssh_config/cert_test.go | 50 ++++++++++++++++++++++ internal/ssh_config/ssh_config.go | 16 ++++++- test/e2e/agent.go | 32 ++++++++++++-- test/e2e/agent_test.go | 29 +++++++++++++ test/testdata/config/ssh_config_agent_cert | 7 +++ 5 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 internal/ssh_config/cert_test.go create mode 100644 test/testdata/config/ssh_config_agent_cert diff --git a/internal/ssh_config/cert_test.go b/internal/ssh_config/cert_test.go new file mode 100644 index 0000000..e465883 --- /dev/null +++ b/internal/ssh_config/cert_test.go @@ -0,0 +1,50 @@ +package ssh_config + +import ( + "crypto/ed25519" + "crypto/rand" + "testing" + + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/agent" +) + +func newTestSigner(t *testing.T) ssh.Signer { + t.Helper() + _, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + s, err := ssh.NewSignerFromKey(priv) + if err != nil { + t.Fatal(err) + } + return s +} + +func TestAsCert(t *testing.T) { + s := newTestSigner(t) + c := &ssh.Certificate{ + Key: s.PublicKey(), + CertType: ssh.UserCert, + ValidBefore: ssh.CertTimeInfinity, + } + if err := c.SignCert(rand.Reader, newTestSigner(t)); err != nil { + t.Fatal(err) + } + + if got, ok := asCert(c); !ok || string(got.Marshal()) != string(c.Marshal()) { + t.Error("certificate not recognized") + } + // What the agent client returns for a certificate identity + wrapped := &agent.Key{Format: c.Type(), Blob: c.Marshal()} + if got, ok := asCert(wrapped); !ok || string(got.Marshal()) != string(c.Marshal()) { + t.Error("certificate from agent not recognized") + } + if _, ok := asCert(s.PublicKey()); ok { + t.Error("plain key taken for a certificate") + } + if _, ok := asCert(dummyKey{}); ok { + t.Error("unparsable key taken for a certificate") + } +} diff --git a/internal/ssh_config/ssh_config.go b/internal/ssh_config/ssh_config.go index 73c68b2..e1ab6af 100644 --- a/internal/ssh_config/ssh_config.go +++ b/internal/ssh_config/ssh_config.go @@ -260,7 +260,7 @@ func (sc *SSHConfig) loadIDs() (fileIDs, agentCertIDs, agentCfgIDs, agentOtherID } else { for _, s := range agSigs { // Agent may return certificate identities (public key is a cert) - if c, ok := s.PublicKey().(*ssh.Certificate); ok { + if c, ok := asCert(s.PublicKey()); ok { fp := keyFP(c.Key) if _, ok := cfgFP[fp]; ok || !sc.IdentitiesOnly { agentCertIDs = append(agentCertIDs, identity{signer: s}) @@ -482,8 +482,20 @@ func loadCert(path string) (*ssh.Certificate, error) { return cert, nil } +// asCert returns the certificate behind pub, if it is one. Keys listed by +// the agent come as *agent.Key regardless of their type, so a plain type +// assertion is not enough; the key is parsed from its wire format instead. +func asCert(pub ssh.PublicKey) (*ssh.Certificate, bool) { + parsed, err := ssh.ParsePublicKey(pub.Marshal()) + if err != nil { + return nil, false + } + c, ok := parsed.(*ssh.Certificate) + return c, ok +} + func certify(cert *ssh.Certificate, sig ssh.Signer) (ssh.Signer, error) { - if _, ok := sig.PublicKey().(*ssh.Certificate); ok { + if _, ok := asCert(sig.PublicKey()); ok { return nil, fmt.Errorf("signer is already a certificate identity") } if keyFP(sig.PublicKey()) != keyFP(cert.Key) { diff --git a/test/e2e/agent.go b/test/e2e/agent.go index ab53301..3231c15 100644 --- a/test/e2e/agent.go +++ b/test/e2e/agent.go @@ -2,6 +2,7 @@ package e2e import ( "context" + "fmt" "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/agent" "net" @@ -14,6 +15,12 @@ const ( ) func startAgent(sock string) (context.CancelFunc, error) { + return startAgentWithCert(sock, "") +} + +// startAgentWithCert starts an agent holding the client key. If certFile is +// set, the key is added together with that certificate. +func startAgentWithCert(sock, certFile string) (context.CancelFunc, error) { // Read and parse the private key keyBytes, err := os.ReadFile(clientKeyFile) if err != nil { @@ -24,12 +31,29 @@ func startAgent(sock string) (context.CancelFunc, error) { return nil, err } - // Create agent and add the key - kr := agent.NewKeyring() - if err := kr.Add(agent.AddedKey{ + key := agent.AddedKey{ PrivateKey: signer, Comment: filepath.Base(clientKeyFile), - }); err != nil { + } + if certFile != "" { + certBytes, err := os.ReadFile(certFile) + if err != nil { + return nil, err + } + pub, _, _, _, err := ssh.ParseAuthorizedKey(certBytes) + if err != nil { + return nil, err + } + cert, ok := pub.(*ssh.Certificate) + if !ok { + return nil, fmt.Errorf("%s is not a certificate", certFile) + } + key.Certificate = cert + } + + // Create agent and add the key + kr := agent.NewKeyring() + if err := kr.Add(key); err != nil { return nil, err } diff --git a/test/e2e/agent_test.go b/test/e2e/agent_test.go index b48af8b..0c7d77c 100644 --- a/test/e2e/agent_test.go +++ b/test/e2e/agent_test.go @@ -83,3 +83,32 @@ func TestAgentIdsOnly(t *testing.T) { t.Fatalf("exit code %d: %s", c, out) } } + +// The server requires a certificate for this user, and the certificate is +// only available through the agent. With IdentitiesOnly and just the public +// key configured, boring has to pick the agent's certificate identity. +func TestAgentCertIdsOnly(t *testing.T) { + cfg := defaultConfig + cfg.sshConfig = "../testdata/config/ssh_config_agent_cert" + cfg.useAgent = true + env, cancel, err := makeEnvWithDaemon(cfg, t) + if err != nil { + t.Fatalf("%v", err.Error()) + } + defer cancel() + + cancel, err = startAgentWithCert(getEnv(env, "SSH_AUTH_SOCK"), "../testdata/keys/cert.pub") + if err != nil { + t.Fatalf("could not start agent: %v", err) + } + defer cancel() + + c, out, err := cliCommand(env, "open", "test") + if err != nil { + t.Fatalf("failed to run CLI command: %v", err) + } + if c != 0 { + t.Fatalf("exit code %d: %s", c, out) + } + testTunnel(t, "localhost:49711", "localhost:49712") +} diff --git a/test/testdata/config/ssh_config_agent_cert b/test/testdata/config/ssh_config_agent_cert new file mode 100644 index 0000000..7767673 --- /dev/null +++ b/test/testdata/config/ssh_config_agent_cert @@ -0,0 +1,7 @@ +Host * + User needs-cert + Port 58391 + # Only the public key is configured, the certificate lives in the agent + IdentityFile ../testdata/keys/client.pub + IdentitiesOnly yes + UserKnownHostsFile ../testdata/known_hosts/known_hosts