use salted hashes for sensitive info

README.md

@@ -145,7 +145,7 @@ One of the biggest threats to the application is the unauthorized access to the
 
 The data at rest is planned to be stored in the Azure Table Storage. Azure Storage encrypts the data at rest with AES 256-bit encryption with the Azure managed keys. In addition to that all of the sensitive data is hashed or encrypted before being stored in the Table Storage with the encryption keys known to the server and administarators only.
 
-In the case of the breach the data is hashed and will soon use the salt to prevent the rainbow table attacks.
+In the case of the breach the data is hashed and salted using an OWASP recommended Argon2ID hashing algorithm to prevent the rainbow table attacks.
 
 Additional protection is in place where the attacker tries to guess the PIN to access the message. The message will be deleted after the number of failed attempts exceeds the threshold.
 
@@ -163,7 +163,7 @@ The GiHub repository subscribes to security notifications and the owner of the r
 
 ## Needs further improvement
 
-- Hashing needs to use salt to prevent rainbow table attacks
+- ~Hashing needs to use salt to prevent rainbow table attacks~
 - ~Secure key material need to be passed through the environment variables~
 - ~Encrypt the session cookie contents in addition to the HMAC~
 - Increase the size of the keys

go.mod

@@ -4,7 +4,10 @@ go 1.22
 
 require (
 	github.com/gorilla/sessions v1.2.2
-	golang.org/x/crypto v0.21.0
+	golang.org/x/crypto v0.22.0
 )
 
-require github.com/gorilla/securecookie v1.1.2 // indirect
+require (
+	github.com/gorilla/securecookie v1.1.2 // indirect
+	golang.org/x/sys v0.19.0 // indirect
+)

go.sum

@@ -4,5 +4,7 @@ github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kX
 github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo=
 github.com/gorilla/sessions v1.2.2 h1:lqzMYz6bOfvn2WriPUjNByzeXIlVzURcPmgMczkmTjY=
 github.com/gorilla/sessions v1.2.2/go.mod h1:ePLdVu+jbEgHH+KWw8I1z2wqd0BAdAQh/8LRvBeoNcQ=
-golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA=
-golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
+golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
+golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
+golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
+golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=

internal/storage/hash.go

@@ -5,43 +5,79 @@ import (
 	"crypto/cipher"
 	"crypto/rand"
 	"crypto/sha256"
+	"crypto/subtle"
 	"encoding/base64"
 	"encoding/binary"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"io"
+	"strings"
 
+	"golang.org/x/crypto/argon2"
 	"golang.org/x/crypto/hkdf"
 )
 
 const keySizeBytes = 16 // 128-bit key
 
-// simple text hashing
-func HashText(text string) string {
-	textHash := sha256.Sum256([]byte(text))
-	return hex.EncodeToString(textHash[:])
-}
-
-// simple pin generator
+// simple pin number generator
+// returns 4-5 digits
 func MakePin() (string, error) {
-	b := make([]byte, 16)
-	if _, err := rand.Read(b); err != nil {
+	b, err := generateRandomBytes(16)
+	if err != nil {
 		return "", err
 	}
 	pin := binary.BigEndian.Uint16(b)
 	return fmt.Sprintf("%d", pin), nil
 }
 
+// simple random token generator (url encoded)
 func MakeToken() (string, error) {
-	b := make([]byte, 32)
-	if _, err := rand.Read(b); err != nil {
+	b, err := generateRandomBytes(32)
+	if err != nil {
 		return "", err
 	}
 	return base64.URLEncoding.EncodeToString(b), nil
 }
 
-// use hkdf to derive a strong key (RFC5869)
-// maybe switch to bcrypt or scrypt
+// simple text hashing
+func HashText(text string) string {
+	textHash := sha256.Sum256([]byte(text))
+	return hex.EncodeToString(textHash[:])
+}
+
+// Uses argon2id to hash and salt the given clear text value
+// returns the encoded string to be used for storage
+func HashPass(password string) (string, error) {
+	// values suggested in the OWASP cheatsheet
+	params := &argon2Params{
+		memory:      12 * 1024, // 12mb
+		iterations:  3,
+		parallelism: 1,
+		saltLength:  16,
+		keyLength:   32,
+	}
+	return genArgon2Hash(password, params)
+}
+
+// Uses argon2id to compare the given encoded string to the given password
+func CompareHashToPass(hash, password string) error {
+	p, salt, key, err := decodeArgon2Hash(hash)
+	if err != nil {
+		return err
+	}
+	// try to recreate the key
+	otherKey := genArgon2Key(p, []byte(password), salt)
+	// compare but mitigate timing attacks
+	if subtle.ConstantTimeCompare(key, otherKey) != 1 {
+		return errors.New("invalid key")
+	}
+
+	return nil
+}
+
+// Use HMAC Key Derivation Function (HKDF) to derive a strong key (RFC5869)
+// This function is useful to derive a key from some small password text the user knows
 func StrongKey(passText string, saltText string) ([]byte, error) {
 	hashFn := sha256.New
 	hashSize := hashFn().Size()
@@ -58,7 +94,8 @@ func StrongKey(passText string, saltText string) ([]byte, error) {
 	return key, nil
 }
 
-// use strong key to encrypt text
+// Encrypts the plain text with a given key
+// see StrongKey() to create one
 func EncryptAES(key []byte, plaintext string) (string, error) {
 	cipherBlock, err := aes.NewCipher(key)
 	if err != nil {
@@ -84,6 +121,7 @@ func EncryptAES(key []byte, plaintext string) (string, error) {
 	return hex.EncodeToString(ciphertext), nil
 }
 
+// Decrypt the cipher text using the key provided
 func DecryptAES(key []byte, ct string) (string, error) {
 	ciphertext, _ := hex.DecodeString(ct)
 	cipherBlock, err := aes.NewCipher(key)
@@ -108,3 +146,84 @@ func DecryptAES(key []byte, ct string) (string, error) {
 
 	return string(plaintext), nil
 }
+
+const argonHashVariantName = "argon2id"
+
+// The parameters to be used in the secure hash generation
+// Please refer to the OWASP cheatsheet to see recommended values
+// https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id
+type argon2Params struct {
+	memory      uint32
+	iterations  uint32
+	parallelism uint8
+	saltLength  uint32
+	keyLength   uint32
+}
+
+// Generate a secure salted hash using Argon2id
+// OWASP https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id
+// Example https://github.com/alexedwards/argon2id
+func genArgon2Hash(password string, p *argon2Params) (string, error) {
+	salt, err := generateRandomBytes(p.saltLength)
+	if err != nil {
+		return "", err
+	}
+	key := genArgon2Key(p, []byte(password), salt)
+	return encodeArgon2Hash(p, salt, key), nil
+}
+
+// shortcut to generate the key
+func genArgon2Key(p *argon2Params, pass, salt []byte) []byte {
+	return argon2.IDKey(pass, salt, p.iterations, p.memory, p.parallelism, p.keyLength)
+}
+
+// encode argon2 derived key into a self-containing string for storage purposes
+func encodeArgon2Hash(params *argon2Params, salt, key []byte) string {
+	b64Salt := base64.RawStdEncoding.EncodeToString(salt)
+	b64Key := base64.RawStdEncoding.EncodeToString(key)
+	return fmt.Sprintf("$%s$v=%d$m=%d,t=%d,p=%d$%s$%s", argonHashVariantName, argon2.Version, params.memory, params.iterations, params.parallelism, b64Salt, b64Key)
+}
+
+// decode argon2 hash string into the parameters ready for comparison
+func decodeArgon2Hash(hash string) (params *argon2Params, salt, key []byte, err error) {
+	vals := strings.Split(hash, "$")
+	if len(vals) != 6 {
+		return nil, nil, nil, fmt.Errorf("unexpected hash structure with %d elements but should be %d", len(vals), 6)
+	}
+	if vals[1] != argonHashVariantName {
+		return nil, nil, nil, fmt.Errorf("unexpected hash function variant %s which should be %s", vals[1], argonHashVariantName)
+	}
+	var version int
+	_, err = fmt.Sscanf(vals[2], "v=%d", &version)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	if version != argon2.Version {
+		return nil, nil, nil, fmt.Errorf("argon2 version mismatch %d <> %d", version, argon2.Version)
+	}
+	params = &argon2Params{}
+	_, err = fmt.Sscanf(vals[3], "m=%d,t=%d,p=%d", &params.memory, &params.iterations, &params.parallelism)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	salt, err = base64.RawStdEncoding.Strict().DecodeString(vals[4])
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	params.saltLength = uint32(len(salt))
+	key, err = base64.RawStdEncoding.Strict().DecodeString(vals[5])
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	params.keyLength = uint32(len(key))
+	return params, salt, key, nil
+}
+
+// Generate cryptographically secure random of a given length
+func generateRandomBytes(size uint32) ([]byte, error) {
+	bucket := make([]byte, size)
+	if _, err := rand.Read(bucket); err != nil {
+		return nil, err
+	}
+	return bucket, nil
+}

internal/storage/hash_test.go

@@ -1,11 +1,45 @@
 package storage_test
 
 import (
+	"regexp"
+	"slices"
+	"strings"
 	"testing"
 
 	"github.com/ivarprudnikov/secretshare/internal/storage"
 )
 
+func TestHash_MakePin(t *testing.T) {
+	pin, err := storage.MakePin()
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+	if len(pin) < 4 || len(pin) > 5 {
+		t.Fatalf("unexpected pin lenght %s", pin)
+	}
+	re := regexp.MustCompile(`\d+`)
+	if !re.Match([]byte(pin)) {
+		t.Fatalf("unexpected pin %s", pin)
+	}
+}
+
+func TestHash_StrongKey(t *testing.T) {
+	salt := "12345678901234567890123456789012"
+	key1, err := storage.StrongKey("1234", salt)
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	key2, err := storage.StrongKey("1234", salt)
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	if slices.Compare(key1, key2) != 0 {
+		t.Fatalf("Keys must match %s %s", key1, key2)
+	}
+}
+
 func TestHash_EncryptDecrypt(t *testing.T) {
 	key := []byte("1234567890123456")
 	cipher, err := storage.EncryptAES(key, "abc")
@@ -30,3 +64,18 @@ func TestHash_HashText(t *testing.T) {
 		t.Fatalf("unexpected digest %s", digest)
 	}
 }
+
+func TestHash_HashPass_ThenCompare(t *testing.T) {
+	hashed, err := storage.HashPass("foobar")
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+	if !strings.HasPrefix(hashed, "$argon2id$v=") {
+		t.Fatalf("unexpected value %s", hashed)
+	}
+
+	err = storage.CompareHashToPass(hashed, "foobar")
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+}

internal/storage/messages.go

@@ -39,6 +39,7 @@ func (s *memMessageStore) Encrypt(text string, pass string) (string, error) {
 	return ciphertext, nil
 }
 
+// Decrypt cipher text with a given PIN which will be used to derive a key
 func (s *memMessageStore) Decrypt(ciphertext string, pass string) (string, error) {
 	// derive a key from the pass
 	key, err := StrongKey(pass, s.salt)
@@ -75,7 +76,10 @@ func (s *memMessageStore) AddMessage(text string, username string) (Message, err
 		return Message{}, err
 	}
 	ciphertext, err := s.Encrypt(text, pin)
-	msg := NewMessage(username, ciphertext, pin)
+	if err != nil {
+		return Message{}, err
+	}
+	msg, err := NewMessage(username, ciphertext, pin)
 	if err != nil {
 		return Message{}, err
 	}
@@ -103,8 +107,8 @@ func (s *memMessageStore) GetMessage(id string) (*Message, error) {
 func (s *memMessageStore) GetFullMessage(id string, pin string) (*Message, error) {
 	if v, ok := s.messages.Load(id); ok {
 		if msg, ok := v.(Message); ok {
-			// TODO use salted hash
-			if msg.Pin == HashText(pin) {
+
+			if err := CompareHashToPass(msg.Pin, pin); err == nil {
 
 				text, err := s.Decrypt(msg.Content, pin)
 				if err != nil {
@@ -149,14 +153,17 @@ type Message struct {
 	Attempt  int       `json:"Attempt,omitempty"`
 }
 
-func NewMessage(username string, content string, pin string) Message {
+func NewMessage(username string, content string, pin string) (Message, error) {
+	pinHash, err := HashPass(pin)
+	if err != nil {
+		return Message{}, err
+	}
 	return Message{
 		Username: username,
 		Content:  content,
 		Digest:   HashText(content),
 		Created:  time.Now(),
-		// TODO use salt for hashing to mitigate rainbow attacks
-		Pin:     HashText(pin),
-		Attempt: 0,
-	}
+		Pin:      pinHash,
+		Attempt:  0,
+	}, nil
 }

internal/storage/users.go

@@ -25,7 +25,10 @@ func (u *memUserStore) AddUser(username string, password string) (User, error) {
 	if _, ok := u.users.Load(username); ok {
 		return User{}, errors.New("username is not available")
 	}
-	usr := NewUser(username, password)
+	usr, err := NewUser(username, password)
+	if err != nil {
+		return User{}, err
+	}
 	u.users.Store(usr.Username, usr)
 	return usr, nil
 }
@@ -45,8 +48,7 @@ func (u *memUserStore) GetUser(username string) (*User, error) {
 func (u *memUserStore) GetUserWithPass(username string, password string) (*User, error) {
 	if v, ok := u.users.Load(username); ok {
 		if usr, ok := v.(User); ok {
-			// TODO use salted hash
-			if usr.Password == HashText(password) {
+			if err := CompareHashToPass(usr.Password, password); err == nil {
 				return &User{
 					Username: usr.Username,
 					Created:  usr.Created,
@@ -64,11 +66,14 @@ type User struct {
 	Created  time.Time `json:"created"`
 }
 
-func NewUser(username string, password string) User {
+func NewUser(username string, password string) (User, error) {
+	hashedPass, err := HashPass(password)
+	if err != nil {
+		return User{}, err
+	}
 	return User{
 		Username: username,
-		// TODO use salt for hashing to mitigate rainbow attacks
-		Password: HashText(password),
+		Password: hashedPass,
 		Created:  time.Now(),
-	}
+	}, nil
 }