Skip to content

Commit 79a3d27

Browse files
zeripathdvejmz
andauthored
Don't panic if we fail to parse U2FRegistration data (#17304) (#17371)
Backport #17304 Downgrade logging statement from Fatal to Error so that errors parsing U2FRegistration data does not panic; instead, the invalid key will be skipped and we will attempt to parse the next one, if available. Signed-off-by: David Jimenez <[email protected]> Co-authored-by: David Jimenez <[email protected]>
1 parent eb748ff commit 79a3d27

File tree

3 files changed

+28
-2
lines changed

3 files changed

+28
-2
lines changed

models/u2f.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (list U2FRegistrationList) ToRegistrations() []u2f.Registration {
5252
for _, reg := range list {
5353
r, err := reg.Parse()
5454
if err != nil {
55-
log.Fatal("parsing u2f registration: %v", err)
55+
log.Error("parsing u2f registration: %v", err)
5656
continue
5757
}
5858
regs = append(regs, *r)

models/u2f_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package models
66

77
import (
8+
"encoding/hex"
89
"testing"
910

1011
"github.com/stretchr/testify/assert"
@@ -27,6 +28,7 @@ func TestGetU2FRegistrationsByUID(t *testing.T) {
2728
assert.NoError(t, PrepareTestDatabase())
2829

2930
res, err := GetU2FRegistrationsByUID(1)
31+
3032
assert.NoError(t, err)
3133
assert.Len(t, res, 1)
3234
assert.Equal(t, "U2F Key", res[0].Name)
@@ -71,3 +73,27 @@ func TestDeleteRegistration(t *testing.T) {
7173
assert.NoError(t, DeleteRegistration(reg))
7274
AssertNotExistsBean(t, &U2FRegistration{ID: 1})
7375
}
76+
77+
const validU2FRegistrationResponseHex = "0504b174bc49c7ca254b70d2e5c207cee9cf174820ebd77ea3c65508c26da51b657c1cc6b952f8621697936482da0a6d3d3826a59095daf6cd7c03e2e60385d2f6d9402a552dfdb7477ed65fd84133f86196010b2215b57da75d315b7b9e8fe2e3925a6019551bab61d16591659cbaf00b4950f7abfe6660e2e006f76868b772d70c253082013c3081e4a003020102020a47901280001155957352300a06082a8648ce3d0403023017311530130603550403130c476e756262792050696c6f74301e170d3132303831343138323933325a170d3133303831343138323933325a3031312f302d0603550403132650696c6f74476e756262792d302e342e312d34373930313238303030313135353935373335323059301306072a8648ce3d020106082a8648ce3d030107034200048d617e65c9508e64bcc5673ac82a6799da3c1446682c258c463fffdf58dfd2fa3e6c378b53d795c4a4dffb4199edd7862f23abaf0203b4b8911ba0569994e101300a06082a8648ce3d0403020347003044022060cdb6061e9c22262d1aac1d96d8c70829b2366531dda268832cb836bcd30dfa0220631b1459f09e6330055722c8d89b7f48883b9089b88d60d1d9795902b30410df304502201471899bcc3987e62e8202c9b39c33c19033f7340352dba80fcab017db9230e402210082677d673d891933ade6f617e5dbde2e247e70423fd5ad7804a6d3d3961ef871"
78+
79+
func TestToRegistrations_SkipInvalidItemsWithoutCrashing(t *testing.T) {
80+
regKeyRaw, _ := hex.DecodeString(validU2FRegistrationResponseHex)
81+
regs := U2FRegistrationList{
82+
&U2FRegistration{ID: 1},
83+
&U2FRegistration{ID: 2, Name: "U2F Key", UserID: 2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800},
84+
}
85+
86+
actual := regs.ToRegistrations()
87+
assert.Len(t, actual, 1)
88+
}
89+
90+
func TestToRegistrations(t *testing.T) {
91+
regKeyRaw, _ := hex.DecodeString(validU2FRegistrationResponseHex)
92+
regs := U2FRegistrationList{
93+
&U2FRegistration{ID: 1, Name: "U2F Key", UserID: 1, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800},
94+
&U2FRegistration{ID: 2, Name: "U2F Key", UserID: 2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800},
95+
}
96+
97+
actual := regs.ToRegistrations()
98+
assert.Len(t, actual, 2)
99+
}

routers/web/user/auth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ func U2FSign(ctx *context.Context) {
451451
for _, reg := range regs {
452452
r, err := reg.Parse()
453453
if err != nil {
454-
log.Fatal("parsing u2f registration: %v", err)
454+
log.Error("parsing u2f registration: %v", err)
455455
continue
456456
}
457457
newCounter, authErr := r.Authenticate(*signResp, *challenge, reg.Counter)

0 commit comments

Comments
 (0)