354 lines
11 KiB
Diff
354 lines
11 KiB
Diff
|
commit f13c08e9f45d7776cb264b17ec41bc4ff51fc0b9
|
||
|
Author: Andreas Gerstmayr <agerstmayr@redhat.com>
|
||
|
Date: Thu Nov 25 18:49:52 2021 +0100
|
||
|
|
||
|
notifications: use HMAC-SHA256 to generate time limit codes
|
||
|
|
||
|
* changes the time limit code generation function to use HMAC-SHA256
|
||
|
instead of SHA-1
|
||
|
* multiple new testcases
|
||
|
|
||
|
diff --git a/pkg/services/notifications/codes.go b/pkg/services/notifications/codes.go
|
||
|
index ea9beb30cc..1ddf05dc69 100644
|
||
|
--- a/pkg/services/notifications/codes.go
|
||
|
+++ b/pkg/services/notifications/codes.go
|
||
|
@@ -1,48 +1,53 @@
|
||
|
package notifications
|
||
|
|
||
|
import (
|
||
|
- "crypto/sha1" // #nosec
|
||
|
+ "crypto/hmac"
|
||
|
+ "crypto/sha256"
|
||
|
"encoding/hex"
|
||
|
"fmt"
|
||
|
+ "strconv"
|
||
|
"time"
|
||
|
|
||
|
- "github.com/unknwon/com"
|
||
|
-
|
||
|
"github.com/grafana/grafana/pkg/models"
|
||
|
"github.com/grafana/grafana/pkg/setting"
|
||
|
)
|
||
|
|
||
|
-const timeLimitCodeLength = 12 + 6 + 40
|
||
|
+const timeLimitStartDateLength = 12
|
||
|
+const timeLimitMinutesLength = 6
|
||
|
+const timeLimitHmacLength = 64
|
||
|
+const timeLimitCodeLength = timeLimitStartDateLength + timeLimitMinutesLength + timeLimitHmacLength
|
||
|
|
||
|
// create a time limit code
|
||
|
-// code format: 12 length date time string + 6 minutes string + 40 sha1 encoded string
|
||
|
-func createTimeLimitCode(data string, minutes int, startInf interface{}) (string, error) {
|
||
|
+// code format: 12 length date time string + 6 minutes string + 64 HMAC-SHA256 encoded string
|
||
|
+func createTimeLimitCode(payload string, minutes int, startStr string) (string, error) {
|
||
|
format := "200601021504"
|
||
|
|
||
|
var start, end time.Time
|
||
|
- var startStr, endStr string
|
||
|
+ var endStr string
|
||
|
|
||
|
- if startInf == nil {
|
||
|
+ if startStr == "" {
|
||
|
// Use now time create code
|
||
|
start = time.Now()
|
||
|
startStr = start.Format(format)
|
||
|
} else {
|
||
|
// use start string create code
|
||
|
- startStr = startInf.(string)
|
||
|
- start, _ = time.ParseInLocation(format, startStr, time.Local)
|
||
|
- startStr = start.Format(format)
|
||
|
+ var err error
|
||
|
+ start, err = time.ParseInLocation(format, startStr, time.Local)
|
||
|
+ if err != nil {
|
||
|
+ return "", err
|
||
|
+ }
|
||
|
}
|
||
|
|
||
|
end = start.Add(time.Minute * time.Duration(minutes))
|
||
|
endStr = end.Format(format)
|
||
|
|
||
|
- // create sha1 encode string
|
||
|
- sh := sha1.New()
|
||
|
- if _, err := sh.Write([]byte(data + setting.SecretKey + startStr + endStr +
|
||
|
- com.ToStr(minutes))); err != nil {
|
||
|
- return "", err
|
||
|
+ // create HMAC-SHA256 encoded string
|
||
|
+ key := []byte(setting.SecretKey)
|
||
|
+ h := hmac.New(sha256.New, key)
|
||
|
+ if _, err := h.Write([]byte(payload + startStr + endStr)); err != nil {
|
||
|
+ return "", fmt.Errorf("cannot create hmac: %v", err)
|
||
|
}
|
||
|
- encoded := hex.EncodeToString(sh.Sum(nil))
|
||
|
+ encoded := hex.EncodeToString(h.Sum(nil))
|
||
|
|
||
|
code := fmt.Sprintf("%s%06d%s", startStr, minutes, encoded)
|
||
|
return code, nil
|
||
|
@@ -50,30 +55,29 @@ func createTimeLimitCode(data string, minutes int, startInf interface{}) (string
|
||
|
|
||
|
// verify time limit code
|
||
|
func validateUserEmailCode(user *models.User, code string) (bool, error) {
|
||
|
- if len(code) <= 18 {
|
||
|
+ if len(code) < timeLimitCodeLength {
|
||
|
return false, nil
|
||
|
}
|
||
|
|
||
|
- minutes := setting.EmailCodeValidMinutes
|
||
|
code = code[:timeLimitCodeLength]
|
||
|
|
||
|
// split code
|
||
|
- start := code[:12]
|
||
|
- lives := code[12:18]
|
||
|
- if d, err := com.StrTo(lives).Int(); err == nil {
|
||
|
- minutes = d
|
||
|
+ startStr := code[:timeLimitStartDateLength]
|
||
|
+ minutesStr := code[timeLimitStartDateLength : timeLimitStartDateLength+timeLimitMinutesLength]
|
||
|
+ minutes, err := strconv.Atoi(minutesStr)
|
||
|
+ if err != nil {
|
||
|
+ return false, fmt.Errorf("invalid time limit code: %v", err)
|
||
|
}
|
||
|
|
||
|
- // right active code
|
||
|
- data := com.ToStr(user.Id) + user.Email + user.Login + user.Password + user.Rands
|
||
|
- retCode, err := createTimeLimitCode(data, minutes, start)
|
||
|
+ // verify code
|
||
|
+ payload := strconv.FormatInt(user.Id, 10) + user.Email + user.Login + user.Password + user.Rands
|
||
|
+ expectedCode, err := createTimeLimitCode(payload, minutes, startStr)
|
||
|
if err != nil {
|
||
|
return false, err
|
||
|
}
|
||
|
- fmt.Printf("code : %s\ncode2: %s", retCode, code)
|
||
|
- if retCode == code && minutes > 0 {
|
||
|
+ if hmac.Equal([]byte(code), []byte(expectedCode)) && minutes > 0 {
|
||
|
// check time is expired or not
|
||
|
- before, _ := time.ParseInLocation("200601021504", start, time.Local)
|
||
|
+ before, _ := time.ParseInLocation("200601021504", startStr, time.Local)
|
||
|
now := time.Now()
|
||
|
if before.Add(time.Minute*time.Duration(minutes)).Unix() > now.Unix() {
|
||
|
return true, nil
|
||
|
@@ -94,15 +98,15 @@ func getLoginForEmailCode(code string) string {
|
||
|
return string(b)
|
||
|
}
|
||
|
|
||
|
-func createUserEmailCode(u *models.User, startInf interface{}) (string, error) {
|
||
|
+func createUserEmailCode(user *models.User, startStr string) (string, error) {
|
||
|
minutes := setting.EmailCodeValidMinutes
|
||
|
- data := com.ToStr(u.Id) + u.Email + u.Login + u.Password + u.Rands
|
||
|
- code, err := createTimeLimitCode(data, minutes, startInf)
|
||
|
+ payload := strconv.FormatInt(user.Id, 10) + user.Email + user.Login + user.Password + user.Rands
|
||
|
+ code, err := createTimeLimitCode(payload, minutes, startStr)
|
||
|
if err != nil {
|
||
|
return "", err
|
||
|
}
|
||
|
|
||
|
// add tail hex username
|
||
|
- code += hex.EncodeToString([]byte(u.Login))
|
||
|
+ code += hex.EncodeToString([]byte(user.Login))
|
||
|
return code, nil
|
||
|
}
|
||
|
diff --git a/pkg/services/notifications/codes_test.go b/pkg/services/notifications/codes_test.go
|
||
|
index d2b1f3a617..bea88e0bf5 100644
|
||
|
--- a/pkg/services/notifications/codes_test.go
|
||
|
+++ b/pkg/services/notifications/codes_test.go
|
||
|
@@ -1,19 +1,129 @@
|
||
|
package notifications
|
||
|
|
||
|
import (
|
||
|
+ "fmt"
|
||
|
+ "strconv"
|
||
|
"testing"
|
||
|
+ "time"
|
||
|
|
||
|
"github.com/grafana/grafana/pkg/models"
|
||
|
"github.com/grafana/grafana/pkg/setting"
|
||
|
. "github.com/smartystreets/goconvey/convey"
|
||
|
+ "github.com/stretchr/testify/require"
|
||
|
)
|
||
|
|
||
|
+func TestTimeLimitCodes(t *testing.T) {
|
||
|
+ user := &models.User{Id: 10, Email: "t@a.com", Login: "asd", Password: "1", Rands: "2"}
|
||
|
+
|
||
|
+ format := "200601021504"
|
||
|
+ mailPayload := strconv.FormatInt(user.Id, 10) + user.Email + user.Login + user.Password + user.Rands
|
||
|
+ tenMinutesAgo := time.Now().Add(-time.Minute * 10)
|
||
|
+
|
||
|
+ tests := []struct {
|
||
|
+ desc string
|
||
|
+ payload string
|
||
|
+ start time.Time
|
||
|
+ minutes int
|
||
|
+ valid bool
|
||
|
+ }{
|
||
|
+ {
|
||
|
+ desc: "code generated 10 minutes ago, 5 minutes valid",
|
||
|
+ payload: mailPayload,
|
||
|
+ start: tenMinutesAgo,
|
||
|
+ minutes: 5,
|
||
|
+ valid: false,
|
||
|
+ },
|
||
|
+ {
|
||
|
+ desc: "code generated 10 minutes ago, 9 minutes valid",
|
||
|
+ payload: mailPayload,
|
||
|
+ start: tenMinutesAgo,
|
||
|
+ minutes: 9,
|
||
|
+ valid: false,
|
||
|
+ },
|
||
|
+ {
|
||
|
+ desc: "code generated 10 minutes ago, 10 minutes valid",
|
||
|
+ payload: mailPayload,
|
||
|
+ start: tenMinutesAgo,
|
||
|
+ minutes: 10,
|
||
|
+ // code was valid exactly 10 minutes since evaluating the tenMinutesAgo assignment
|
||
|
+ // by the time this test is run the code is already expired
|
||
|
+ valid: false,
|
||
|
+ },
|
||
|
+ {
|
||
|
+ desc: "code generated 10 minutes ago, 11 minutes valid",
|
||
|
+ payload: mailPayload,
|
||
|
+ start: tenMinutesAgo,
|
||
|
+ minutes: 11,
|
||
|
+ valid: true,
|
||
|
+ },
|
||
|
+ {
|
||
|
+ desc: "code generated 10 minutes ago, 20 minutes valid",
|
||
|
+ payload: mailPayload,
|
||
|
+ start: tenMinutesAgo,
|
||
|
+ minutes: 20,
|
||
|
+ valid: true,
|
||
|
+ },
|
||
|
+ {
|
||
|
+ desc: "code generated 10 minutes ago, 20 minutes valid, tampered payload",
|
||
|
+ payload: mailPayload[:len(mailPayload)-1] + "x",
|
||
|
+ start: tenMinutesAgo,
|
||
|
+ minutes: 20,
|
||
|
+ valid: false,
|
||
|
+ },
|
||
|
+ }
|
||
|
+
|
||
|
+ for _, test := range tests {
|
||
|
+ t.Run(test.desc, func(t *testing.T) {
|
||
|
+ code, err := createTimeLimitCode(test.payload, test.minutes, test.start.Format(format))
|
||
|
+ require.NoError(t, err)
|
||
|
+
|
||
|
+ isValid, err := validateUserEmailCode(user, code)
|
||
|
+ require.NoError(t, err)
|
||
|
+ require.Equal(t, test.valid, isValid)
|
||
|
+ })
|
||
|
+ }
|
||
|
+
|
||
|
+ t.Run("tampered minutes", func(t *testing.T) {
|
||
|
+ code, err := createTimeLimitCode(mailPayload, 5, tenMinutesAgo.Format(format))
|
||
|
+ require.NoError(t, err)
|
||
|
+
|
||
|
+ // code is expired
|
||
|
+ isValid, err := validateUserEmailCode(user, code)
|
||
|
+ require.NoError(t, err)
|
||
|
+ require.Equal(t, false, isValid)
|
||
|
+
|
||
|
+ // let's try to extend the code by tampering the minutes
|
||
|
+ code = code[:12] + fmt.Sprintf("%06d", 20) + code[18:]
|
||
|
+ isValid, err = validateUserEmailCode(user, code)
|
||
|
+ require.NoError(t, err)
|
||
|
+ require.Equal(t, false, isValid)
|
||
|
+ })
|
||
|
+
|
||
|
+ t.Run("tampered start string", func(t *testing.T) {
|
||
|
+ code, err := createTimeLimitCode(mailPayload, 5, tenMinutesAgo.Format(format))
|
||
|
+ require.NoError(t, err)
|
||
|
+
|
||
|
+ // code is expired
|
||
|
+ isValid, err := validateUserEmailCode(user, code)
|
||
|
+ require.NoError(t, err)
|
||
|
+ require.Equal(t, false, isValid)
|
||
|
+
|
||
|
+ // let's try to extend the code by tampering the start string
|
||
|
+ oneMinuteAgo := time.Now().Add(-time.Minute)
|
||
|
+
|
||
|
+ code = oneMinuteAgo.Format(format) + code[12:]
|
||
|
+ isValid, err = validateUserEmailCode(user, code)
|
||
|
+ require.NoError(t, err)
|
||
|
+ require.Equal(t, false, isValid)
|
||
|
+ })
|
||
|
+}
|
||
|
+
|
||
|
func TestEmailCodes(t *testing.T) {
|
||
|
Convey("When generating code", t, func() {
|
||
|
setting.EmailCodeValidMinutes = 120
|
||
|
|
||
|
user := &models.User{Id: 10, Email: "t@a.com", Login: "asd", Password: "1", Rands: "2"}
|
||
|
- code, err := createUserEmailCode(user, nil)
|
||
|
+ code, err := createUserEmailCode(user, "")
|
||
|
So(err, ShouldBeNil)
|
||
|
|
||
|
Convey("getLoginForCode should return login", func() {
|
||
|
@@ -27,7 +137,7 @@ func TestEmailCodes(t *testing.T) {
|
||
|
So(isValid, ShouldBeTrue)
|
||
|
})
|
||
|
|
||
|
- Convey("Cannot verify in-valid code", func() {
|
||
|
+ Convey("Cannot verify invalid code", func() {
|
||
|
code = "ASD"
|
||
|
isValid, err := validateUserEmailCode(user, code)
|
||
|
So(err, ShouldBeNil)
|
||
|
diff --git a/pkg/services/notifications/notifications.go b/pkg/services/notifications/notifications.go
|
||
|
index beea82f43e..5a575d1415 100644
|
||
|
--- a/pkg/services/notifications/notifications.go
|
||
|
+++ b/pkg/services/notifications/notifications.go
|
||
|
@@ -149,7 +149,7 @@ func (ns *NotificationService) sendEmailCommandHandler(cmd *models.SendEmailComm
|
||
|
}
|
||
|
|
||
|
func (ns *NotificationService) sendResetPasswordEmail(cmd *models.SendResetPasswordEmailCommand) error {
|
||
|
- code, err := createUserEmailCode(cmd.User, nil)
|
||
|
+ code, err := createUserEmailCode(cmd.User, "")
|
||
|
if err != nil {
|
||
|
return err
|
||
|
}
|
||
|
diff --git a/pkg/services/notifications/notifications_test.go b/pkg/services/notifications/notifications_test.go
|
||
|
index e7680c3943..fb73e332ea 100644
|
||
|
--- a/pkg/services/notifications/notifications_test.go
|
||
|
+++ b/pkg/services/notifications/notifications_test.go
|
||
|
@@ -1,12 +1,14 @@
|
||
|
package notifications
|
||
|
|
||
|
import (
|
||
|
+ "regexp"
|
||
|
"testing"
|
||
|
|
||
|
"github.com/grafana/grafana/pkg/bus"
|
||
|
"github.com/grafana/grafana/pkg/models"
|
||
|
"github.com/grafana/grafana/pkg/setting"
|
||
|
. "github.com/smartystreets/goconvey/convey"
|
||
|
+ "github.com/stretchr/testify/require"
|
||
|
)
|
||
|
|
||
|
func TestNotifications(t *testing.T) {
|
||
|
@@ -25,13 +27,28 @@ func TestNotifications(t *testing.T) {
|
||
|
So(err, ShouldBeNil)
|
||
|
|
||
|
Convey("When sending reset email password", func() {
|
||
|
- err := ns.sendResetPasswordEmail(&models.SendResetPasswordEmailCommand{User: &models.User{Email: "asd@asd.com"}})
|
||
|
+ user := models.User{Email: "asd@asd.com", Login: "asd@asd.com"}
|
||
|
+ err := ns.sendResetPasswordEmail(&models.SendResetPasswordEmailCommand{User: &user})
|
||
|
So(err, ShouldBeNil)
|
||
|
|
||
|
sentMsg := <-ns.mailQueue
|
||
|
So(sentMsg.Body, ShouldContainSubstring, "body")
|
||
|
So(sentMsg.Subject, ShouldEqual, "Reset your Grafana password - asd@asd.com")
|
||
|
So(sentMsg.Body, ShouldNotContainSubstring, "Subject")
|
||
|
+
|
||
|
+ // find code in mail
|
||
|
+ r, _ := regexp.Compile(`code=(\w+)`)
|
||
|
+ match := r.FindString(sentMsg.Body)
|
||
|
+ code := match[len("code="):]
|
||
|
+
|
||
|
+ // verify code
|
||
|
+ bus.AddHandler("test", func(query *models.GetUserByLoginQuery) error {
|
||
|
+ query.Result = &user
|
||
|
+ return nil
|
||
|
+ })
|
||
|
+ query := models.ValidateResetPasswordCodeQuery{Code: code}
|
||
|
+ err = ns.validateResetPasswordCode(&query)
|
||
|
+ require.NoError(t, err)
|
||
|
})
|
||
|
})
|
||
|
}
|