[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 13/43] userns: Add kuid_t and kgid_t and associated infrastructure in uidgid.h



Hello.

I have two questions regarding commit 30fe03cf "userns: Convert tomoyo to use
kuid and kgid where appropriate".

One question is

  --- a/security/tomoyo/common.c
  +++ b/security/tomoyo/common.c
  @@ -925,7 +925,9 @@ static bool tomoyo_manager(void)
   
   	if (!tomoyo_policy_loaded)
   		return true;
  -	if (!tomoyo_manage_by_non_root && (task->cred->uid || task->cred->euid))
  +	if (!tomoyo_manage_by_non_root &&
  +	    (!uid_eq(task->cred->uid,  GLOBAL_ROOT_UID) ||
  +	     !uid_eq(task->cred->euid, GLOBAL_ROOT_UID)))
   		return false;
   	exe = tomoyo_get_exe();
   	if (!exe)

. Since task == current, this block is changing from "current_uid() != 0" to
"!uid_eq(current_uid(), GLOBAL_ROOT_UID)".

On the other hand,

  --- a/security/tomoyo/audit.c
  +++ b/security/tomoyo/audit.c
  @@ -168,9 +168,14 @@ static char *tomoyo_print_header(struct tomoyo_request_info *r)
   		       stamp.day, stamp.hour, stamp.min, stamp.sec, r->profile,
   		       tomoyo_mode[r->mode], tomoyo_yesno(r->granted), gpid,
   		       tomoyo_sys_getpid(), tomoyo_sys_getppid(),
  -		       current_uid(), current_gid(), current_euid(),
  -		       current_egid(), current_suid(), current_sgid(),
  -		       current_fsuid(), current_fsgid());
  +		       from_kuid(&init_user_ns, current_uid()),
  +		       from_kgid(&init_user_ns, current_gid()),
  +		       from_kuid(&init_user_ns, current_euid()),
  +		       from_kgid(&init_user_ns, current_egid()),
  +		       from_kuid(&init_user_ns, current_suid()),
  +		       from_kgid(&init_user_ns, current_sgid()),
  +		       from_kuid(&init_user_ns, current_fsuid()),
  +		       from_kgid(&init_user_ns, current_fsgid()));
   	if (!obj)
   		goto no_obj_info;
   	if (!obj->validate_done) {

this block is changing from "current_uid()" to
"from_kuid(&init_user_ns, current_uid())".

I want to use uid seen from init_user_ns in tomoyo_manager() because I want to
allow only uid=0 in init_user_ns namespace to modify policy configuration.
I want to use uid seen from init_user_ns in tomoyo_print_header() because I
want to use this uid for auditing and policy generation.

I think we need

diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index f89a033..7086317 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -919,15 +919,14 @@ static bool tomoyo_manager(void)
 {
 	struct tomoyo_manager *ptr;
 	const char *exe;
-	const struct task_struct *task = current;
 	const struct tomoyo_path_info *domainname = tomoyo_domain()->domainname;
 	bool found = false;
 
 	if (!tomoyo_policy_loaded)
 		return true;
 	if (!tomoyo_manage_by_non_root &&
-	    (!uid_eq(task->cred->uid,  GLOBAL_ROOT_UID) ||
-	     !uid_eq(task->cred->euid, GLOBAL_ROOT_UID)))
+	    (from_kuid(&init_user_ns, current_uid()) ||
+	     from_kuid(&init_user_ns, current_euid())))
 		return false;
 	exe = tomoyo_get_exe();
 	if (!exe)

unless "!uid_eq(current_uid(), GLOBAL_ROOT_UID)" is identical with
"from_kuid(&init_user_ns, current_uid()) != 0".

The other question is

  --- a/security/tomoyo/common.h
  +++ b/security/tomoyo/common.h
  @@ -561,8 +561,8 @@ struct tomoyo_address_group {
  
   /* Subset of "struct stat". Used by conditional ACL and audit logs. */
   struct tomoyo_mini_stat {
  -	uid_t uid;
  -	gid_t gid;
  +	kuid_t uid;
  +	kgid_t gid;
   	ino_t ino;
   	umode_t mode;
   	dev_t dev;

. Since "struct tomoyo_mini_stat" is filled by tomoyo_get_attributes() and
read by tomoyo_condition()/tomoyo_print_header(), can't we convert kuid_t value
to uid_t value at tomoyo_get_attributes() and omit conversion at
tomoyo_condition()/tomoyo_print_header() like below?

diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index af010b6..19fa8a5 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -561,8 +561,8 @@ struct tomoyo_address_group {
 
 /* Subset of "struct stat". Used by conditional ACL and audit logs. */
 struct tomoyo_mini_stat {
-	kuid_t uid;
-	kgid_t gid;
+	uid_t uid; /* Already converted by from_kuid(&init_user_ns). */
+	gid_t gid; /* Already converted by from_kgid(&init_user_ns). */
 	ino_t ino;
 	umode_t mode;
 	dev_t dev;
diff --git a/security/tomoyo/condition.c b/security/tomoyo/condition.c
index 63681e8..f3672f7 100644
--- a/security/tomoyo/condition.c
+++ b/security/tomoyo/condition.c
@@ -717,8 +717,8 @@ void tomoyo_get_attributes(struct tomoyo_obj_info *obj)
 		inode = dentry->d_inode;
 		if (inode) {
 			struct tomoyo_mini_stat *stat = &obj->stat[i];
-			stat->uid  = inode->i_uid;
-			stat->gid  = inode->i_gid;
+			stat->uid  = from_kuid(&init_user_ns, inode->i_uid);
+			stat->gid  = from_kgid(&init_user_ns, inode->i_gid);
 			stat->ino  = inode->i_ino;
 			stat->mode = inode->i_mode;
 			stat->dev  = inode->i_sb->s_dev;
@@ -970,13 +970,13 @@ bool tomoyo_condition(struct tomoyo_request_info *r,
 					case TOMOYO_PATH2_UID:
 					case TOMOYO_PATH1_PARENT_UID:
 					case TOMOYO_PATH2_PARENT_UID:
-						value = from_kuid(&init_user_ns, stat->uid);
+						value = stat->uid;
 						break;
 					case TOMOYO_PATH1_GID:
 					case TOMOYO_PATH2_GID:
 					case TOMOYO_PATH1_PARENT_GID:
 					case TOMOYO_PATH2_PARENT_GID:
-						value = from_kgid(&init_user_ns, stat->gid);
+						value = stat->gid;
 						break;
 					case TOMOYO_PATH1_INO:
 					case TOMOYO_PATH2_INO:
diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c
index c1b0037..906f14d 100644
--- a/security/tomoyo/audit.c
+++ b/security/tomoyo/audit.c
@@ -196,19 +196,15 @@ static char *tomoyo_print_header(struct tomoyo_request_info *r)
 					tomoyo_buffer_len - 1 - pos,
 					" path%u.parent={ uid=%u gid=%u "
 					"ino=%lu perm=0%o }", (i >> 1) + 1,
-					from_kuid(&init_user_ns, stat->uid),
-					from_kgid(&init_user_ns, stat->gid),
-					(unsigned long)stat->ino,
-					stat->mode & S_IALLUGO);
+					stat->uid, stat->gid, (unsigned long)
+					stat->ino, stat->mode & S_IALLUGO);
 			continue;
 		}
 		pos += snprintf(buffer + pos, tomoyo_buffer_len - 1 - pos,
 				" path%u={ uid=%u gid=%u ino=%lu major=%u"
 				" minor=%u perm=0%o type=%s", (i >> 1) + 1,
-				from_kuid(&init_user_ns, stat->uid),
-				from_kgid(&init_user_ns, stat->gid),
-				(unsigned long)stat->ino,
-				MAJOR(dev), MINOR(dev),
+				stat->uid, stat->gid, (unsigned long)
+				stat->ino, MAJOR(dev), MINOR(dev),
 				mode & S_IALLUGO, tomoyo_filetype(mode));
 		if (S_ISCHR(mode) || S_ISBLK(mode)) {
 			dev = stat->rdev;

Regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]

Powered by Linux