From 972cdc560e339d70c0ee5fb70ec636ab78f00bca Mon Sep 17 00:00:00 2001
From: Donatas Abraitis <donatas@opensourcerouting.org>
Date: Mon, 10 Oct 2022 18:27:59 +0300
Subject: [PATCH] tools: Use `install` instead of `touch/chown` combination

touch + chown can have a gap between the commands (or the second failed).

This could lead to unexpected permissions (root, instead of frr) for some
.conf files or directories.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
---
 tools/frr.in          | 18 ++++--------------
 tools/frrcommon.sh.in | 16 +++-------------
 2 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/tools/frr.in b/tools/frr.in
index 27b2c0ab84d8..f0c665fdee72 100755
--- a/tools/frr.in
+++ b/tools/frr.in
@@ -53,13 +53,6 @@ vtyfile()
 	echo "$V_PATH/$1.vty"
 }
 
-chownfrr()
-{
-	test -n "$FRR_USER" && chown "$FRR_USER" "$1"
-	test -n "$FRR_GROUP" && chgrp "$FRR_GROUP" "$1"
-	test -n "$FRR_CONFIG_MODE" && chmod "$FRR_CONFIG_MODE" "$1"
-}
-
 # Check if daemon is started by using the pidfile.
 started()
 {
@@ -103,12 +96,10 @@ check_daemon()
 		# check for config file
 		if [ -n "$2" ]; then
 			if [ ! -r "$C_PATH/$1-$2.conf" ]; then
-				touch "$C_PATH/$1-$2.conf"
-				chownfrr "$C_PATH/$1-$2.conf"
+				install -g "$FRR_GROUP" -o "$FRR_USER" -m "$FRR_CONFIG_MODE" /dev/null "$C_PATH/$1-$2.conf"
 			fi
 		elif [ ! -r "$C_PATH/$1.conf" ]; then
-			touch "$C_PATH/$1.conf"
-			chownfrr "$C_PATH/$1.conf"
+			install -g "$FRR_GROUP" -o "$FRR_USER" -m "$FRR_CONFIG_MODE" /dev/null "$C_PATH/$1.conf"
 		fi
 	fi
 	return 0
@@ -533,9 +524,8 @@ convert_daemon_prios
 
 if [ ! -d $V_PATH ]; then
 	echo "Creating $V_PATH"
-	mkdir -p $V_PATH
-	chownfrr $V_PATH
-	chmod 755 /$V_PATH
+	install -g "$FRR_GROUP" -o "$FRR_USER" -m "$FRR_CONFIG_MODE" -d /proc "$V_PATH"
+	chmod gu+x "${V_PATH}"
 fi
 
 if [ -n "$3" ] && [ "$3" != "all" ]; then
diff --git a/tools/frrcommon.sh.in b/tools/frrcommon.sh.in
index b589ced965ae..469b9c5d8c1e 100755
--- a/tools/frrcommon.sh.in
+++ b/tools/frrcommon.sh.in
@@ -62,15 +62,6 @@ debug() {
 	printf '\n' >&2
 }
 
-chownfrr() {
-	[ -n "$FRR_USER" ] && chown "$FRR_USER" "$1"
-	[ -n "$FRR_GROUP" ] && chgrp "$FRR_GROUP" "$1"
-	[ -n "$FRR_CONFIG_MODE" ] && chmod "$FRR_CONFIG_MODE" "$1"
-	if [ -d "$1" ]; then
-		chmod gu+x "$1"
-	fi
-}
-
 vtysh_b () {
 	[ "$1" = "watchfrr" ] && return 0
 	if [ ! -r "$C_PATH/frr.conf" ]; then
@@ -152,8 +143,7 @@ daemon_prep() {
 
 	cfg="$C_PATH/$daemon${inst:+-$inst}.conf"
 	if [ ! -r "$cfg" ]; then
-		touch "$cfg"
-		chownfrr "$cfg"
+		install -g "$FRR_GROUP" -o "$FRR_USER" -m "$FRR_CONFIG_MODE" /dev/null "$cfg"
 	fi
 	return 0
 }
@@ -171,8 +161,8 @@ daemon_start() {
 	[ "$MAX_FDS" != "" ] && ulimit -n "$MAX_FDS" > /dev/null 2> /dev/null
 	daemon_prep "$daemon" "$inst" || return 1
 	if test ! -d "$V_PATH"; then
-		mkdir -p "$V_PATH"
-		chownfrr "$V_PATH"
+		install -g "$FRR_GROUP" -o "$FRR_USER" -m "$FRR_CONFIG_MODE" -d /proc "$V_PATH"
+		chmod gu+x "${V_PATH}"
 	fi
 
 	eval wrap="\$${daemon}_wrap"