testsuite fix: timezone: failing due to permissions in cross testing
diff mbox series

Message ID 1545088170-24103-1-git-send-email-vgupta@synopsys.com
State New
Headers show
Series
  • testsuite fix: timezone: failing due to permissions in cross testing
Related show

Commit Message

Vineet Gupta Dec. 17, 2018, 11:09 p.m. UTC
timezone test driver "zic" creates testdata directory wuth umask 755, so
only root owner/group has write permissions. However the buildroot
system created has sshd as default user, so sh fails to write to test
results in this folder. Work around by relaxing umask to 777

| /bin/sh: build/glibc-fa3b6cd5bdbc/build/timezone/testdata/America/New_York.test-result: Permission denied
| Makefile:96: recipe for target 'build/glibc-fa3b6cd5bdbc/build/timezone/testdata/America/New_York' failed
| make[2]: *** [build/glibc-fa3b6cd5bdbc/build/timezone/testdata/America/New_York] Error 1
| make[2]: Leaving directory 'build/glibc-fa3b6cd5bdbc/timezone'
| Makefile:258: recipe for target 'timezone/xtests' failed

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 timezone/zic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paul Eggert Dec. 17, 2018, 11:17 p.m. UTC | #1
>   #ifdef S_IRUSR
> -#define MKDIR_UMASK (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH)
> +#define MKDIR_UMASK (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IWGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH)
>   #else
>   #define MKDIR_UMASK 0755
>   #endif

The zic.c code is taken from upstream, and I'm loath to change it 
everywhere simply because a glibc test driver isn't working somewhere, 
as I expect this would lead to security holes elsewhere. Intead, how 
about changing the test script to chmod the testdata directory after zic 
creates it, or to segregate the tzdata directory from the other test 
results?
Vineet Gupta Dec. 17, 2018, 11:23 p.m. UTC | #2
On 12/17/18 3:17 PM, Paul Eggert wrote:
> 
>>   #ifdef S_IRUSR
>> -#define MKDIR_UMASK (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH)
>> +#define MKDIR_UMASK
>> (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IWGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH)
>>   #else
>>   #define MKDIR_UMASK 0755
>>   #endif
> 
> The zic.c code is taken from upstream, and I'm loath to change it everywhere
> simply because a glibc test driver isn't working somewhere, as I expect this would
> lead to security holes elsewhere. Intead, how about changing the test script to
> chmod the testdata directory after zic creates it, or to segregate the tzdata
> directory from the other test results?

OK, the idea here was to solicit ideas about how to go about addressing this. I
need to dig into the test harness to see how it can be addressed in the way you
suggest.

Thx,
-Vineet
Joseph Myers Dec. 17, 2018, 11:25 p.m. UTC | #3
On Mon, 17 Dec 2018, Vineet Gupta wrote:

> timezone test driver "zic" creates testdata directory wuth umask 755, so
> only root owner/group has write permissions. However the buildroot

root should not be involved in running tests at all; all tests should run 
as a normal user, the same one that owns the build directory, and thus all 
files and directories in the build directory should be owned by that user.  
(Some tests use user namespaces to test functionality that requires root, 
but that's root inside a namespace, not the real user root.)

So I think you need to explain more about your test configuration that 
results in permissions problems so we can tell if you have a valid test 
configuration at all.
Vineet Gupta Dec. 17, 2018, 11:38 p.m. UTC | #4
On 12/17/18 3:25 PM, Joseph Myers wrote:
> On Mon, 17 Dec 2018, Vineet Gupta wrote:
> 
>> timezone test driver "zic" creates testdata directory wuth umask 755, so
>> only root owner/group has write permissions. However the buildroot
> root should not be involved in running tests at all; all tests should run 
> as a normal user, the same one that owns the build directory, and thus all 
> files and directories in the build directory should be owned by that user.  
> (Some tests use user namespaces to test functionality that requires root, 
> but that's root inside a namespace, not the real user root.)
> 
> So I think you need to explain more about your test configuration that 
> results in permissions problems so we can tell if you have a valid test 
> configuration at all.

The setup is obviously is cross test, built using buildroot. The system is minimal
with root allowed w/o password to enable tests to run quickly.

On host I do the following which certainly involves root.

| make test-wrapper='<path-to-build-folder>/scripts/cross-test-ssh.sh
root@192.168.0.20' xcheck

So guess it needs to be sshd or some other user on target ?
Joseph Myers Dec. 17, 2018, 11:42 p.m. UTC | #5
On Mon, 17 Dec 2018, Vineet Gupta wrote:

> The setup is obviously is cross test, built using buildroot. The system is minimal
> with root allowed w/o password to enable tests to run quickly.
> 
> On host I do the following which certainly involves root.
> 
> | make test-wrapper='<path-to-build-folder>/scripts/cross-test-ssh.sh
> root@192.168.0.20' xcheck
> 
> So guess it needs to be sshd or some other user on target ?

The user on target should be the same as the user on the system where you 
build glibc (you may need to create a corresponding user on the target).
Florian Weimer Dec. 18, 2018, 9:12 a.m. UTC | #6
* Joseph Myers:

> On Mon, 17 Dec 2018, Vineet Gupta wrote:
>
>> timezone test driver "zic" creates testdata directory wuth umask 755, so
>> only root owner/group has write permissions. However the buildroot
>
> root should not be involved in running tests at all; all tests should run 
> as a normal user, the same one that owns the build directory, and thus all 
> files and directories in the build directory should be owned by that user.
> (Some tests use user namespaces to test functionality that requires root, 
> but that's root inside a namespace, not the real user root.)

There are also some xtests that require root privileges, such as
nptl/tst-setuid1.  Building and testing as root is supported (but
obviously not recommended unless you use a throwaway machine).

Thanks,
Florian
Vineet Gupta Dec. 18, 2018, 4:53 p.m. UTC | #7
On 12/17/18 3:25 PM, Joseph Myers wrote:
> root should not be involved in running tests at all; all tests should run 
> as a normal user, the same one that owns the build directory, and thus all 
> files and directories in the build directory should be owned by that user.  
> (Some tests use user namespaces to test functionality that requires root, 
> but that's root inside a namespace, not the real user root.)

I agree with this, but it is really hard /cumbersome to do this in a cross setup,
with a busybox based system. We need the real/full blown utilities and do this
natively.
Joseph Myers Dec. 18, 2018, 5:17 p.m. UTC | #8
On Tue, 18 Dec 2018, Florian Weimer wrote:

> There are also some xtests that require root privileges, such as
> nptl/tst-setuid1.  Building and testing as root is supported (but

That looks like it ought to become a test using user namespaces (and so 
not an xtest).

> obviously not recommended unless you use a throwaway machine).

And the combination of some parts of the testsuite writing to the build 
directory as root and other parts as a normal user isn't supported, in 
particular.
Florian Weimer Jan. 9, 2019, 12:15 p.m. UTC | #9
* Joseph Myers:

> On Tue, 18 Dec 2018, Florian Weimer wrote:
>
>> There are also some xtests that require root privileges, such as
>> nptl/tst-setuid1.  Building and testing as root is supported (but
>
> That looks like it ought to become a test using user namespaces (and so 
> not an xtest).

Can an unprivileged user namespace really be used to get a working
setuid?  The setup code in support_become_root/setup_uid_gid_mapping
does not provide that.

Thanks,
Florian

Patch
diff mbox series

diff --git a/timezone/zic.c b/timezone/zic.c
index cb1bf28bfbc8..b7a5b0f6c80c 100644
--- a/timezone/zic.c
+++ b/timezone/zic.c
@@ -39,7 +39,7 @@  typedef int_fast64_t	zic_t;
 #include <sys/stat.h>
 #endif
 #ifdef S_IRUSR
-#define MKDIR_UMASK (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH)
+#define MKDIR_UMASK (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IWGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH)
 #else
 #define MKDIR_UMASK 0755
 #endif