Refactor hashing: add support for bcrypt and argond2id hashers.

Fix cache security issue.

.github/CODEOWNERS

@@ -0,0 +1,8 @@
+# This is a comment.
+# Each line is a file pattern followed by one or more owners.
+
+# These owners will be the default owners for everything in
+# the repo. Unless a later match takes precedence,
+# @global-owner1 and @global-owner2 will be requested for
+# review when someone opens a pull request.
+*       @iegomez

Makefile

@@ -8,7 +8,7 @@ dev-requirements:
 	go get -u github.com/smartystreets/goconvey
 
 test:
-	go test ./backends ./cache -v -bench=none -count=1
+	go test ./backends ./cache ./hashing -v -bench=none -count=1
 
 benchmark:
 	go test ./backends -v -bench=. -run=^a

README.md

@@ -35,6 +35,7 @@ Please open an issue with the `feature` or `enhancement` tag to request new back
 - [Configuration](#configuration)
 	- [General options](#general-options)
 	- [Cache](#cache)
+	- [Hashing](#hashing)
 	- [Log level](#log-level)
 	- [Prefixes](#prefixes)
 	- [Backend options](#backend-options)
@@ -278,6 +279,61 @@ auth_opt_cache_addresses host1:port1,host2:port2,host3:port3
 
 Notice that if `cache_mode` is not provided or isn't equal to `cluster`, cache will default to use a single instance with the common options. If instead the mode is set to `cluster` but no addresses are given, the plugin will default to not use a cache.
 
+#### Hashing
+
+There are 3 options for password hashing available: `PBKDF2` (default), `Bcrypt` and `Argon2ID`. Every backend that needs one -that's all but `grpc`, `http` and `custom`- gets a hasher and whether it uses specific options or general ones depends on the auth opts passed.
+
+Provided options define what hasher each backend will use:
+- If there are general hashing options available but no backend ones, then every backend will use those general ones for its hasher.
+- If there are no options available in general and none for a given backend either, that backend will use defaults (see `hashing/hashing.go` for default values).
+- If there are options for a given backend but no general ones, the backend will use its own hasher and any backend that doesn't register a hasher will use defaults.
+
+You may set the desired general hasher with this option, passing either `pbkdf2`, `bcrypt` or `argon2id` values. When not set, the option will default to `pbkdf2`.
+
+```
+auth_opt_hasher pbkdf2
+
+```
+
+Each hasher has specific options. Notice that when using the `pw` utility, these values must match those used to generate the password.
+
+##### PBKDF2
+
+```
+auth_opt_salt_size 16           # salt bytes length
+auth_opt_iterations 100000      # number of iterations
+auth_opt_keylen 64              # key length
+auth_opt_algorithm sha512       # hashing algorithm, either sha512 (default) or sha256
+auth_opt_salt_encoding          # salt encoding, either base64 (default) or utf-8
+```
+
+##### Bcrypt
+
+```
+auth_opt_cost 10                # key expansion iteration count
+```
+
+##### Argon2ID
+
+```
+auth_opt_salt_size 16           # salt bytes length
+auth_opt_iterations 3           # number of iterations
+auth_opt_keylen 64              # key length
+auth_opt_memory 4096            # amount of memory (in kibibytes) to use
+auth_opt_parallelism            # degree of parallelism (i.e. number of threads)
+```
+
+**These options may be defined for each backend that needs a hasher by prepending the backend's name to the option, e.g. for setting `argon2id` as `Postgres'` hasher**:
+
+```
+auth_opt_pg_hasher argon2id
+auth_opt_pg_salt_size 16           # salt bytes length
+auth_opt_pg_iterations 3           # number of iterations
+auth_opt_pg_keylen 64              # key length
+auth_opt_pg_memory 4096            # amount of memory (in kibibytes) to use
+auth_opt_pg_parallelism            # degree of parallelism (i.e. number of threads)
+```
+
 #### Logging
 
 You can set the log level with the `log_level` option. Valid values are: debug, info, warn, error, fatal and panic. If not set, default value is `info`.
@@ -319,6 +375,38 @@ auth_opt_disable_superuser true
 
 Any other value or missing option will have `superuser` enabled.
 
+#### ACL access values
+
+Mosquitto 1.5. introduced a new ACL access value, `MOSQ_ACL_SUBSCRIBE`, which is similar to the classic `MOSQ_ACL_READ` value but not quite the same:
+
+```
+ *  MOSQ_ACL_SUBSCRIBE when a client is asking to subscribe to a topic string.
+ *                     This differs from MOSQ_ACL_READ in that it allows you to
+ *                     deny access to topic strings rather than by pattern. For
+ *                     example, you may use MOSQ_ACL_SUBSCRIBE to deny
+ *                     subscriptions to '#', but allow all topics in
+ *                     MOSQ_ACL_READ. This allows clients to subscribe to any
+ *                     topic they want, but not discover what topics are in use
+ *                     on the server.
+ *  MOSQ_ACL_READ      when a message is about to be sent to a client (i.e. whether
+ *                     it can read that topic or not).
+```
+
+The main difference is that subscribe is checked at first, when a client connects and tells the broker it wants to subscribe to some topic, while read is checked when an actual message is being published to that topic, which makes it particular.
+So in practice you could deny general subscriptions such as # by returning false from the acl check when you receive `MOSQ_ACL_SUBSCRIBE`, but allow any particular one by returning true on `MOSQ_ACL_READ`.
+Please take this into consideration when designing your ACL records on every backend.
+
+Also, these are the current available values from `mosquitto`:
+
+```
+#define MOSQ_ACL_NONE 0x00
+#define MOSQ_ACL_READ 0x01
+#define MOSQ_ACL_WRITE 0x02
+#define MOSQ_ACL_SUBSCRIBE 0x04
+```
+
+If you're using prior versions then `MOSQ_ACL_SUBSCRIBE` is not available and you don't need to worry about it.
+
 #### Backend options
 
 Any other options with a leading ```auth_opt_``` are handed to the plugin and used by the backends.
@@ -333,20 +421,33 @@ This issue captures these concerns and a basic plan to refactor tests: https://g
 
 ### Files
 
-The `files` backend implements the regular password and acl checks as described in mosquitto. Passwords should be in PBKDF2 format (for other backends too), and may be generated using the `pw` utility (built by default when running `make`) included in the plugin (or one of your own). Passwords may also be tested using the [pw-test package](https://github.com/iegomez/pw-test).
+The `files` backend implements the regular password and acl checks as described in mosquitto. Passwords should be in `PBKDF2`, `Bcrypt` or `Argon2ID` format (for other backends too), see [Hashing](#hashing) for more details about different hashing strategies. Hashes may be generated using the `pw` utility (built by default when running `make`) included in the plugin (or one of your own). Passwords may also be tested using the [pw-test package](https://github.com/iegomez/pw-test).
 
 Usage of `pw`:
 
 ```
 Usage of ./pw:
   -a string
     	algorithm: sha256 or sha512 (default "sha512")
+  -c int
+    	bcrypt ost param (default 10)
+  -e string
+    	salt encoding (default "base64")
+  -h string
+    	hasher: pbkdf2, argon2 or bcrypt (default "pbkdf2")
   -i int
-    	hash iterations (default 100000)
+    	hash iterations: defaults to 100000 for pbkdf2, please set to a reasonable value for argon2 (default 100000)
+  -l int
+    	key length, recommended values are 32 for sha256 and 64 for sha512
+  -m int
+    	memory for argon2 hash (default 4096)
   -p string
     	password
+  -pl int
+    	parallelism for argon2 (default 2)
   -s int
     	salt size (default 16)
+
 ```
 
 For this backend passwords and acls file paths must be given:
@@ -455,7 +556,7 @@ Queries work pretty much the same as in jpmen's plugin, so here's his discriptio
 	In the following example, the table has a column `rw` containing 1 for
 	readonly topics, 2 for writeonly topics and 3 for readwrite topics:
 
-	SELECT topic FROM acl WHERE (username = $1) AND (rw = $2 or rw = 3) 
+	SELECT topic FROM acl WHERE (username = $1) AND rw = $2 
 
 
 When option pg_superquery is not present, Superuser check will always return false, hence there'll be no superusers.
@@ -476,6 +577,9 @@ auth_opt_pg_aclquery select distinct 'application/' || a.id || '/#' from "user"
 
 ```
 
+#### Password hashing
+
+For instructions on how to set a backend specific hasher or use the general one, see [Hashing](#hashing).
 
 #### Testing Postgres
 
@@ -527,7 +631,7 @@ To allow native passwords, set the option to true:
 auth_opt_mysql_allow_native_passwords true
 ```
 
-Finally, placeholders for mysql differ from those of postgres, changing from $1, $2, etc., to simply ?. So, following the postgres examples, same queries for mysql would look like these:
+Finally, placeholders for mysql differ from those of postgres, changing from $1, $2, etc., to simply ?. These are some **example** queries for `mysql`:
 
 User query:
 
@@ -545,9 +649,12 @@ SELECT COUNT(*) FROM account WHERE username = ? AND super = 1
 Acl query:
 
 ```sql
-SELECT topic FROM acl WHERE (username = ?) AND rw >= ?
+SELECT topic FROM acl WHERE (username = ?) AND rw = ?
 ```
 
+#### Password hashing
+
+For instructions on how to set a backend specific hasher or use the general one, see [Hashing](#hashing).
 
 #### Testing Mysql
 
@@ -616,6 +723,9 @@ sqlite_superquery SELECT COUNT(*) FROM account WHERE username = ? AND super = 1
 sqlite_aclquery SELECT topic FROM acl WHERE (username = ?) AND rw >= ?
 ```
 
+#### Password hashing
+
+For instructions on how to set a backend specific hasher or use the general one, see [Hashing](#hashing).
 
 #### Testing SQLite3
 
@@ -813,6 +923,9 @@ When option jwt_superquery is not present, Superuser check will always return fa
 
 When option jwt_aclquery is not present, AclCheck will always return true, hence all authenticated users will be authorized to pub/sub to any topic.
 
+#### Password hashing
+
+When using local mode, a hasher is expected. For instructions on how to set a backend specific hasher or use the general one, see [Hashing](#hashing).
 
 #### Prefixes
 
@@ -911,6 +1024,10 @@ When not present, host defaults to "localhost", port to 6379, db to 2 and no pas
 If you want to use a Redis Cluster as your backend, you need to set `auth_opt_redis_mode` to `cluster` and provide the different addresses as a list of comma separated `host:port` strings with the `auth_opt_redis_addresses` options.
 If `auth_opt_redis_mode` is set to another value or not set, Redis defaults to single instance behaviour. If it is correctly set but no addresses are given, the backend will fail to initialize.
 
+#### Password hashing
+
+For instructions on how to set a backend specific hasher or use the general one, see [Hashing](#hashing).
+
 #### Testing Redis
 
 In order to test the Redis backend, the plugin needs to be able to connect to a redis server located at localhost, on port 6379, without using password and that a database named 2  exists (to avoid messing with the commonly used 0 and 1). 
@@ -990,6 +1107,10 @@ When not set, these options default to:
 
 If you experience any problem connecting to a replica set, please refer to [this issue](https://github.com/iegomez/mosquitto-go-auth/issues/32).
 
+#### Password hashing
+
+For instructions on how to set a backend specific hasher or use the general one, see [Hashing](#hashing).
+
 #### Testing MongoDB
 
 Much like `redis`, to test this backend the plugin needs to be able to connect to a mongodb server located at localhost, on port 27017, without using username or password. 

backends/db.go

@@ -0,0 +1,31 @@
+package backends
+
+import (
+	"time"
+
+	"github.com/jmoiron/sqlx"
+	"github.com/pkg/errors"
+	log "github.com/sirupsen/logrus"
+)
+
+// OpenDatabase opens the database and performs a ping to make sure the
+// database is up.
+// Taken from brocaar's lora-app-server: https://github.com/brocaar/lora-app-server
+func OpenDatabase(dsn, engine string) (*sqlx.DB, error) {
+
+	db, err := sqlx.Open(engine, dsn)
+	if err != nil {
+		return nil, errors.Wrap(err, "database connection error")
+	}
+
+	for {
+		if err = db.Ping(); err != nil {
+			log.Errorf("ping database error, will retry in 2s: %s", err)
+			time.Sleep(2 * time.Second)
+		} else {
+			break
+		}
+	}
+
+	return db, nil
+}

backends/files.go

@@ -6,14 +6,11 @@ import (
 	"os"
 	"strings"
 
-	"github.com/iegomez/mosquitto-go-auth/common"
+	"github.com/iegomez/mosquitto-go-auth/hashing"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
 )
 
-// hashIterations defines the number of hash iterations.
-var hashIterations = 100000
-
 //FileUer keeps a user password and acl records.
 type FileUser struct {
 	Password   string
@@ -30,15 +27,15 @@ type AclRecord struct {
 type Files struct {
 	PasswordPath string
 	AclPath      string
-	SaltEncoding string
 	CheckAcls    bool
 	Users        map[string]*FileUser //Users keeps a registry of username/FileUser pairs, holding a user's password and Acl records.
 	AclRecords   []AclRecord
 	filesOnly    bool
+	hasher       hashing.HashComparer
 }
 
 //NewFiles initializes a files backend.
-func NewFiles(authOpts map[string]string, logLevel log.Level) (Files, error) {
+func NewFiles(authOpts map[string]string, logLevel log.Level, hasher hashing.HashComparer) (Files, error) {
 
 	log.SetLevel(logLevel)
 
@@ -48,8 +45,8 @@ func NewFiles(authOpts map[string]string, logLevel log.Level) (Files, error) {
 		CheckAcls:    false,
 		Users:        make(map[string]*FileUser),
 		AclRecords:   make([]AclRecord, 0),
-		SaltEncoding: "base64",
 		filesOnly:    true,
+		hasher:       hasher,
 	}
 
 	if len(strings.Split(strings.Replace(authOpts["backends"], " ", "", -1), ",")) > 1 {
@@ -62,16 +59,6 @@ func NewFiles(authOpts map[string]string, logLevel log.Level) (Files, error) {
 		return files, errors.New("Files backend error: no password path given")
 	}
 
-	if saltEncoding, ok := authOpts["salt_encoding"]; ok {
-		switch saltEncoding {
-		case common.Base64, common.UTF8:
-			files.SaltEncoding = saltEncoding
-			log.Debugf("files backend: set salt encoding to: %s", saltEncoding)
-		default:
-			log.Errorf("files backend: invalid salt encoding specified: %s, will default to base64 instead", saltEncoding)
-		}
-	}
-
 	if aclPath, ok := authOpts["acl_path"]; ok {
 		files.AclPath = aclPath
 		files.CheckAcls = true
@@ -306,7 +293,7 @@ func (o Files) GetUser(username, password, clientid string) bool {
 		return false
 	}
 
-	if common.HashCompare(password, fileUser.Password, o.SaltEncoding) {
+	if o.hasher.Compare(password, fileUser.Password) {
 		return true
 	}
 
@@ -334,7 +321,7 @@ func (o Files) CheckAcl(username, topic, clientid string, acc int32) bool {
 	//If user exists, check against his acls and common ones. If not, check against common acls only.
 	if ok {
 		for _, aclRecord := range fileUser.AclRecords {
-			if common.TopicsMatch(aclRecord.Topic, topic) && (acc == int32(aclRecord.Acc) || int32(aclRecord.Acc) == MOSQ_ACL_READWRITE || (acc == MOSQ_ACL_SUBSCRIBE && topic != "#" && (int32(aclRecord.Acc) == MOSQ_ACL_READ || int32(aclRecord.Acc) == MOSQ_ACL_SUBSCRIBE))) {
+			if TopicsMatch(aclRecord.Topic, topic) && (acc == int32(aclRecord.Acc) || int32(aclRecord.Acc) == MOSQ_ACL_READWRITE || (acc == MOSQ_ACL_SUBSCRIBE && topic != "#" && (int32(aclRecord.Acc) == MOSQ_ACL_READ || int32(aclRecord.Acc) == MOSQ_ACL_SUBSCRIBE))) {
 				return true
 			}
 		}
@@ -343,7 +330,7 @@ func (o Files) CheckAcl(username, topic, clientid string, acc int32) bool {
 		//Replace all occurrences of %c for clientid and %u for username
 		aclTopic := strings.Replace(aclRecord.Topic, "%c", clientid, -1)
 		aclTopic = strings.Replace(aclTopic, "%u", username, -1)
-		if common.TopicsMatch(aclTopic, topic) && (acc == int32(aclRecord.Acc) || int32(aclRecord.Acc) == MOSQ_ACL_READWRITE || (acc == MOSQ_ACL_SUBSCRIBE && topic != "#" && (int32(aclRecord.Acc) == MOSQ_ACL_READ || int32(aclRecord.Acc) == MOSQ_ACL_SUBSCRIBE))) {
+		if TopicsMatch(aclTopic, topic) && (acc == int32(aclRecord.Acc) || int32(aclRecord.Acc) == MOSQ_ACL_READWRITE || (acc == MOSQ_ACL_SUBSCRIBE && topic != "#" && (int32(aclRecord.Acc) == MOSQ_ACL_READ || int32(aclRecord.Acc) == MOSQ_ACL_SUBSCRIBE))) {
 			return true
 		}
 	}

backends/files_test.go

@@ -4,6 +4,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/iegomez/mosquitto-go-auth/hashing"
 	log "github.com/sirupsen/logrus"
 	. "github.com/smartystreets/goconvey/convey"
 )
@@ -14,7 +15,7 @@ func TestFiles(t *testing.T) {
 	authOpts := make(map[string]string)
 
 	Convey("Given empty opts NewFiles should fail", t, func() {
-		_, err := NewFiles(authOpts, log.DebugLevel)
+		_, err := NewFiles(authOpts, log.DebugLevel, hashing.NewHasher(authOpts, "files"))
 		So(err, ShouldBeError)
 	})
 
@@ -25,7 +26,7 @@ func TestFiles(t *testing.T) {
 	clientID := "test_client"
 
 	Convey("Given valid params NewFiles should return a new files backend instance", t, func() {
-		files, err := NewFiles(authOpts, log.DebugLevel)
+		files, err := NewFiles(authOpts, log.DebugLevel, hashing.NewHasher(authOpts, "files"))
 		So(err, ShouldBeNil)
 
 		/*