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