syscalls/getgroups/getgroups01: set supplementary groups to prevent TCONF

PATCH In review Han Pingtian 3 participants 8 messages Dec 4, 2014 – Dec 8, 2014

lore

Share & tools

lore

Han Pingtian

author

2014-12-04 08:54 UTC #0 lore raw

PATCH 1/1 syscalls/getgroups/getgroups01: set supplementary groups to prevent TCONF

Hi,

On our systems, root belongs to only one group. getgroups01 has this result:

getgroups01    1  TPASS  :  getgroups failed as expected with EINVAL
getgroups01    2  TPASS  :  getgroups did not modify the gidset array
getgroups01    3  TCONF  :  getgroups01.c:129: getgroups returned 1; unable to test that using ngrps >=1 but less than number of grps
getgroups01    4  TPASS  :  getgroups(NGROUPS,gidset) returned 1 contains gid 0 (from getgid)

Looks like we can set supplementary groups by setgroups() in setup(), then 3# will succeed if running as privileged user. If we call setgroups(), then initgroups() can be left out. With this patch, when running as root:

getgroups01    1  TPASS  :  getgroups failed as expected with EINVAL
getgroups01    2  TPASS  :  getgroups did not modify the gidset array
getgroups01    3  TPASS  :  getgroups failed as expected with EINVAL
getgroups01    4  TPASS  :  getgroups(NGROUPS,gidset) returned 3 contains gid 0 (from getgid)

running as unprivileged user:

getgroups01    1  TPASS  :  getgroups failed as expected with EINVAL
getgroups01    2  TPASS  :  getgroups did not modify the gidset array
getgroups01    3  TCONF  :  getgroups01.c:127: getgroups returned 1; unable to test that using ngrps >=1 but less than number of grps
getgroups01    4  TPASS  :  getgroups(NGROUPS,gidset) returned 1 contains gid 1001 (from getgid)

From caf7b59519b0f9323805aae9d6a607a30ea3ba4e Mon Sep 17 00:00:00 2001
From: Han Pingtian <[email protected]>
Date: Thu, 4 Dec 2014 16:07:25 +0800
Subject: [PATCH] set supplementary groups instead of relying on system setting

On a system where root belongs to only one group, the 3# test of getgroups01 will fail by TCONF. But if we call setgroups() to set supplementary groups, the test can succeed if run as privileged user.

With this patch, it'll still fail by TCONF when being run by unprivileged user.

Signed-off-by Han Pingtian <[email protected]>

---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

testcases/kernel/syscalls/getgroups/getgroups01.c +3 -2

index 9fb4c98..28559d3 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);

@@ -183,6 +181,9 @@ static void setup(void)
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
+
+	GID_T init_gidset[3] = {0, 1, 2};
+	setgroups(3, init_gidset);
 }
 
 static void cleanup(void)

Signature · 13 lines

1.9.3


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Cyril Hrubis

2014-12-04 10:17 UTC #1 lore raw

Hi!

Quoted 19 lines

Signed-off-by Han Pingtian <[email protected]>

---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

testcases/kernel/syscalls/getgroups/getgroups01.c +0 -2

index 9fb4c98..28559d3 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);

testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 185

Quoted 5 lines

testcases/kernel/syscalls/getgroups/getgroups01.c +2 -0

@@ -183,6 +181,9 @@ static void setup(void)
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
+
+	GID_T init_gidset[3] = {0, 1, 2};

This should be gid_t instead of GID_T. The uppercase prototypes are for raw syscalls only. The setgroups() you call is glibc wrapper instead.

testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 186

Quoted 1 line

testcases/kernel/syscalls/getgroups/getgroups01.c +1 -0
+	setgroups(3, init_gidset);

Otherwise it looks good to me.

Signature · 13 lines

Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Jan Stancek

2014-12-04 10:44 UTC #2 lore raw

----- Original Message -----

Quoted 20 lines

From: "Cyril Hrubis" <[email protected]>
To: [email protected]
Sent: Thursday, 4 December, 2014 11:17:41 AM
Subject: Re: [LTP] [PATCH] syscalls/getgroups/getgroups01: set supplementary groups to prevent TCONF
Hi!
Quoted 14 lines
Signed-off-by Han Pingtian <[email protected]>
---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -0
index 9fb4c98..28559d3 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 

Quoted 6 lines

testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void)
Quoted 6 lines
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -2
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
 
@@ -183,6 +181,9 @@ static void setup(void)

I'd add tst_require_root() to setup, since setgroups() requires CAP_SETGID.

Quoted 9 lines

testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 185
Quoted 5 lines
testcases/kernel/syscalls/getgroups/getgroups01.c +2 -0
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
+
+	GID_T init_gidset[3] = {0, 1, 2};
This should be gid_t instead of GID_T. The uppercase prototypes are for raw syscalls only. The setgroups() you call is glibc wrapper instead.

Quoted 3 lines

testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 186
Quoted 1 line
testcases/kernel/syscalls/getgroups/getgroups01.c +1 -0
+	setgroups(3, init_gidset);
Otherwise it looks good to me.

To me as well.

Regards,
Jan

Quoted 5 lines

--
Cyril Hrubis
[email protected]

Quoted 11 lines

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Han Pingtian

author

2014-12-05 01:37 UTC #3 lore raw

Quoted 21 lines · Jan Stancek

----- Original Message -----
Quoted 20 lines
From: "Cyril Hrubis" <[email protected]>
To: [email protected]
Sent: Thursday, 4 December, 2014 11:17:41 AM
Subject: Re: [LTP] [PATCH] syscalls/getgroups/getgroups01: set supplementary groups to prevent TCONF
Hi!
Quoted 14 lines
Signed-off-by Han Pingtian <[email protected]>
---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -0
index 9fb4c98..28559d3 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 

Quoted 8 lines

Quoted 6 lines
testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void)
Quoted 6 lines
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -2
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
 
@@ -183,6 +181,9 @@ static void setup(void)
I'd add tst_require_root() to setup, since setgroups() requires CAP_SETGID.

I think getgroups() doesn't need privileged power, so we shouldn't limit this case with tst_require_root, because this case is testing getgroups(). With this patch applied, if run by unprivileged user, 3# will fail by TCONF, but other tests will pass.

Thanks.

Jan Stancek

2014-12-05 07:50 UTC #4 lore raw

----- Original Message -----

Quoted 33 lines

From: "Han Pingtian" <[email protected]>
To: [email protected]
Sent: Friday, 5 December, 2014 2:37:22 AM
Subject: Re: [LTP] [PATCH] syscalls/getgroups/getgroups01: set supplementary groups to prevent TCONF
Quoted 27 lines · Jan Stancek
----- Original Message -----
Quoted 26 lines
From: "Cyril Hrubis" <[email protected]>
To: [email protected]
Sent: Thursday, 4 December, 2014 11:17:41 AM
Subject: Re: [LTP] [PATCH] syscalls/getgroups/getgroups01: set supplementary groups to prevent TCONF
Hi!
Quoted 19 lines
Signed-off-by Han Pingtian <[email protected]>
---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -2
index 9fb4c98..28559d3 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
 

Quoted 8 lines

Quoted 3 lines
Quoted 1 line
testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void)
Quoted 1 line
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -0
@@ -183,6 +181,9 @@ static void setup(void)
I'd add tst_require_root() to setup, since setgroups() requires CAP_SETGID.
I think getgroups() doesn't need privileged power, so we shouldn't limit this case with tst_require_root, because this case is testing getgroups(). With this patch applied, if run by unprivileged user, 3# will fail by TCONF, but other tests will pass.

OK, looks like initgroups is implemented via setgroups, so this was failing silently before as well.

Regards,
Jan

Quoted 4 lines

Thanks.

Quoted 11 lines

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Han Pingtian

author

2014-12-05 01:45 UTC #5 lore raw

In reply to Cyril Hrubis

Quoted 29 lines · Cyril Hrubis

Hi!
Quoted 19 lines
Signed-off-by Han Pingtian <[email protected]>
---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -2
index 9fb4c98..28559d3 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
 
testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 185
Quoted 5 lines
testcases/kernel/syscalls/getgroups/getgroups01.c +2 -0
@@ -183,6 +181,9 @@ static void setup(void)
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
+
+	GID_T init_gidset[3] = {0, 1, 2};
This should be gid_t instead of GID_T. The uppercase prototypes are for raw syscalls only. The setgroups() you call is glibc wrapper instead.

Quoted 3 lines

testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 186
Quoted 1 line
testcases/kernel/syscalls/getgroups/getgroups01.c +1 -0
+	setgroups(3, init_gidset);
Otherwise it looks good to me.

Thanks, this is the new patch for review.

From 261b475fe4ecf8572003b0c444394b67feb0c781 Mon Sep 17 00:00:00 2001
From: Han Pingtian <[email protected]>
Date: Thu, 4 Dec 2014 16:07:25 +0800
Subject: [PATCH] set supplementary groups instead of relying on system setting

With this patch, it'll still fail by TCONF when being run by unprivileged user.

Signed-off-by Han Pingtian <[email protected]>

---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

testcases/kernel/syscalls/getgroups/getgroups01.c +3 -2

index 9fb4c98..02c8ce0 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);

@@ -183,6 +181,9 @@ static void setup(void)
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
+
+	git_t init_gidset[3] = {0, 1, 2};
+	setgroups(3, init_gidset);
 }
 
 static void cleanup(void)

Signature · 13 lines

1.9.3


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Han Pingtian

author

2014-12-05 05:37 UTC #6 lore raw

Quoted 30 lines · Han Pingtian

Quoted 29 lines · Cyril Hrubis
Hi!
Quoted 19 lines
Signed-off-by Han Pingtian <[email protected]>
---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
testcases/kernel/syscalls/getgroups/getgroups01.c +0 -2
index 9fb4c98..28559d3 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
 
testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 185
Quoted 5 lines
testcases/kernel/syscalls/getgroups/getgroups01.c +2 -0
@@ -183,6 +181,9 @@ static void setup(void)
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
+
+	GID_T init_gidset[3] = {0, 1, 2};
This should be gid_t instead of GID_T. The uppercase prototypes are for raw syscalls only. The setgroups() you call is glibc wrapper instead.

Quoted 5 lines

Quoted 3 lines
testcases/kernel/syscalls/getgroups/getgroups01.c static void setup(void) · line 186
Quoted 1 line
testcases/kernel/syscalls/getgroups/getgroups01.c +1 -0
+	setgroups(3, init_gidset);
Otherwise it looks good to me.
Thanks, this is the new patch for review.

I'm sorry that there is a typo in V2. Please have a look at this:

From 1f260f261266ccddf16854c87dd72b4cf91bd745 Mon Sep 17 00:00:00 2001
From: Han Pingtian <[email protected]>
Date: Thu, 4 Dec 2014 16:07:25 +0800
Subject: [PATCH] set supplementary groups instead of relying on system setting

With this patch, it'll still fail by TCONF when being run by unprivileged user.

Signed-off-by Han Pingtian <[email protected]>

---
 testcases/kernel/syscalls/getgroups/getgroups01.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

testcases/kernel/syscalls/getgroups/getgroups01.c +3 -2

index 9fb4c98..2c1407b 100644
@@ -74,8 +74,6 @@ int main(int ac, char **av)
 	int i;
 	int entries;
 
-	initgroups("root", 0);
-
 	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);

@@ -183,6 +181,9 @@ static void setup(void)
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
+
+	gid_t init_gidset[3] = {0, 1, 2};
+	setgroups(3, init_gidset);
 }
 
 static void cleanup(void)

Signature · 13 lines

1.9.3


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Cyril Hrubis

2014-12-08 12:55 UTC #7 lore raw

Hi!
Pushed, thanks.

Signature · 13 lines

Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list