From b0d50cfcf2f48219a26d994f1c123dc994287fd1 Mon Sep 17 00:00:00 2001 From: Nicola Date: Sun, 8 Feb 2026 15:28:56 +0100 Subject: [PATCH] ssh/agent: reject keys with unsupported confirm constraint The in-memory keyring supports the "lifetime" constraint but does not implement the "confirm" constraint. Previously, keyring.Add silently ignored ConfirmBeforeUse: the key was stored, advertised through List, and used for signing without any interactive confirmation, potentially misleading callers into believing this security measure was enforced. Return an error when ConfirmBeforeUse is set instead of silently downgrading the caller's security expectations. Implementing real confirm-before-use in an in-memory library keyring is infeasible (there is no UI or confirmation callback), so failing closed is the correct behavior; adding actual confirm support would require an API addition and is out of scope. This is a deliberate behavior change: keyring.Add previously accepted and ignored ConfirmBeforeUse and now returns an error. This change also updates the keyring doc comments to document the supported constraints. This issue was found during a security audit by NCC Group Cryptography Services, sponsored by Teleport. Fixes CVE-2026-39833 Updates golang/go#47533 Fixes golang/go#79436 Change-Id: I1b3a286f0c1e4a4e08ac37109f7e491692ca90ae Reviewed-on: https://go-review.googlesource.com/c/crypto/+/778642 Reviewed-by: Dmitri Shuralyov Reviewed-by: Neal Patel Reviewed-by: Neal Patel Auto-Submit: Neal Patel LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com --- ssh/agent/keyring.go | 6 ++++++ ssh/agent/keyring_test.go | 13 +++++++++++++ ssh/test/agent_unix_test.go | 5 ++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/ssh/agent/keyring.go b/ssh/agent/keyring.go index c1b4361087..41509c0d66 100644 --- a/ssh/agent/keyring.go +++ b/ssh/agent/keyring.go @@ -34,6 +34,8 @@ var errLocked = errors.New("agent: locked") // NewKeyring returns an Agent that holds keys in memory. It is safe // for concurrent use by multiple goroutines. +// +// The returned Agent only supports the "lifetime" constraint. func NewKeyring() Agent { return &keyring{} } @@ -146,12 +148,16 @@ func (r *keyring) List() ([]*Key, error) { // Insert adds a private key to the keyring. If a certificate // is given, that certificate is added as public key. Note that // any constraints given are ignored. +// Add returns an error if key contains ConfirmBeforeUse func (r *keyring) Add(key AddedKey) error { r.mu.Lock() defer r.mu.Unlock() if r.locked { return errLocked } + if key.ConfirmBeforeUse { + return errors.New("agent: confirm before use constraint is not supported") + } signer, err := ssh.NewSignerFromKey(key.PrivateKey) if err != nil { diff --git a/ssh/agent/keyring_test.go b/ssh/agent/keyring_test.go index e9c90a3131..cd6aed29df 100644 --- a/ssh/agent/keyring_test.go +++ b/ssh/agent/keyring_test.go @@ -120,3 +120,16 @@ func TestAddDuplicateKey(t *testing.T) { t.Fatal("key with the updated comment not found") } } + +func TestAddKeyWithConfirmBeforeUse(t *testing.T) { + agent, cleanup := startKeyringAgent(t) + defer cleanup() + key := testPrivateKeys["rsa"] + err := agent.Add(AddedKey{ + PrivateKey: key, + ConfirmBeforeUse: true, + }) + if err == nil { + t.Fatal("adding a key with confirm before use constraint succeeded") + } +} diff --git a/ssh/test/agent_unix_test.go b/ssh/test/agent_unix_test.go index 9257bfe1bc..fb99c9073b 100644 --- a/ssh/test/agent_unix_test.go +++ b/ssh/test/agent_unix_test.go @@ -24,9 +24,8 @@ func TestAgentForward(t *testing.T) { t.Fatalf("Error adding key: %s", err) } if err := keyring.Add(agent.AddedKey{ - PrivateKey: testPrivateKeys["ecdsa"], - ConfirmBeforeUse: true, - LifetimeSecs: 3600, + PrivateKey: testPrivateKeys["ecdsa"], + LifetimeSecs: 3600, }); err != nil { t.Fatalf("Error adding key with constraints: %s", err) }