From b06bd51d219ca6720b4e5c91510b09fd8b3e0989 Mon Sep 17 00:00:00 2001
From: eikendev <raphael@eiken.dev>
Date: Sat, 16 Jan 2021 15:29:04 +0100
Subject: [PATCH] Add option to check for weak passwords

---
 api/interfaces.go                         |  2 +-
 api/user.go                               |  7 ++-
 app.go                                    |  2 +-
 authentication/credentials/credentials.go |  8 ++-
 authentication/credentials/hibp.go        | 61 +++++++++++++++++++++++
 authentication/credentials/hibp_test.go   | 23 +++++++++
 authentication/credentials/password.go    | 14 +++++-
 config.example.yml                        |  4 ++
 configuration/configuration.go            |  3 ++
 database/database.go                      |  5 +-
 database/user.go                          |  5 +-
 model/user.go                             | 22 +++++---
 12 files changed, 141 insertions(+), 15 deletions(-)
 create mode 100644 authentication/credentials/hibp.go
 create mode 100644 authentication/credentials/hibp_test.go

diff --git a/api/interfaces.go b/api/interfaces.go
index 6f465d4..090390c 100644
--- a/api/interfaces.go
+++ b/api/interfaces.go
@@ -32,5 +32,5 @@ type Dispatcher interface {
 
 // The CredentialsManager interface for updating credentials.
 type CredentialsManager interface {
-	CreatePasswordHash(password string) []byte
+	CreatePasswordHash(password string) ([]byte, error)
 }
diff --git a/api/user.go b/api/user.go
index 3a59ac4..4b1852b 100644
--- a/api/user.go
+++ b/api/user.go
@@ -90,7 +90,12 @@ func (h *UserHandler) updateUser(ctx *gin.Context, u *model.User, updateUser mod
 		u.Name = *updateUser.Name
 	}
 	if updateUser.Password != nil {
-		u.PasswordHash = h.CM.CreatePasswordHash(*updateUser.Password)
+		hash, err := h.CM.CreatePasswordHash(*updateUser.Password)
+		if success := successOrAbort(ctx, http.StatusBadRequest, err); !success {
+			return err
+		}
+
+		u.PasswordHash = hash
 	}
 	if updateUser.MatrixID != nil {
 		u.MatrixID = *updateUser.MatrixID
diff --git a/app.go b/app.go
index d90035f..98b1ca8 100644
--- a/app.go
+++ b/app.go
@@ -35,7 +35,7 @@ func main() {
 		log.Printf("%+v\n", c)
 	}
 
-	cm := credentials.CreateManager(c.Crypto)
+	cm := credentials.CreateManager(c.Security.CheckHIBP, c.Crypto)
 
 	db, err := database.Create(cm, c.Database.Dialect, c.Database.Connection)
 	if err != nil {
diff --git a/authentication/credentials/credentials.go b/authentication/credentials/credentials.go
index 4174a18..eda3e7c 100644
--- a/authentication/credentials/credentials.go
+++ b/authentication/credentials/credentials.go
@@ -10,11 +10,12 @@ import (
 
 // Manager holds information for managing credentials.
 type Manager struct {
+	checkHIBP    bool
 	argon2Params *argon2id.Params
 }
 
 // CreateManager instanciates a credential manager.
-func CreateManager(c configuration.CryptoConfig) *Manager {
+func CreateManager(checkHIBP bool, c configuration.CryptoConfig) *Manager {
 	log.Println("Setting up credential manager.")
 
 	argon2Params := &argon2id.Params{
@@ -25,5 +26,8 @@ func CreateManager(c configuration.CryptoConfig) *Manager {
 		KeyLength:   c.Argon2.KeyLength,
 	}
 
-	return &Manager{argon2Params: argon2Params}
+	return &Manager{
+		checkHIBP:    checkHIBP,
+		argon2Params: argon2Params,
+	}
 }
diff --git a/authentication/credentials/hibp.go b/authentication/credentials/hibp.go
new file mode 100644
index 0000000..5836126
--- /dev/null
+++ b/authentication/credentials/hibp.go
@@ -0,0 +1,61 @@
+package credentials
+
+import (
+	"crypto/sha1"
+	"fmt"
+	"io/ioutil"
+	"log"
+	"net/http"
+	"strings"
+)
+
+const (
+	base                = "https://api.pwnedpasswords.com"
+	pwnedHashesEndpoint = "/range"
+	pwnedHashesURL      = base + pwnedHashesEndpoint + "/"
+)
+
+// IsPasswordPwned determines whether or not the password is weak.
+func IsPasswordPwned(password string) (bool, error) {
+	if len(password) == 0 {
+		return true, nil
+	}
+
+	hash := sha1.Sum([]byte(password))
+	hashStr := fmt.Sprintf("%X", hash)
+	lookup := hashStr[0:5]
+	match := hashStr[5:]
+
+	log.Printf("Checking HIBP for hashes starting with '%s'.\n", lookup)
+
+	resp, err := http.Get(pwnedHashesURL + lookup)
+	if err != nil {
+		return false, err
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		log.Fatalf("Request failed with HTTP %s.", resp.Status)
+	}
+
+	defer resp.Body.Close()
+	bodyText, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	bodyStr := string(bodyText)
+	lines := strings.Split(bodyStr, "\n")
+
+	for _, line := range lines {
+		separated := strings.Split(line, ":")
+		if len(separated) != 2 {
+			return false, fmt.Errorf("HIPB API returned malformed response: %s", line)
+		}
+
+		if separated[0] == match {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}
diff --git a/authentication/credentials/hibp_test.go b/authentication/credentials/hibp_test.go
new file mode 100644
index 0000000..c3db450
--- /dev/null
+++ b/authentication/credentials/hibp_test.go
@@ -0,0 +1,23 @@
+package credentials
+
+import "testing"
+
+type isPasswordPwnedTest struct {
+	arg  string
+	exp1 bool
+	exp2 error
+}
+
+var isPasswordPwnedTests = []isPasswordPwnedTest{
+	{"", true, nil},
+	{"password", true, nil},
+	{"2y6bWMETuHpNP08HCZq00QAAzE6nmwEb", false, nil},
+}
+
+func TestIsPasswordPwned(t *testing.T) {
+	for _, test := range isPasswordPwnedTests {
+		if out1, out2 := IsPasswordPwned(test.arg); out1 != test.exp1 || out2 != test.exp2 {
+			t.Errorf("Output (%t,%q) not equal to expected (%t,%q)", out1, out2, test.exp1, test.exp2)
+		}
+	}
+}
diff --git a/authentication/credentials/password.go b/authentication/credentials/password.go
index b2896b4..729838a 100644
--- a/authentication/credentials/password.go
+++ b/authentication/credentials/password.go
@@ -1,13 +1,23 @@
 package credentials
 
 import (
+	"errors"
 	"log"
 
 	"github.com/alexedwards/argon2id"
 )
 
 // CreatePasswordHash returns a hashed version of the given password.
-func (m *Manager) CreatePasswordHash(password string) []byte {
+func (m *Manager) CreatePasswordHash(password string) ([]byte, error) {
+	if m.checkHIBP {
+		pwned, err := IsPasswordPwned(password)
+		if err != nil {
+			return []byte{}, errors.New("HIBP is not available, please wait until service is available again")
+		} else if pwned {
+			return []byte{}, errors.New("Password is pwned, please choose another one")
+		}
+	}
+
 	hash, err := argon2id.CreateHash(password, m.argon2Params)
 
 	if err != nil {
@@ -15,7 +25,7 @@ func (m *Manager) CreatePasswordHash(password string) []byte {
 		panic(err)
 	}
 
-	return []byte(hash)
+	return []byte(hash), nil
 }
 
 // ComparePassword compares a hashed password with its possible plaintext equivalent.
diff --git a/config.example.yml b/config.example.yml
index 7de98cc..f67ed72 100644
--- a/config.example.yml
+++ b/config.example.yml
@@ -44,6 +44,10 @@ matrix:
     # [required]
     password: ''
 
+security:
+    # Wether or not to check for weak passwords using HIBP.
+    checkhibp: false
+
 crypto:
     # Configuration of the KDF for password storage. Do not change unless you know what you are doing!
     argon2:
diff --git a/configuration/configuration.go b/configuration/configuration.go
index b7c43ca..b74e7f9 100644
--- a/configuration/configuration.go
+++ b/configuration/configuration.go
@@ -39,6 +39,9 @@ type Configuration struct {
 		Username   string `required:"true"`
 		Password   string `required:"true"`
 	}
+	Security struct {
+		CheckHIBP bool `default:"false"`
+	}
 	Crypto CryptoConfig
 }
 
diff --git a/database/database.go b/database/database.go
index 5953ad4..bc56d38 100644
--- a/database/database.go
+++ b/database/database.go
@@ -84,7 +84,10 @@ func (d *Database) Populate(name, password, matrixID string) error {
 	query := d.gormdb.Where("name = ?", name).First(&user)
 
 	if errors.Is(query.Error, gorm.ErrRecordNotFound) {
-		user := model.NewUser(d.credentialsManager, name, password, true, matrixID)
+		user, err := model.NewUser(d.credentialsManager, name, password, true, matrixID)
+		if err != nil {
+			log.Fatal(err)
+		}
 
 		if err := d.gormdb.Create(&user).Error; err != nil {
 			return errors.New("user cannot be created")
diff --git a/database/user.go b/database/user.go
index 23861f0..57fc674 100644
--- a/database/user.go
+++ b/database/user.go
@@ -11,7 +11,10 @@ import (
 
 // CreateUser creates a user.
 func (d *Database) CreateUser(createUser model.CreateUser) (*model.User, error) {
-	user := createUser.IntoInternalUser(d.credentialsManager)
+	user, err := createUser.IntoInternalUser(d.credentialsManager)
+	if err != nil {
+		return nil, err
+	}
 
 	return user, d.gormdb.Create(user).Error
 }
diff --git a/model/user.go b/model/user.go
index c2bd7c7..e984672 100644
--- a/model/user.go
+++ b/model/user.go
@@ -36,25 +36,35 @@ type CreateUser struct {
 }
 
 // NewUser creates a new user.
-func NewUser(cm *credentials.Manager, name, password string, isAdmin bool, matrixID string) *User {
+func NewUser(cm *credentials.Manager, name, password string, isAdmin bool, matrixID string) (*User, error) {
 	log.Printf("Creating user %s.\n", name)
 
+	passwordHash, err := cm.CreatePasswordHash(password)
+	if err != nil {
+		return nil, err
+	}
+
 	return &User{
 		Name:         name,
-		PasswordHash: cm.CreatePasswordHash(password),
+		PasswordHash: passwordHash,
 		IsAdmin:      isAdmin,
 		MatrixID:     matrixID,
-	}
+	}, nil
 }
 
 // IntoInternalUser converts a CreateUser into a User.
-func (u *CreateUser) IntoInternalUser(cm *credentials.Manager) *User {
+func (u *CreateUser) IntoInternalUser(cm *credentials.Manager) (*User, error) {
+	passwordHash, err := cm.CreatePasswordHash(u.Password)
+	if err != nil {
+		return nil, err
+	}
+
 	return &User{
 		Name:         u.Name,
-		PasswordHash: cm.CreatePasswordHash(u.Password),
+		PasswordHash: passwordHash,
 		IsAdmin:      u.IsAdmin,
 		MatrixID:     u.MatrixID,
-	}
+	}, nil
 }
 
 // IntoExternalUser converts a User into a ExternalUser.