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 <dmitshur@google.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Neal Patel <neal@golang.org>
Auto-Submit: Neal Patel <nealpatel@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Nicola
2026-05-21 10:54:57 -07:00
committed by Neal Patel
parent e3d1254f1e
commit 0fb843a472
3 changed files with 27 additions and 6 deletions
+10 -3
View File
@@ -32,8 +32,10 @@ type keyring struct {
var errLocked = errors.New("agent: locked")
// NewKeyring returns an Agent that holds keys in memory. It is safe
// for concurrent use by multiple goroutines.
// 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,7 +148,8 @@ func (r *keyring) List() ([]*Key, error) {
// Add adds a private key to the keyring. If a certificate is given, that
// certificate is added as public key.
//
// Add returns an error if key contains ConstraintExtensions.
// Add returns an error if key contains ConstraintExtensions or
// ConfirmBeforeUse.
func (r *keyring) Add(key AddedKey) error {
r.mu.Lock()
defer r.mu.Unlock()
@@ -154,6 +157,10 @@ func (r *keyring) Add(key AddedKey) error {
return errLocked
}
if key.ConfirmBeforeUse {
return errors.New("agent: confirm before use constraint is not supported")
}
if len(key.ConstraintExtensions) > 0 {
return errors.New("agent: constraint extensions are present but not supported")
}
+15
View File
@@ -144,3 +144,18 @@ func TestAddKeyWithConstraints(t *testing.T) {
t.Fatal("adding a key with unsupported constraints succeeded")
}
}
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")
}
}
+2 -3
View File
@@ -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)
}