From 656d4b1abf778598a7577d8b5a21f18dc936e2de Mon Sep 17 00:00:00 2001 From: David Jimenez Date: Thu, 14 Oct 2021 01:07:18 +0100 Subject: [PATCH 1/2] Don't panic if we fail to parse a U2FRegistration data 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 --- models/login/u2f.go | 2 +- models/login/u2f_test.go | 27 +++++++++++++++++++++++++++ routers/web/user/auth.go | 2 +- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/models/login/u2f.go b/models/login/u2f.go index 64b1fb322ac8..05d39cc05ec1 100644 --- a/models/login/u2f.go +++ b/models/login/u2f.go @@ -81,7 +81,7 @@ func (list U2FRegistrationList) ToRegistrations() []u2f.Registration { for _, reg := range list { r, err := reg.Parse() if err != nil { - log.Fatal("parsing u2f registration: %v", err) + log.Error("parsing u2f registration: %v", err) continue } regs = append(regs, *r) diff --git a/models/login/u2f_test.go b/models/login/u2f_test.go index b0305775caf5..3fca8f006c96 100644 --- a/models/login/u2f_test.go +++ b/models/login/u2f_test.go @@ -6,6 +6,7 @@ package login import ( "testing" + "encoding/hex" "code.gitea.io/gitea/models/db" @@ -29,6 +30,7 @@ func TestGetU2FRegistrationsByUID(t *testing.T) { assert.NoError(t, db.PrepareTestDatabase()) res, err := GetU2FRegistrationsByUID(1) + assert.NoError(t, err) assert.Len(t, res, 1) assert.Equal(t, "U2F Key", res[0].Name) @@ -72,3 +74,28 @@ func TestDeleteRegistration(t *testing.T) { assert.NoError(t, DeleteRegistration(reg)) db.AssertNotExistsBean(t, &U2FRegistration{ID: 1}) } + +const validU2FRegistrationResponseHex = +"0504b174bc49c7ca254b70d2e5c207cee9cf174820ebd77ea3c65508c26da51b657c1cc6b952f8621697936482da0a6d3d3826a59095daf6cd7c03e2e60385d2f6d9402a552dfdb7477ed65fd84133f86196010b2215b57da75d315b7b9e8fe2e3925a6019551bab61d16591659cbaf00b4950f7abfe6660e2e006f76868b772d70c253082013c3081e4a003020102020a47901280001155957352300a06082a8648ce3d0403023017311530130603550403130c476e756262792050696c6f74301e170d3132303831343138323933325a170d3133303831343138323933325a3031312f302d0603550403132650696c6f74476e756262792d302e342e312d34373930313238303030313135353935373335323059301306072a8648ce3d020106082a8648ce3d030107034200048d617e65c9508e64bcc5673ac82a6799da3c1446682c258c463fffdf58dfd2fa3e6c378b53d795c4a4dffb4199edd7862f23abaf0203b4b8911ba0569994e101300a06082a8648ce3d0403020347003044022060cdb6061e9c22262d1aac1d96d8c70829b2366531dda268832cb836bcd30dfa0220631b1459f09e6330055722c8d89b7f48883b9089b88d60d1d9795902b30410df304502201471899bcc3987e62e8202c9b39c33c19033f7340352dba80fcab017db9230e402210082677d673d891933ade6f617e5dbde2e247e70423fd5ad7804a6d3d3961ef871" + +func TestToRegistrations_SkipInvalidItemsWithoutCrashing(t *testing.T) { + regKeyRaw, _ := hex.DecodeString(validU2FRegistrationResponseHex) + regs := U2FRegistrationList{ + &U2FRegistration{ID:1}, + &U2FRegistration{ID:2, Name: "U2F Key", UserID:2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800 }, + } + + actual := regs.ToRegistrations() + assert.Len(t, actual, 1) +} + +func TestToRegistrations(t *testing.T) { + regKeyRaw, _ := hex.DecodeString(validU2FRegistrationResponseHex) + regs := U2FRegistrationList{ + &U2FRegistration{ID:1, Name: "U2F Key", UserID:1, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800 }, + &U2FRegistration{ID:2, Name: "U2F Key", UserID:2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800 }, + } + + actual := regs.ToRegistrations() + assert.Len(t, actual, 2) +} diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 65515402cf5d..99885e5ce3db 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -462,7 +462,7 @@ func U2FSign(ctx *context.Context) { for _, reg := range regs { r, err := reg.Parse() if err != nil { - log.Fatal("parsing u2f registration: %v", err) + log.Error("parsing u2f registration: %v", err) continue } newCounter, authErr := r.Authenticate(*signResp, *challenge, reg.Counter) From 57c23365d9de6bacf45f45026cfa36752e9036c9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 14 Oct 2021 18:11:24 +0200 Subject: [PATCH 2/2] make fmt --- models/login/u2f_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/models/login/u2f_test.go b/models/login/u2f_test.go index 3fca8f006c96..32505b62a67d 100644 --- a/models/login/u2f_test.go +++ b/models/login/u2f_test.go @@ -5,8 +5,8 @@ package login import ( - "testing" "encoding/hex" + "testing" "code.gitea.io/gitea/models/db" @@ -75,14 +75,13 @@ func TestDeleteRegistration(t *testing.T) { db.AssertNotExistsBean(t, &U2FRegistration{ID: 1}) } -const validU2FRegistrationResponseHex = -"0504b174bc49c7ca254b70d2e5c207cee9cf174820ebd77ea3c65508c26da51b657c1cc6b952f8621697936482da0a6d3d3826a59095daf6cd7c03e2e60385d2f6d9402a552dfdb7477ed65fd84133f86196010b2215b57da75d315b7b9e8fe2e3925a6019551bab61d16591659cbaf00b4950f7abfe6660e2e006f76868b772d70c253082013c3081e4a003020102020a47901280001155957352300a06082a8648ce3d0403023017311530130603550403130c476e756262792050696c6f74301e170d3132303831343138323933325a170d3133303831343138323933325a3031312f302d0603550403132650696c6f74476e756262792d302e342e312d34373930313238303030313135353935373335323059301306072a8648ce3d020106082a8648ce3d030107034200048d617e65c9508e64bcc5673ac82a6799da3c1446682c258c463fffdf58dfd2fa3e6c378b53d795c4a4dffb4199edd7862f23abaf0203b4b8911ba0569994e101300a06082a8648ce3d0403020347003044022060cdb6061e9c22262d1aac1d96d8c70829b2366531dda268832cb836bcd30dfa0220631b1459f09e6330055722c8d89b7f48883b9089b88d60d1d9795902b30410df304502201471899bcc3987e62e8202c9b39c33c19033f7340352dba80fcab017db9230e402210082677d673d891933ade6f617e5dbde2e247e70423fd5ad7804a6d3d3961ef871" +const validU2FRegistrationResponseHex = "0504b174bc49c7ca254b70d2e5c207cee9cf174820ebd77ea3c65508c26da51b657c1cc6b952f8621697936482da0a6d3d3826a59095daf6cd7c03e2e60385d2f6d9402a552dfdb7477ed65fd84133f86196010b2215b57da75d315b7b9e8fe2e3925a6019551bab61d16591659cbaf00b4950f7abfe6660e2e006f76868b772d70c253082013c3081e4a003020102020a47901280001155957352300a06082a8648ce3d0403023017311530130603550403130c476e756262792050696c6f74301e170d3132303831343138323933325a170d3133303831343138323933325a3031312f302d0603550403132650696c6f74476e756262792d302e342e312d34373930313238303030313135353935373335323059301306072a8648ce3d020106082a8648ce3d030107034200048d617e65c9508e64bcc5673ac82a6799da3c1446682c258c463fffdf58dfd2fa3e6c378b53d795c4a4dffb4199edd7862f23abaf0203b4b8911ba0569994e101300a06082a8648ce3d0403020347003044022060cdb6061e9c22262d1aac1d96d8c70829b2366531dda268832cb836bcd30dfa0220631b1459f09e6330055722c8d89b7f48883b9089b88d60d1d9795902b30410df304502201471899bcc3987e62e8202c9b39c33c19033f7340352dba80fcab017db9230e402210082677d673d891933ade6f617e5dbde2e247e70423fd5ad7804a6d3d3961ef871" func TestToRegistrations_SkipInvalidItemsWithoutCrashing(t *testing.T) { regKeyRaw, _ := hex.DecodeString(validU2FRegistrationResponseHex) regs := U2FRegistrationList{ - &U2FRegistration{ID:1}, - &U2FRegistration{ID:2, Name: "U2F Key", UserID:2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800 }, + &U2FRegistration{ID: 1}, + &U2FRegistration{ID: 2, Name: "U2F Key", UserID: 2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800}, } actual := regs.ToRegistrations() @@ -92,8 +91,8 @@ func TestToRegistrations_SkipInvalidItemsWithoutCrashing(t *testing.T) { func TestToRegistrations(t *testing.T) { regKeyRaw, _ := hex.DecodeString(validU2FRegistrationResponseHex) regs := U2FRegistrationList{ - &U2FRegistration{ID:1, Name: "U2F Key", UserID:1, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800 }, - &U2FRegistration{ID:2, Name: "U2F Key", UserID:2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800 }, + &U2FRegistration{ID: 1, Name: "U2F Key", UserID: 1, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800}, + &U2FRegistration{ID: 2, Name: "U2F Key", UserID: 2, Counter: 0, Raw: regKeyRaw, CreatedUnix: 946684800, UpdatedUnix: 946684800}, } actual := regs.ToRegistrations()