feat(security): Address PR feedback from Tony

Change the redis.conf file permission to 0600
Make chown for redis' conf to redis:redis 999:1000 as part of redis' uid and gid creation
Add doc's url and detailed explanation for redis' conf EdgeX is currently using

Signed-off-by: Jim Wang <[email protected]>

internal/security/bootstrapper/helper/helper.go

@@ -48,6 +48,16 @@ func CreateDirectoryIfNotExists(dirName string) (err error) {
 	return
 }
 
+// ChownDirRecursive changes ownership of files and directory dirName recursively
+func ChownDirRecursive(dirName string, uid, gid int) error {
+	return filepath.Walk(dirName, func(fileName string, info os.FileInfo, err error) error {
+		if err == nil {
+			err = os.Chown(fileName, uid, gid)
+		}
+		return err
+	})
+}
+
 func checkIfFileExists(fileName string) bool {
 	fileInfo, statErr := os.Stat(fileName)
 	if os.IsNotExist(statErr) {

internal/security/bootstrapper/helper/helper_test.go

@@ -16,6 +16,7 @@
 package helper
 
 import (
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -57,6 +58,34 @@ func TestCreateDirectoryIfNotExists(t *testing.T) {
 	require.DirExists(t, testPreCreatedDir)
 }
 
+func TestChownDirRecursively(t *testing.T) {
+	testDir := "testChownDir"
+	testFile := "testChownFile"
+
+	defer (cleanupDir(testDir))()
+
+	err := os.MkdirAll(testDir, os.ModePerm)
+	require.NoError(t, err)
+
+	testFilePath := filepath.Join(testDir, testFile)
+
+	err = ioutil.WriteFile(testFilePath, []byte("this is a test"), 0600)
+	require.NoError(t, err)
+
+	err = ChownDirRecursive(testDir, os.Geteuid(), os.Getegid())
+	require.NoError(t, err)
+
+	// if try to change ownership to a different uid, since this test most likely will be run
+	// as non-root user, so the error will be expected
+	err = ChownDirRecursive(testDir, 999, os.Getegid())
+	if os.Geteuid() != 0 {
+		// expected permission issue since it is not allowed to change ownership to other user
+		require.Error(t, err)
+	} else {
+		require.NoError(t, err)
+	}
+}
+
 // cleanupDir deletes all files in the directory and files in the directory
 func cleanupDir(dir string) func() {
 	return func() {

internal/security/bootstrapper/helper/redis_conf.go

@@ -23,7 +23,8 @@ import (
 
 /* Redis ACL configuration
 *
-* The followings are contents excerpted from redis 6.0 conf documentation:
+* The followings are some contents excerpted from Redis 6.0 online conf documentation
+* regarding the ACL rules as seen in this https://redis.io/topics/acl#acl-rules:
 *
 # Redis ACL users are defined in the following format:
 #
@@ -44,67 +45,23 @@ import (
 # The ACL rules that describe what a user can do are the following:
 #
 #  on           Enable the user: it is possible to authenticate as this user.
-#  off          Disable the user: it's no longer possible to authenticate
-#               with this user, however the already authenticated connections
-#               will still work.
-#  +<command>   Allow the execution of that command
-#  -<command>   Disallow the execution of that command
 #  +@<category> Allow the execution of all the commands in such category
 #               with valid categories are like @admin, @set, @sortedset, ...
 #               and so forth, see the full list in the server.c file where
 #               the Redis command table is described and defined.
 #               The special category @all means all the commands, but currently
 #               present in the server, and that will be loaded in the future
 #               via modules.
-#  +<command>|subcommand    Allow a specific subcommand of an otherwise
-#                           disabled command. Note that this form is not
-#                           allowed as negative like -DEBUG|SEGFAULT, but
-#                           only additive starting with "+".
 #  allcommands  Alias for +@all. Note that it implies the ability to execute
 #               all the future commands loaded via the modules system.
-#  nocommands   Alias for -@all.
-#  ~<pattern>   Add a pattern of keys that can be mentioned as part of
-#               commands. For instance ~* allows all the keys. The pattern
-#               is a glob-style pattern like the one of KEYS.
-#               It is possible to specify multiple patterns.
 #  allkeys      Alias for ~*
-#  resetkeys    Flush the list of allowed keys patterns.
 #  ><password>  Add this password to the list of valid password for the user.
 #               For example >mypass will add "mypass" to the list.
 #               This directive clears the "nopass" flag (see later).
-#  <<password>  Remove this password from the list of valid passwords.
-#  nopass       All the set passwords of the user are removed, and the user
-#               is flagged as requiring no password: it means that every
-#               password will work against this user. If this directive is
-#               used for the default user, every new connection will be
-#               immediately authenticated with the default user without
-#               any explicit AUTH command required. Note that the "resetpass"
-#               directive will clear this condition.
-#  resetpass    Flush the list of allowed passwords. Moreover removes the
-#               "nopass" status. After "resetpass" the user has no associated
-#               passwords and there is no way to authenticate without adding
-#               some password (or setting it as "nopass" later).
-#  reset        Performs the following actions: resetpass, resetkeys, off,
-#               -@all. The user returns to the same state it has immediately
-#               after its creation.
 #
 # ACL rules can be specified in any order: for instance you can start with
 # passwords, then flags, or key patterns. However note that the additive
 # and subtractive rules will CHANGE MEANING depending on the ordering.
-# For instance see the following example:
-#
-#   user alice on +@all -DEBUG ~* >somepassword
-#
-# This will allow "alice" to use all the commands with the exception of the
-# DEBUG command, since +@all added all the commands to the set of the commands
-# alice can use, and later DEBUG was removed. However if we invert the order
-# of two ACL rules the result will be different:
-#
-#   user alice on -DEBUG +@all ~* >somepassword
-#
-# Now DEBUG was removed when alice had yet no commands in the set of allowed
-# commands, later all the commands are added, so the user will be able to
-# execute everything.
 #
 # Basically ACL rules are processed left-to-right.
 #
@@ -119,6 +76,15 @@ import (
 #
 # requirepass foobared
 *
+* For EdgeX's use case today, the ACL rules are defined in a template as seen in
+* the following constant aclDefaultUserTemplate.
+* In particular, it defines the ACL rule for default user with the following accesses:
+*   1) allkeys: it allows the user to access all the keys
+*   2) +@all: this is an alias for allcommands and + means to allow
+*   3) -@dangerous: disallow all the commands that are tagged as dangerous inside the Redis command table
+*   4) >{{.RedisPwd}}: add the dynamically injected password for this user
+*
+*
 */
 
 const (

internal/security/bootstrapper/redis/handlers/handlers.go

@@ -34,6 +34,14 @@ import (
 	"github.com/edgexfoundry/go-mod-bootstrap/v2/di"
 )
 
+const (
+	// based on the Redis' alpine Dockerfile:
+	// https://github.com/docker-library/redis/blob/68595be6067839e5c5c1a35bdbb6357d017a8a4e/6.0/alpine/Dockerfile#L4
+	// redis server runs with redis uid 999 and redis group gid 1000
+	redisUid = 999
+	redisGid = 1000
+)
+
 // Handler is the redis bootstrapping handler
 type Handler struct {
 	credentials bootstrapConfig.Credentials
@@ -107,14 +115,11 @@ func (handler *Handler) SetupConfFile(ctx context.Context, _ *sync.WaitGroup, _
 	lc.Infof("Setting up the database config file %s", dbConfigFilePath)
 
 	// open config file with read-write and overwritten attribute (TRUNC)
-	confFile, err := os.OpenFile(dbConfigFilePath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755)
+	confFile, err := os.OpenFile(dbConfigFilePath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
 	if err != nil {
 		lc.Errorf("failed to open db config file %s: %v", dbConfigFilePath, err)
 		return false
 	}
-	defer func() {
-		_ = confFile.Close()
-	}()
 
 	// writing the config file
 	fwriter := bufio.NewWriter(confFile)
@@ -126,6 +131,14 @@ func (handler *Handler) SetupConfFile(ctx context.Context, _ *sync.WaitGroup, _
 		lc.Errorf("failed to flush the file writer buffer %v", err)
 		return false
 	}
+	if err := confFile.Close(); err != nil {
+		lc.Errorf("failed to close the config file %v", err)
+		return false
+	}
+	if err := helper.ChownDirRecursive(dbConfigDir, redisUid, redisGid); err != nil {
+		lc.Errorf("failed to change the ownership for redis' configDir %s:  %v", dbConfigDir, err)
+		return false
+	}
 
 	lc.Info("database credentials have been set in the config file")