From 4fa540e3f22f670146b375a86c8d8f3b9f4b521f Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Tue, 31 Dec 2024 17:06:16 +0100 Subject: [PATCH 1/6] chore: add more password hashing tests --- internal/crypto/password_test.go | 69 ++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/internal/crypto/password_test.go b/internal/crypto/password_test.go index 3f210810cb..85c8005f48 100644 --- a/internal/crypto/password_test.go +++ b/internal/crypto/password_test.go @@ -123,3 +123,72 @@ func TestScrypt(t *testing.T) { }) } } + +type bcryptTestCase struct { + name string + hash string + password string + shouldPass bool +} + +func TestBcrypt(t *testing.T) { + testCases := []bcryptTestCase{ + { + name: "Valid bcrypt hash, valid password", + hash: "$2a$10$vVz26aE3xkpSS9HFgafcH.M0Ina2tRm.Kp08WcVfjipXccGakj6i.", + password: "test", + shouldPass: true, + }, + { + name: "Invalid bycrypt hash format", + hash: "x2a$10$vVz26aE3xkpSS9HFgafcH.M0Ina2tRm.Kp08WcVfjipXccGakj6i.", + password: "test", + shouldPass: false, + }, + { + name: "Invalid bycrypt hash rounds, negative", + hash: "$2a$-1$vVz26aE3xkpSS9HFgafcH.M0Ina2tRm.Kp08WcVfjipXccGakj6i.", + password: "test", + shouldPass: false, + }, + { + name: "Invalid bycrypt hash rounds", + hash: "$2a$2000$vVz26aE3xkpSS9HFgafcH.M0Ina2tRm.Kp08WcVfjipXccGakj6i.", + password: "test", + shouldPass: false, + }, + { + name: "Valid bcrypt hash, invalid password", + hash: "$2a$10$vVz26aE3xkpSS9HFgafcH.M0Ina2tRm.Kp08WcVfjipXccGakj6i.", + password: "test_Password", + shouldPass: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := CompareHashAndPassword(context.Background(), tc.hash, tc.password) + if tc.shouldPass { + assert.NoError(t, err, "Expected test case to pass, but it failed") + } else { + assert.Error(t, err, "Expected test case to fail, but it passed") + } + }) + } +} + +func TestBcryptHashGeneration(t *testing.T) { + plainText := "testPassword" + ctx := context.Background() + + hashedPassword, e := GenerateFromPassword(ctx, plainText) + assert.NoError(t, e, "No error was expected") + assert.NotNil(t, hashedPassword) + + err := CompareHashAndPassword(context.Background(), hashedPassword, plainText) + assert.NoError(t, err, "Expected hashedPassword to be valid") + + // validate hash is unique each time + newHashedPassword, _ := GenerateFromPassword(ctx, plainText) + assert.NotEqual(t, hashedPassword, newHashedPassword) +} From 89d2958ec51f72b49f388af15e863d6c94ecbe58 Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 2 Jan 2025 11:07:51 +0100 Subject: [PATCH 2/6] feat: check bcrypt format --- internal/crypto/password_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/crypto/password_test.go b/internal/crypto/password_test.go index 85c8005f48..fbbad2b9fa 100644 --- a/internal/crypto/password_test.go +++ b/internal/crypto/password_test.go @@ -2,6 +2,7 @@ package crypto import ( "context" + "regexp" "strings" "testing" @@ -185,6 +186,12 @@ func TestBcryptHashGeneration(t *testing.T) { assert.NoError(t, e, "No error was expected") assert.NotNil(t, hashedPassword) + // validate bcrypt format -- https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#format-algorithm + bcryptRegex, _ := regexp.Compile(`^\$(?P2[abxy])\$(?P[0-9]{1,})\$(?P[./A-Za-z0-9]{21}[.Oeu]{1})(?P[./A-Za-z0-9]{31})$`) + match := bcryptRegex.MatchString(hashedPassword) + assert.Equal(t, match, true, "Produced hash not valid bcrypt format") + + // validate hash resolves to plainText err := CompareHashAndPassword(context.Background(), hashedPassword, plainText) assert.NoError(t, err, "Expected hashedPassword to be valid") From 69fc37027bcbd2e162bd2add4e9098ff38a6cb2a Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 2 Jan 2025 11:17:07 +0100 Subject: [PATCH 3/6] feat: check for password truncation bug --- internal/crypto/password_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/crypto/password_test.go b/internal/crypto/password_test.go index fbbad2b9fa..cf4c0f09c8 100644 --- a/internal/crypto/password_test.go +++ b/internal/crypto/password_test.go @@ -198,4 +198,11 @@ func TestBcryptHashGeneration(t *testing.T) { // validate hash is unique each time newHashedPassword, _ := GenerateFromPassword(ctx, plainText) assert.NotEqual(t, hashedPassword, newHashedPassword) + + // validate password truncation causes error (passwords longer than 72 chars) + // this is technically testing the bcrypt libary but make sure we are erroring because it + // very much matters in the context of this package + longPassword := strings.Repeat("A", 73) + _, e = GenerateFromPassword(ctx, longPassword) + assert.Error(t, e, "Password longer than 72 chars did not error") } From 2782e12f9379ea33709872511e761a413ebd7cad Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 2 Jan 2025 13:00:30 +0100 Subject: [PATCH 4/6] feat: add more tests for argon2 and scrypt --- internal/crypto/password_test.go | 181 ++++++++++++++++++++++++++++--- 1 file changed, 166 insertions(+), 15 deletions(-) diff --git a/internal/crypto/password_test.go b/internal/crypto/password_test.go index cf4c0f09c8..14b81fbd71 100644 --- a/internal/crypto/password_test.go +++ b/internal/crypto/password_test.go @@ -9,20 +9,112 @@ import ( "github.com/stretchr/testify/assert" ) -func TestArgon2(t *testing.T) { - // all of these hash the `test` string with various parameters - - examples := []string{ - "$argon2i$v=19$m=16,t=2,p=1$bGJRWThNOHJJTVBSdHl2dQ$NfEnUOuUpb7F2fQkgFUG4g", - "$argon2id$v=19$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", - } +type argonTestCase struct { + name string + hash string + password string + shouldPass bool + expectedErr string +} - for _, example := range examples { - assert.NoError(t, CompareHashAndPassword(context.Background(), example, "test")) +func TestArgon2(t *testing.T) { + testCases := []argonTestCase{ + { + name: "Argon2: valid hash", + hash: "$argon2i$v=19$m=16,t=2,p=1$bGJRWThNOHJJTVBSdHl2dQ$NfEnUOuUpb7F2fQkgFUG4g", + password: "test", + shouldPass: true, + }, + { + name: "Argon2: valid hash2", + hash: "$argon2id$v=19$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test", + shouldPass: true, + }, + { + name: "Argon2: valid hash, wrong plaintext", + hash: "$argon2id$v=19$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test1", + shouldPass: false, + expectedErr: "crypto: argon2 hash and password mismatch", + }, + { + name: "Argon2: invalid hash alg", + hash: "$argon2ix$v=19$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test1", + shouldPass: false, + expectedErr: "crypto: incorrect argon2 hash format", + }, + { + name: "Argon2: invalid hash v", + hash: "$argon2id$v=20$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test1", + shouldPass: false, + expectedErr: "crypto: incorrect argon2 hash format", + }, + { + name: "Argon2: invalid hash keyid", + hash: "$argon2id$v=19$m=32,t=3,p=2,keyid=1$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test1", + shouldPass: false, + expectedErr: "crypto: argon2 hashes with the keyid parameter not supported", + }, + { + name: "Argon2: invalid hash data", + hash: "$argon2id$v=19$m=32,t=3,p=2,data=1$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test1", + shouldPass: false, + expectedErr: "crypto: argon2 hashes with the data parameter not supported", + }, + { + name: "Argon2: invalid hash memory", + hash: "$argon2id$v=19$m=4294967296,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test", + shouldPass: false, + expectedErr: "crypto: argon2 hash has invalid m parameter \"4294967296\" strconv.ParseUint: parsing \"4294967296\": value out of range", + }, + { + name: "Argon2: invalid hash time", + hash: "$argon2id$v=19$m=32,t=4294967296,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test", + shouldPass: false, + expectedErr: "crypto: argon2 hash has invalid t parameter \"4294967296\" strconv.ParseUint: parsing \"4294967296\": value out of range", + }, + { + name: "Argon2: invalid hash p", + hash: "$argon2id$v=19$m=32,t=3,p=4294967296$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test", + shouldPass: false, + expectedErr: "crypto: argon2 hash has invalid p parameter \"4294967296\" strconv.ParseUint: parsing \"4294967296\": value out of range", + }, + { + name: "Argon2: invalid hash, bad saltB64", + hash: "$argon2id$v=19$m=32,t=3,p=2$S!VpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test", + shouldPass: false, + expectedErr: "crypto: argon2 hash has invalid base64 in the salt section illegal base64 data at input byte 1", + }, + { + name: "Argon2: invalid hash, bad hashB64", + hash: "$argon2id$v=19$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$-Xnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test", + shouldPass: false, + expectedErr: "crypto: argon2 hash has invalid base64 in the hash section illegal base64 data at input byte 0", + }, } - for _, example := range examples { - assert.Error(t, CompareHashAndPassword(context.Background(), example, "test1")) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := CompareHashAndPassword(context.Background(), tc.hash, tc.password) + if tc.shouldPass { + assert.NoError(t, err, "Expected test case to pass, but it failed") + } else { + assert.Error(t, err, "Expected test case to fail, but it passed") + if tc.expectedErr != "" { + assert.Equal(t, tc.expectedErr, err.Error(), "Expected error doesn't match") + } + } + }) } } @@ -91,10 +183,11 @@ func TestGeneratePassword(t *testing.T) { } type scryptTestCase struct { - name string - hash string - password string - shouldPass bool + name string + hash string + password string + shouldPass bool + expectedErr string } func TestScrypt(t *testing.T) { @@ -111,6 +204,61 @@ func TestScrypt(t *testing.T) { password: "mytestpassword", shouldPass: false, }, + { + name: "Firebase Scrypt: mismatch hash plaintext", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "not_mytestpassword", + shouldPass: false, + }, + { + name: "Firebase Scrypt: mismatch hash plaintext", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "not_mytestpassword", + shouldPass: false, + expectedErr: "crypto: fbscrypt hash and password mismatch", + }, + { + name: "Firebase Scrypt: bad hash", + hash: "$fbscrypts$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "crypto: incorrect scrypt hash format", + }, + { + name: "Firebase Scrypt: bad n", + hash: "$fbscrypt$v=1,n=4294967296,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "crypto: Firebase scrypt hash has invalid n parameter \"4294967296\" strconv.ParseUint: parsing \"4294967296\": value out of range", + }, + { + name: "Firebase Scrypt: bad rounds", + hash: "$fbscrypt$v=1,n=14,r=18446744073709551616,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "crypto: Firebase scrypt hash has invalid r parameter \"18446744073709551616\": strconv.ParseUint: parsing \"18446744073709551616\": value out of range", + }, + { + name: "Firebase Scrypt: bad threads - wrap around", + hash: "$fbscrypt$v=1,n=14,r=8,p=256,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "crypto: Firebase scrypt hash has invalid p parameter \"256\" strconv.ParseUint: parsing \"256\": value out of range", + }, + { + name: "Firebase Scrypt: bad salt", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw?==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "illegal base64 data at input byte 2", + }, + { + name: "Firebase Scrypt: bad hash", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9!tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "illegal base64 data at input byte 3", + }, } for _, tc := range testCases { @@ -120,6 +268,9 @@ func TestScrypt(t *testing.T) { assert.NoError(t, err, "Expected test case to pass, but it failed") } else { assert.Error(t, err, "Expected test case to fail, but it passed") + if tc.expectedErr != "" { + assert.Equal(t, tc.expectedErr, err.Error(), "Expected error doesn't match") + } } }) } From 41a6db56c5cf81ecd65f79589670f3c0645ead64 Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 2 Jan 2025 15:00:16 +0100 Subject: [PATCH 5/6] add some missed error check tests --- internal/crypto/password_test.go | 49 +++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/internal/crypto/password_test.go b/internal/crypto/password_test.go index 14b81fbd71..ed18a189dc 100644 --- a/internal/crypto/password_test.go +++ b/internal/crypto/password_test.go @@ -38,6 +38,13 @@ func TestArgon2(t *testing.T) { shouldPass: false, expectedErr: "crypto: argon2 hash and password mismatch", }, + { + name: "Argon2: unsupported algorith", + hash: "$argon2d$v=19$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + password: "test1", + shouldPass: false, + expectedErr: "crypto: argon2 hash uses unsupported algorithm \"argon2d\" only argon2i and argon2id supported", + }, { name: "Argon2: invalid hash alg", hash: "$argon2ix$v=19$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", @@ -47,10 +54,10 @@ func TestArgon2(t *testing.T) { }, { name: "Argon2: invalid hash v", - hash: "$argon2id$v=20$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", + hash: "$argon2id$v=16$m=32,t=3,p=2$SFVpOWJ0eXhjRzVkdGN1RQ$RXnb8rh7LaDcn07xsssqqulZYXOM/EUCEFMVcAcyYVk", password: "test1", shouldPass: false, - expectedErr: "crypto: incorrect argon2 hash format", + expectedErr: "crypto: argon2 hash uses unsupported version \"16\" only 19 is supported", }, { name: "Argon2: invalid hash keyid", @@ -217,6 +224,13 @@ func TestScrypt(t *testing.T) { shouldPass: false, expectedErr: "crypto: fbscrypt hash and password mismatch", }, + { + name: "Firebase Scrypt: bad hash version", + hash: "$fbscrypt$v=2,n=14,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "crypto: Firebase scrypt hash uses unsupported version \"2\" only version 1 is supported", + }, { name: "Firebase Scrypt: bad hash", hash: "$fbscrypts$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", @@ -231,6 +245,13 @@ func TestScrypt(t *testing.T) { shouldPass: false, expectedErr: "crypto: Firebase scrypt hash has invalid n parameter \"4294967296\" strconv.ParseUint: parsing \"4294967296\": value out of range", }, + { + name: "Firebase Scrypt: zero n", + hash: "$fbscrypt$v=1,n=0,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "crypto: Firebase scrypt hash has invalid n parameter \"0\": must be greater than 0", + }, { name: "Firebase Scrypt: bad rounds", hash: "$fbscrypt$v=1,n=14,r=18446744073709551616,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", @@ -245,19 +266,33 @@ func TestScrypt(t *testing.T) { shouldPass: false, expectedErr: "crypto: Firebase scrypt hash has invalid p parameter \"256\" strconv.ParseUint: parsing \"256\": value out of range", }, + { + name: "Firebase Scrypt: bad hash", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$!KVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "crypto: Firebase scrypt hash has invalid base64 in the hash section illegal base64 data at input byte 0", + }, { name: "Firebase Scrypt: bad salt", - hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw?==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$!0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", password: "mytestpassword", shouldPass: false, - expectedErr: "illegal base64 data at input byte 2", + expectedErr: "crypto: Firebase scrypt salt has invalid base64 in the hash section illegal base64 data at input byte 0", }, { - name: "Firebase Scrypt: bad hash", - hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw==,sk=ou9!tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + name: "Firebase Scrypt: bad ss", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=B!w==,sk=ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", + password: "mytestpassword", + shouldPass: false, + expectedErr: "illegal base64 data at input byte 1", + }, + { + name: "Firebase Scrypt: bad sk", + hash: "$fbscrypt$v=1,n=14,r=8,p=1,ss=Bw==,sk=!ou9tdYTGyYm8kuR6Dt0Bp0kDuAYoXrK16mbZO4yGwAn3oLspjnN0/c41v8xZnO1n14J3MjKj1b2g6AUCAlFwMw==$C0sHCg9ek77hsg==$zKVTMvnWVw5BBOZNUdnsalx4c4c7y/w7IS5p6Ut2+CfEFFlz37J9huyQfov4iizN8dbjvEJlM5tQaJP84+hfTw==", password: "mytestpassword", shouldPass: false, - expectedErr: "illegal base64 data at input byte 3", + expectedErr: "illegal base64 data at input byte 0", }, } From 0981f9cb0aefcc928bb766f61977ef2600901f2a Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 2 Jan 2025 15:56:51 +0100 Subject: [PATCH 6/6] add some more crypto tests --- internal/crypto/crypto_test.go | 77 ++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/internal/crypto/crypto_test.go b/internal/crypto/crypto_test.go index b677b918d7..edd5c3ae2b 100644 --- a/internal/crypto/crypto_test.go +++ b/internal/crypto/crypto_test.go @@ -1,7 +1,10 @@ package crypto import ( + "encoding/base64" + "fmt" "testing" + "time" "github.com/gofrs/uuid" "github.com/stretchr/testify/assert" @@ -32,3 +35,77 @@ func TestEncryptedString(t *testing.T) { assert.NoError(t, err) assert.Equal(t, []byte("data"), decrypted) } + +func TestSecureToken(t *testing.T) { + secureToken := SecureToken() + secureTokenTwo := SecureToken() + // token must be decoded to check length, we could use base64.RawURLEncoding.DecodedLen + decodedToken, err := base64.RawURLEncoding.DecodeString(secureToken) + assert.NoError(t, err, "Token should be base64 URL encoded") + assert.Len(t, decodedToken, 16, "Tokens should be generated with default length of 16") + assert.NotEqual(t, secureToken, secureTokenTwo, "Tokens MUST always be random") + + // test custom length + secureToken = SecureToken(32) + // token must be decoded to check length, we could use base64.RawURLEncoding.DecodedLen + decodedToken, err = base64.RawURLEncoding.DecodeString(secureToken) + assert.NoError(t, err, "Token should be base64 URL encoded") + assert.Len(t, decodedToken, 32, "Tokens should be generated with default length of 16") +} + +func TestGenerateOTP(t *testing.T) { + otp, err := GenerateOtp(5) + assert.NoError(t, err) + assert.NotEmpty(t, otp, "Empty OTP generated") + assert.Len(t, otp, 5, "OTP generated to unexpected length") +} + +type signatureTestCase struct { + name string + id uuid.UUID + secrets []string + data []byte + shouldPass bool + expectedErr string +} + +func TestGenerateSignatures(t *testing.T) { + testCases := []signatureTestCase{ + { + name: "Valid signature", + id: uuid.Must(uuid.NewV4()), + secrets: []string{fmt.Sprintf("v1,%s", base64.StdEncoding.EncodeToString([]byte("randomsecret")))}, + shouldPass: true, + }, + { + name: "Invalid secret prefix", + id: uuid.Must(uuid.NewV4()), + secrets: []string{base64.StdEncoding.EncodeToString([]byte("randomsecret"))}, + shouldPass: false, + expectedErr: "invalid signature format", + }, + { + name: "Invalid secret encoding", + id: uuid.Must(uuid.NewV4()), + secrets: []string{"v1,random secret"}, + shouldPass: false, + expectedErr: "unable to create webhook, err: illegal base64 data at input byte 6", + }, + } + currentTime := time.Now() + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + signatureList, err := GenerateSignatures(tc.secrets, tc.id, currentTime, tc.data) + if tc.shouldPass { + assert.NoError(t, err) + assert.Len(t, signatureList, 1) + assert.NotEqual(t, signatureList[0], tc.secrets[0]) + } else { + assert.Error(t, err, "Expected test case to fail, but it passed") + if tc.expectedErr != "" { + assert.Equal(t, tc.expectedErr, err.Error(), "Expected error doesn't match") + } + } + }) + } +}