git: 36d67475f549 - main - xinstall: fix dounpriv logic, add tests

Reply: Jessica Clarke : "Re: git: 36d67475f549 - main - xinstall: fix dounpriv logic, add tests"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Wed, 03 Aug 2022 19:04:28 UTC
The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=36d67475f5497664d33c41c2f6745dcb30b0ec42

commit 36d67475f5497664d33c41c2f6745dcb30b0ec42
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2022-08-03 18:59:28 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2022-08-03 19:03:49 +0000

    xinstall: fix dounpriv logic, add tests
    
    Sponsored by:   Klara, Inc.
    MFC after:      1 week
---
 usr.bin/xinstall/tests/install_test.sh | 80 ++++++++++++++++++++++++++++++++++
 usr.bin/xinstall/xinstall.c            | 15 +++----
 2 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/usr.bin/xinstall/tests/install_test.sh b/usr.bin/xinstall/tests/install_test.sh
index c723a6e0d280..e95f40fc19e7 100755
--- a/usr.bin/xinstall/tests/install_test.sh
+++ b/usr.bin/xinstall/tests/install_test.sh
@@ -404,6 +404,84 @@ symbolic_link_relative_absolute_common_body() {
 	fi
 }
 
+atf_test_case set_owner_group_mode
+set_owner_group_mode_head() {
+	atf_set "require.user" "root"
+}
+set_owner_group_mode_body() {
+	local fu=65531 fg=65531
+	local cu=65532 cg=65532
+	local u="$(id -u)"
+	local g="$(id -g)"
+	local m=0755 cm=4444
+	printf "test" >testf
+	atf_check chown "$fu:$fg" testf
+	atf_check chmod "$m" testf
+
+	atf_check install testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" testf testc
+	atf_check_equal "$cu:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -g "$cg" testf testc
+	atf_check_equal "$u:$cg:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" -g "$cg" testf testc
+	atf_check_equal "$cu:$cg:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cm" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" -m "$cm" testf testc
+	atf_check_equal "$cu:$g:10$cm" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$u:$cg:10$cm" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$cu:$cg:10$cm" "$(stat -f"%u:%g:%p" testc)"
+}
+
+atf_test_case set_owner_group_mode_unpriv
+set_owner_group_mode_unpriv_head() {
+	atf_set "require.user" "root"
+}
+set_owner_group_mode_unpriv_body() {
+	local fu=65531 fg=65531
+	local cu=65532 cg=65532
+	local u="$(id -u)"
+	local g="$(id -g)"
+	local m=0755 cm=4444 cM=0444
+	printf "test" >testf
+	atf_check chown "$fu:$fg" testf
+	atf_check chmod "$m" testf
+
+	atf_check install -U testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -g "$cg" testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" -g "$cg" testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+}
+
 atf_init_test_cases() {
 	atf_add_test_case copy_to_nonexistent
 	atf_add_test_case copy_to_nonexistent_safe
@@ -444,4 +522,6 @@ atf_init_test_cases() {
 	atf_add_test_case symbolic_link_relative_absolute_source_and_dest2
 	atf_add_test_case symbolic_link_relative_absolute_common
 	atf_add_test_case mkdir_simple
+	atf_add_test_case set_owner_group_mode
+	atf_add_test_case set_owner_group_mode_unpriv
 }
diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c
index 05b1444506db..ddad7ba9115e 100644
--- a/usr.bin/xinstall/xinstall.c
+++ b/usr.bin/xinstall/xinstall.c
@@ -1009,19 +1009,18 @@ install(const char *from_name, const char *to_name, u_long fset, u_int flags)
 #endif
 	}
 
-	if (!dounpriv & 
-	    (gid != (gid_t)-1 && gid != to_sb.st_gid) ||
-	    (uid != (uid_t)-1 && uid != to_sb.st_uid))
+	if (!dounpriv && ((gid != (gid_t)-1 && gid != to_sb.st_gid) ||
+	    (uid != (uid_t)-1 && uid != to_sb.st_uid))) {
 		if (fchown(to_fd, uid, gid) == -1) {
 			serrno = errno;
 			(void)unlink(to_name);
 			errno = serrno;
 			err(EX_OSERR,"%s: chown/chgrp", to_name);
 		}
-
+	}
 	if (mode != (to_sb.st_mode & ALLPERMS)) {
 		if (fchmod(to_fd,
-		     dounpriv ? mode & (S_IRWXU|S_IRWXG|S_IRWXO) : mode)) {
+		    dounpriv ? mode & (S_IRWXU|S_IRWXG|S_IRWXO) : mode)) {
 			serrno = errno;
 			(void)unlink(to_name);
 			errno = serrno;
@@ -1036,7 +1035,7 @@ install(const char *from_name, const char *to_name, u_long fset, u_int flags)
 	 * trying to turn off UF_NODUMP.  If we're trying to set real flags,
 	 * then warn if the fs doesn't support it, otherwise fail.
 	 */
-	if (!dounpriv & !devnull && (flags & SETFLAGS ||
+	if (!dounpriv && !devnull && (flags & SETFLAGS ||
 	    (from_sb.st_flags & ~UF_NODUMP) != to_sb.st_flags) &&
 	    fchflags(to_fd,
 	    flags & SETFLAGS ? fset : from_sb.st_flags & ~UF_NODUMP)) {
@@ -1082,7 +1081,7 @@ compare(int from_fd, const char *from_name __unused, size_t from_len,
 		return 1;
 
 	do_digest = (digesttype != DIGEST_NONE && dresp != NULL &&
-			*dresp == NULL);
+	    *dresp == NULL);
 	if (from_len <= MAX_CMP_SIZE) {
 		if (do_digest)
 			digest_init(&ctx);
@@ -1390,7 +1389,7 @@ install_dir(char *path)
 			ch = *p;
 			*p = '\0';
 again:
-			if (stat(path, &sb) < 0) {
+			if (stat(path, &sb) != 0) {
 				if (errno != ENOENT || tried_mkdir)
 					err(EX_OSERR, "stat %s", path);
 				if (mkdir(path, 0755) < 0) {