diff --git a/pkg/auth/manager/publicshares/attemptops.go b/pkg/auth/manager/publicshares/attemptops.go new file mode 100644 index 0000000000..f8ccb70809 --- /dev/null +++ b/pkg/auth/manager/publicshares/attemptops.go @@ -0,0 +1,87 @@ +package publicshares + +import ( + "time" +) + +// InsertAttempt ensures the attempt is added in the right possition. +// Since multiple attempts can happen very closely, the attempt at time 73 +// might have been registered before the attempt at time 72 +// Multiple attempts can have the same timestamp but different IDs. If a new +// timestamp has the same timestamp than any other, the new attempt will +// appear first. +// Attempts with the same ID and timestamp will be considered as duplicates, +// so the attempt won't be re-inserted. +func InsertAttempt(attemptList []*attemptData, attempt *attemptData) []*attemptData { + if attempt == nil { + return attemptList + } + + // the expected case is that the attempt's timestamp is newer than + // the last registered attempt, so we can just add it at the end. + if len(attemptList) == 0 || attemptList[len(attemptList)-1].Timestamp < attempt.Timestamp { + return append(attemptList, attempt) + } + + // the attempt should be added close to the end, so start checking + // from the last item + i := len(attemptList) + for i > 0 { + i = i - 1 + if attemptList[i].Timestamp == attempt.Timestamp && attemptList[i].ID == attempt.ID { + // attempt already inserted + return attemptList + } + + if attemptList[i].Timestamp < attempt.Timestamp { + i = i + 1 // adjust the index to point where the attempt should be added + break + } + } + + return append(attemptList[:i], append([]*attemptData{attempt}, attemptList[i:]...)...) +} + +// CleanAttempts will remove obsolete attempt data +func CleanAttempts(attemptList []*attemptData, timeGap time.Duration) []*attemptData { + minTimestamp := time.Now().Add(-1 * timeGap).Unix() + + var index int + for index = 0; index < len(attemptList); index++ { + if attemptList[index].Timestamp >= minTimestamp { + break + } + } + + return attemptList[index:] +} + +// IsSubsetOfAttempts checks if "b" is a subset of "a". Both "a" and "b" MUST +// be sorted +// Note: an empty list is a subset of any list, including other empty list. +// This means that if "b" is empty, this function will return true +// Note2: using nil in either "a" or "b" is discouraged. It will return true +// in both cases, but do NOT rely on this behavior. +func IsSubsetOfAttempts(a, b []*attemptData) bool { + if len(b) > len(a) { + return false + } + + aindex := 0 + for _, bvalue := range b { + for aindex < len(a) { + if a[aindex].Timestamp == bvalue.Timestamp && a[aindex].ID == bvalue.ID { + break + } + if a[aindex].Timestamp > bvalue.Timestamp { + return false + } + + aindex = aindex + 1 + if aindex >= len(a) { + return false + } + } + } + return true +} diff --git a/pkg/auth/manager/publicshares/attemptops_test.go b/pkg/auth/manager/publicshares/attemptops_test.go new file mode 100644 index 0000000000..06b368d74e --- /dev/null +++ b/pkg/auth/manager/publicshares/attemptops_test.go @@ -0,0 +1,456 @@ +package publicshares + +import ( + "encoding/json" + "slices" + "testing" + "time" +) + +func TestInsertAttempt(t *testing.T) { + var testData = map[string]struct { + attemptList []*attemptData + attempt *attemptData + expectedResult []*attemptData + }{ + "general": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + }, + attempt: &attemptData{ID: 22, Timestamp: 2000}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 22, Timestamp: 2000}, + }, + }, + "add first": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + }, + attempt: &attemptData{ID: 22, Timestamp: 500}, + expectedResult: []*attemptData{ + &attemptData{ID: 22, Timestamp: 500}, + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + }, + }, + "add last": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + }, + attempt: &attemptData{ID: 22, Timestamp: 5000}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 22, Timestamp: 5000}, + }, + }, + "add middle": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 48, Timestamp: 1500}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 48, Timestamp: 1500}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "already added first": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 111, Timestamp: 1000}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "already added last": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 101, Timestamp: 2007}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "already added middle": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 123, Timestamp: 1002}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "same timestamp": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 789, Timestamp: 2002}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 789, Timestamp: 2002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "same timestamp2": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 89, Timestamp: 2002}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 89, Timestamp: 2002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "same timestamp already added": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 789, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 126, Timestamp: 2002}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 789, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "same timestamp already added2": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 789, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: &attemptData{ID: 789, Timestamp: 2002}, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 789, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "nil": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 789, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + attempt: nil, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1000}, + &attemptData{ID: 123, Timestamp: 1002}, + &attemptData{ID: 126, Timestamp: 2002}, + &attemptData{ID: 789, Timestamp: 2002}, + &attemptData{ID: 101, Timestamp: 2007}, + }, + }, + "empty list": { + attemptList: []*attemptData{}, + attempt: &attemptData{ID: 789, Timestamp: 2002}, + expectedResult: []*attemptData{ + &attemptData{ID: 789, Timestamp: 2002}, + }, + }, + } + + for name, test := range testData { + t.Run(name, func(t *testing.T) { + result := InsertAttempt(test.attemptList, test.attempt) + areEqual := slices.EqualFunc(result, test.expectedResult, func(a, b *attemptData) bool { + return a.ID == b.ID && a.Timestamp == b.Timestamp + }) + + if !areEqual { + jresult, _ := json.Marshal(result) + jexpected, _ := json.Marshal(test.expectedResult) + t.Errorf("attempt data wasn't added properly. result = %s , expected result = %s", jresult, jexpected) + } + }) + } +} + +func TestCleanAttempts(t *testing.T) { + now := time.Now().Unix() + + var testData = map[string]struct { + attemptList []*attemptData + timeGap time.Duration + expectedResult []*attemptData + }{ + "just added": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: now}, + &attemptData{ID: 123, Timestamp: now}, + &attemptData{ID: 126, Timestamp: now}, + }, + timeGap: 15 * 24 * time.Hour, + expectedResult: []*attemptData{ + &attemptData{ID: 111, Timestamp: now}, + &attemptData{ID: 123, Timestamp: now}, + &attemptData{ID: 126, Timestamp: now}, + }, + }, + "some obsolete": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 22}, + &attemptData{ID: 123, Timestamp: now}, + &attemptData{ID: 126, Timestamp: now}, + }, + timeGap: 15 * 24 * time.Hour, + expectedResult: []*attemptData{ + &attemptData{ID: 123, Timestamp: now}, + &attemptData{ID: 126, Timestamp: now}, + }, + }, + "all obsolete": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 22}, + &attemptData{ID: 123, Timestamp: 23}, + &attemptData{ID: 126, Timestamp: 24}, + }, + timeGap: 15 * 24 * time.Hour, + expectedResult: []*attemptData{}, + }, + } + + for name, test := range testData { + t.Run(name, func(t *testing.T) { + result := CleanAttempts(test.attemptList, test.timeGap) + areEqual := slices.EqualFunc(result, test.expectedResult, func(a, b *attemptData) bool { + return a.ID == b.ID && a.Timestamp == b.Timestamp + }) + + if !areEqual { + jresult, _ := json.Marshal(result) + jexpected, _ := json.Marshal(test.expectedResult) + t.Errorf("attempt data wasn't cleaned properly. result = %s , expected result = %s", jresult, jexpected) + } + }) + } +} + +func TestIsSubsetOfAttempts(t *testing.T) { + var testData = map[string]struct { + attemptList []*attemptData + subset []*attemptData + expectedResult bool + }{ + "same list": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: true, + }, + "subset longer": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + &attemptData{ID: 526, Timestamp: 2234}, + }, + expectedResult: false, + }, + "new added last": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + &attemptData{ID: 226, Timestamp: 2234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: true, + }, + "new added middle": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 226, Timestamp: 1034}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: true, + }, + "new added first": { + attemptList: []*attemptData{ + &attemptData{ID: 226, Timestamp: 500}, + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: true, + }, + "new with same timestamp": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 223, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: true, + }, + "new with same timestamp2": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 223, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 223, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: true, + }, + "matching timestamp different id": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 993, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: false, + }, + "empty list": { + attemptList: []*attemptData{}, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 993, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: false, + }, + "empty subset": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: []*attemptData{}, + expectedResult: true, + }, + "empty list and subset": { + attemptList: []*attemptData{}, + subset: []*attemptData{}, + expectedResult: true, + }, + "nil list": { + attemptList: nil, + subset: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 993, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + expectedResult: false, + }, + "nil subset": { + attemptList: []*attemptData{ + &attemptData{ID: 111, Timestamp: 1001}, + &attemptData{ID: 123, Timestamp: 1005}, + &attemptData{ID: 126, Timestamp: 1234}, + }, + subset: nil, + expectedResult: true, + }, + "nil list and subset": { + attemptList: nil, + subset: nil, + expectedResult: true, + }, + } + + for name, test := range testData { + t.Run(name, func(t *testing.T) { + result := IsSubsetOfAttempts(test.attemptList, test.subset) + + if result != test.expectedResult { + jlist, _ := json.Marshal(test.attemptList) + jsub, _ := json.Marshal(test.subset) + t.Errorf("b isn't a subset of a. a = %s , b = %s", jlist, jsub) + } + }) + } +} diff --git a/pkg/auth/manager/publicshares/bruteforceprotection.go b/pkg/auth/manager/publicshares/bruteforceprotection.go index 65c3b9f682..44fb1309a8 100644 --- a/pkg/auth/manager/publicshares/bruteforceprotection.go +++ b/pkg/auth/manager/publicshares/bruteforceprotection.go @@ -113,7 +113,7 @@ func (bfp *BruteForceProtection) Verify(ctx context.Context, shareToken string) return false } - updatedList := bfp.cleanAttempts(attemptList) + updatedList := CleanAttempts(attemptList, bfp.timeGap) if len(attemptList) != len(updatedList) { sublog.Debug().Msg("Cleaning obsolete attempts") if _, err := bfp.writeWithRetry(shareToken, nil, sublog); err != nil { @@ -131,52 +131,6 @@ func (bfp *BruteForceProtection) Verify(ctx context.Context, shareToken string) return updatedListCount <= bfp.maxAttempts } -// Ensure the attempt is added in the right possition. Since multiple attempts -// can happen very closely, the attempt at time 73 might have been registered -// before the attempt at time 72 -func (bfp *BruteForceProtection) insertAttempt(attemptList []*attemptData, attempt *attemptData) []*attemptData { - if attempt == nil { - return attemptList - } - - var i int - for i = 0; i < len(attemptList); i++ { - if attemptList[i].Timestamp > attempt.Timestamp { - break - } - } - - return append(attemptList[:i], append([]*attemptData{attempt}, attemptList[i:]...)...) -} - -// cleanAttempts will remove obsolete attempt data -func (bfp *BruteForceProtection) cleanAttempts(attemptList []*attemptData) []*attemptData { - minTimestamp := time.Now().Add(-1 * bfp.timeGap).Unix() - - var index int - for index = 0; index < len(attemptList); index++ { - if attemptList[index].Timestamp >= minTimestamp { - break - } - } - - return attemptList[index:] -} - -// areEqualLists checks that both lists have the same data -func (bfp *BruteForceProtection) areEqualLists(a, b []*attemptData) bool { - if len(a) != len(b) { - return false - } - - for i := 0; i < len(a); i++ { // already checked that the length of both lists are the same - if a[i].Timestamp != b[i].Timestamp { - return false - } - } - return true -} - // writeWithRetry will write the attempt data for the share. You can use a // nil attempt in order to just update and remove obsolete attempts. // This involves several steps: @@ -204,7 +158,7 @@ func (bfp *BruteForceProtection) writeWithRetry(shareToken string, attempt *atte tries := 0 for tries = 0; tries < maxWriteRetries; tries++ { - updatedList := bfp.cleanAttempts(bfp.insertAttempt(attemptList, attempt)) + updatedList := CleanAttempts(InsertAttempt(attemptList, attempt), bfp.timeGap) if len(updatedList) == 0 { // if all attempts expired, delete the info // the new attempt shouldn't have expired, so this shouldn't happen @@ -228,11 +182,10 @@ func (bfp *BruteForceProtection) writeWithRetry(shareToken string, attempt *atte return 0, err } - // TODO: the areEqualLists might be too strict. We might just need - // to check that the data we want to add is present despite there - // could be additional data from other replicas - if bfp.areEqualLists(attemptList, updatedList) { - // if both lists are equal, the write was successful + if IsSubsetOfAttempts(attemptList, updatedList) { + // Note: both attemptList and updatedList MUST be sorted by timestamp. + // Check if the updated list is a subset of the list just obtained. + // If so, the write was successful despite having additional data. break }