From 4905045f54c1517c117d28471f5b68ac79d640a3 Mon Sep 17 00:00:00 2001 From: Andreas Gerstmayr Date: Tue, 18 Jan 2022 17:03:36 +0100 Subject: [PATCH] use HMAC-SHA-256 instead of SHA-1 to generate password reset tokens update FIPS tests in check phase --- ...ac-sha-256-for-password-reset-tokens.patch | 353 ++++++++++++++++++ grafana.spec | 57 +-- 2 files changed, 362 insertions(+), 48 deletions(-) create mode 100644 012-use-hmac-sha-256-for-password-reset-tokens.patch diff --git a/012-use-hmac-sha-256-for-password-reset-tokens.patch b/012-use-hmac-sha-256-for-password-reset-tokens.patch new file mode 100644 index 0000000..91b6b46 --- /dev/null +++ b/012-use-hmac-sha-256-for-password-reset-tokens.patch @@ -0,0 +1,353 @@ +commit f13c08e9f45d7776cb264b17ec41bc4ff51fc0b9 +Author: Andreas Gerstmayr +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) + }) + }) + } diff --git a/grafana.spec b/grafana.spec index 9ce5a1a..c1d94ab 100644 --- a/grafana.spec +++ b/grafana.spec @@ -30,7 +30,7 @@ end} Name: grafana Version: 7.5.11 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Metrics dashboard and graph editor License: ASL 2.0 URL: https://grafana.org @@ -93,6 +93,8 @@ Patch10: 010-fips.patch Patch11: 011-CVE-2021-43813.patch +Patch12: 012-use-hmac-sha-256-for-password-reset-tokens.patch + # Intersection of go_arches and nodejs_arches ExclusiveArch: %{grafana_arches} @@ -495,6 +497,7 @@ rm -r plugins-bundled %patch10 -p1 %endif %patch11 -p1 +%patch12 -p1 # Set up build subdirs and links mkdir -p %{_builddir}/src/github.com/grafana @@ -629,53 +632,7 @@ rm -r pkg/macaron %gotest ./pkg/... %if %{enable_fips_mode} -# FIPS setup instructions lifted from golang.spec: -# https://gitlab.com/redhat/centos-stream/rpms/golang/-/blob/c9s/golang.spec - -TEST_BORING_CONFIGS=`mktemp -d` -TEST_BORING_CNF=$TEST_BORING_CONFIGS/openssl-boring.cnf -TEST_BORING_FIPS_CNF=$TEST_BORING_CONFIGS/fipsmodule.cnf -trap "rm -rf $TEST_BORING_CONFIGS" EXIT - -cp /etc/pki/tls/openssl.cnf $TEST_BORING_CNF -openssl fipsinstall -module /usr/lib64/ossl-modules/fips.so -out $TEST_BORING_FIPS_CNF - -cat > $TEST_BORING_CNF << EOM -openssl_conf = openssl_test - -[openssl_test] -providers = provider_test -alg_section = algorithm_test -ssl_conf = ssl_module - -[algorithm_test] -default_properties = fips=yes - -[provider_test] -default = default_sect - # The fips section name should match the section name inside the - # included fipsmodule.cnf. -fips = fips_sect -.include $TEST_BORING_FIPS_CNF - -[default_sect] -activate = 1 - -[ ssl_module ] - -system_default = crypto_policy - -[ crypto_policy ] - -.include = /etc/crypto-policies/back-ends/opensslcnf.config - -[ new_oids ] - -EOM - - -export OPENSSL_CONF=$TEST_BORING_CNF -GOLANG_FIPS=1 go test -v ./pkg/util -run TestEncryption +OPENSSL_FORCE_FIPS_MODE=1 GOLANG_FIPS=1 go test -v ./pkg/util -run TestEncryption %endif %files @@ -723,6 +680,10 @@ GOLANG_FIPS=1 go test -v ./pkg/util -run TestEncryption %changelog +* Tue Jan 18 2022 Andreas Gerstmayr 7.5.11-3 +- use HMAC-SHA-256 instead of SHA-1 to generate password reset tokens +- update FIPS tests in check phase + * Thu Dec 16 2021 Andreas Gerstmayr 7.5.11-2 - resolve CVE-2021-44716 golang: net/http: limit growth of header canonicalization cache - resolve CVE-2021-43813 grafana: directory traversal vulnerability for *.md files