Skip to content

Commit b61cf85

Browse files
drakkanthatnealpatel
authored andcommitted
ssh: enforce user presence verification for security keys
Previously the library did not verify the "User Presence" (UP) bit in signatures generated by FIDO/U2F security keys (sk-ecdsa-sha2-nistp256@openssh.com and sk-ssh-ed25519@openssh.com). This allowed signatures without physical interaction to be accepted if the underlying hardware produced them, deviating from the default secure behavior expected by the FIDO standards and OpenSSH. skECDSAPublicKey.Verify and skEd25519PublicKey.Verify now enforce the user-presence bit (0x01, constant flagUserPresence) by default. Signatures whose flags byte has UP clear fail with the sentinel errSKMissingUserPresence. The server public-key authentication path honors the OpenSSH "no-touch-required" extension as an opt-out. noTouchAllowed reports true when the extension is present either in the Permissions returned by PublicKeyCallback (authorized_keys-level opt-out) or in the certificate's own Extensions (CA-level opt-out); in that case skKeyWithoutUP is used to derive a clone of the SK public key (and, for certificates, a clone of the wrapping Certificate whose inner Key is the cloned SK key) whose Verify accepts UP-clear signatures. The originals are never mutated, so a per-session opt-out cannot leak across authentication attempts or connections. Matching OpenSSH, the opt-out is read only from Extensions, never from CriticalOptions. skKeyWithoutUP is iterative and unwraps at most one level of *Certificate: the SSH cert format forbids Certificate.Key from being another Certificate (parseCert rejects it) but callers can still construct such a value directly in Go, so a recursive descent would be driven to unbounded depth by malformed or cyclic input. Any such pathological *Certificate is returned unchanged. CertChecker.CheckCert applies skKeyWithoutUP unconditionally to the certificate's CA key before verifying the CA signature, matching OpenSSH, which calls sshkey_verify with detailsp==NULL in sshkey.c:cert_parse and never extracts or enforces UP/UV flags on CA signatures. The UP bit on a CA signature reflects the CA operator's presence at cert-issuance time, which has no bearing on whether the user being authenticated is present now, so enforcing it here would only break interop with certificates issued by non-interactive SK CAs without a corresponding security benefit. The skKeyWithoutUP call is a no-op for non-SK CA keys (the common case). This change breaks backward compatibility for clients or keys that generate user-authentication signatures without the User Presence flag set. Previously those signatures were accepted by the server. They will now be rejected with "ssh: signature missing required user presence flag" unless the "no-touch-required" extension is explicitly granted to the session by the server callbacks, or carried by the user certificate. This issue was found during a security audit by NCC Group Cryptography Services, sponsored by Teleport. Fixes golang/go#79566 Fixes CVE-2026-39831 Change-Id: I74b6de3bb6a2d7a0f34d7fa36bbbbf06f0b3fc6b Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781662 Reviewed-by: Neal Patel <nealpatel@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 9c2cd33 commit b61cf85

6 files changed

Lines changed: 651 additions & 12 deletions

File tree

ssh/certs.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,17 @@ func (c *CertChecker) CheckCert(principal string, cert *Certificate) error {
444444
if before := int64(cert.ValidBefore); cert.ValidBefore != uint64(CertTimeInfinity) && (unixNow >= before || before < 0) {
445445
return fmt.Errorf("ssh: cert has expired")
446446
}
447-
if err := cert.SignatureKey.Verify(cert.bytesForSigning(), cert.Signature); err != nil {
447+
// Match OpenSSH: the SK user-presence flag is never enforced on a
448+
// certificate's CA signature. OpenSSH calls sshkey_verify with
449+
// detailsp==NULL in sshkey.c:cert_parse, so the UP/UV flags are
450+
// not even extracted. The UP bit on a CA signature reflects the
451+
// CA operator's presence at signing time, which has no bearing on
452+
// whether the user being authenticated is present now; enforcing
453+
// it here would only break interop with certificates issued by
454+
// non-interactive SK CAs. skKeyWithoutUP is a no-op for non-SK
455+
// keys (the common case).
456+
caKey := skKeyWithoutUP(cert.SignatureKey)
457+
if err := caKey.Verify(cert.bytesForSigning(), cert.Signature); err != nil {
448458
return fmt.Errorf("ssh: certificate signature does not verify")
449459
}
450460

ssh/certs_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import (
99
"crypto/ecdsa"
1010
"crypto/elliptic"
1111
"crypto/rand"
12+
"crypto/sha256"
1213
"fmt"
1314
"io"
15+
"math/big"
1416
"net"
1517
"reflect"
1618
"testing"
@@ -128,6 +130,103 @@ func TestValidateCert(t *testing.T) {
128130
}
129131
}
130132

133+
// signSKCert builds an SK-ECDSA signature over cert.bytesForSigning()
134+
// using caKey as the simulated hardware token. The SK signing flags
135+
// byte (UP bit and others) is caller-controlled so tests can exercise
136+
// the user-presence enforcement paths. caApp is the SK application
137+
// string (typically "ssh:").
138+
func signSKCert(t *testing.T, cert *Certificate, caKey *ecdsa.PrivateKey, caApp string, flags byte) {
139+
t.Helper()
140+
cert.SignatureKey = &skECDSAPublicKey{
141+
application: caApp,
142+
PublicKey: caKey.PublicKey,
143+
}
144+
cert.Nonce = make([]byte, 32)
145+
if _, err := rand.Read(cert.Nonce); err != nil {
146+
t.Fatal(err)
147+
}
148+
149+
h := sha256.New()
150+
h.Write([]byte(caApp))
151+
appDigest := h.Sum(nil)
152+
h.Reset()
153+
h.Write(cert.bytesForSigning())
154+
dataDigest := h.Sum(nil)
155+
156+
var counter uint32 = 1
157+
blob := struct {
158+
ApplicationDigest []byte `ssh:"rest"`
159+
Flags byte
160+
Counter uint32
161+
MessageDigest []byte `ssh:"rest"`
162+
}{appDigest, flags, counter, dataDigest}
163+
h.Reset()
164+
h.Write(Marshal(blob))
165+
digest := h.Sum(nil)
166+
167+
r, s, err := ecdsa.Sign(rand.Reader, caKey, digest)
168+
if err != nil {
169+
t.Fatal(err)
170+
}
171+
cert.Signature = &Signature{
172+
Format: KeyAlgoSKECDSA256,
173+
Blob: Marshal(struct{ R, S *big.Int }{r, s}),
174+
Rest: Marshal(struct {
175+
Flags byte
176+
Counter uint32
177+
}{flags, counter}),
178+
}
179+
}
180+
181+
// TestCheckCertSKAuthorityNoUPRequired pins the OpenSSH-parity
182+
// behavior of CertChecker.CheckCert for SK CA signatures: a
183+
// certificate signed by an SK CA that produced a UP=0 signature is
184+
// accepted, because the UP bit on a CA signature has no bearing on
185+
// the current user authentication (see sshkey.c:cert_parse, which
186+
// passes detailsp==NULL to sshkey_verify). Without this, certs issued
187+
// by non-interactive SK CAs would fail to verify.
188+
func TestCheckCertSKAuthorityNoUPRequired(t *testing.T) {
189+
caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
190+
if err != nil {
191+
t.Fatal(err)
192+
}
193+
userKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
194+
if err != nil {
195+
t.Fatal(err)
196+
}
197+
userPub, err := NewPublicKey(&userKey.PublicKey)
198+
if err != nil {
199+
t.Fatal(err)
200+
}
201+
202+
mkCert := func(flags byte) *Certificate {
203+
c := &Certificate{
204+
CertType: UserCert,
205+
Key: userPub,
206+
ValidBefore: CertTimeInfinity,
207+
ValidPrincipals: []string{"user"},
208+
}
209+
signSKCert(t, c, caKey, "ssh:", flags)
210+
return c
211+
}
212+
213+
checker := CertChecker{
214+
IsUserAuthority: func(k PublicKey) bool {
215+
caSK := &skECDSAPublicKey{application: "ssh:", PublicKey: caKey.PublicKey}
216+
return bytes.Equal(k.Marshal(), caSK.Marshal())
217+
},
218+
}
219+
220+
// Both UP=0 and UP=1 CA signatures verify: UP is not enforced
221+
// here, matching OpenSSH.
222+
if err := checker.CheckCert("user", mkCert(0)); err != nil {
223+
t.Errorf("UP=0 CA signature must verify (OpenSSH parity): %v", err)
224+
}
225+
if err := checker.CheckCert("user", mkCert(flagUserPresence)); err != nil {
226+
t.Errorf("UP=1 CA signature must verify: %v", err)
227+
}
228+
}
229+
131230
func TestValidateCertTime(t *testing.T) {
132231
cert := Certificate{
133232
ValidPrincipals: []string{"user"},

ssh/keys.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,11 +901,25 @@ type skFields struct {
901901
Counter uint32
902902
}
903903

904+
// flagUserPresence is the "user present" bit (UP) in the SK signature
905+
// flags, matching the FIDO CTAP2 authenticatorData UP flag. See
906+
// openssh/PROTOCOL.u2f.
907+
const flagUserPresence = 0x01
908+
909+
// errSKMissingUserPresence is returned by SK key Verify methods when
910+
// the signature does not assert user presence and the key was not
911+
// marked as no-touch-required.
912+
var errSKMissingUserPresence = errors.New("ssh: signature missing required user presence flag")
913+
904914
type skECDSAPublicKey struct {
905915
// application is a URL-like string, typically "ssh:" for SSH.
906916
// see openssh/PROTOCOL.u2f for details.
907917
application string
908918
ecdsa.PublicKey
919+
// noTouchRequired, when true, disables the default user-presence
920+
// check in Verify. It is set by skKeyWithoutUP on a clone of the
921+
// key, never on an instance shared across authentication attempts.
922+
noTouchRequired bool
909923
}
910924

911925
func (k *skECDSAPublicKey) Type() string {
@@ -991,6 +1005,10 @@ func (k *skECDSAPublicKey) Verify(data []byte, sig *Signature) error {
9911005
return err
9921006
}
9931007

1008+
if skf.Flags&flagUserPresence == 0 && !k.noTouchRequired {
1009+
return errSKMissingUserPresence
1010+
}
1011+
9941012
blob := struct {
9951013
ApplicationDigest []byte `ssh:"rest"`
9961014
Flags byte
@@ -1024,6 +1042,10 @@ type skEd25519PublicKey struct {
10241042
// see openssh/PROTOCOL.u2f for details.
10251043
application string
10261044
ed25519.PublicKey
1045+
// noTouchRequired, when true, disables the default user-presence
1046+
// check in Verify. It is set by skKeyWithoutUP on a clone of the
1047+
// key, never on an instance shared across authentication attempts.
1048+
noTouchRequired bool
10271049
}
10281050

10291051
func (k *skEd25519PublicKey) Type() string {
@@ -1098,6 +1120,10 @@ func (k *skEd25519PublicKey) Verify(data []byte, sig *Signature) error {
10981120
return err
10991121
}
11001122

1123+
if skf.Flags&flagUserPresence == 0 && !k.noTouchRequired {
1124+
return errSKMissingUserPresence
1125+
}
1126+
11011127
blob := struct {
11021128
ApplicationDigest []byte `ssh:"rest"`
11031129
Flags byte

0 commit comments

Comments
 (0)