[PATCH] ci: make vm_start logs readable by all users

[PATCH] ci: make vm_start logs readable by all users

[Uladzimir Bely]

This changes permissions for ${BUILD_DIR}/vm_start_.*_log.txt from 600
to 644 to make them readable not only for jenkins user.

Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
---
 testsuite/vm_boot_test/vm_boot_test.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testsuite/vm_boot_test/vm_boot_test.py b/testsuite/vm_boot_test/vm_boot_test.py
index 01623c5c..69e7f4ae 100644
--- a/testsuite/vm_boot_test/vm_boot_test.py
+++ b/testsuite/vm_boot_test/vm_boot_test.py
@@ -46,6 +46,7 @@ class VmBase(Test):
         fd, output_file = tempfile.mkstemp(suffix='_log.txt',
                                            prefix='vm_start_' + distro + '_' +
                                            arch + '_', dir=build_dir, text=True)
+        os.chmod(output_file, 0o644)
 
         cmdline = start_vm.format_qemu_cmdline(arch, build_dir, distro,
                                                output_file, None)
-- 
2.20.1

Re: [PATCH] ci: make vm_start logs readable by all users

[Henning Schild]

Am Thu, 23 Dec 2021 06:57:20 +0100
schrieb Uladzimir Bely <ubely@ilbers.de>:

> This changes permissions for ${BUILD_DIR}/vm_start_.*_log.txt from 600
> to 644 to make them readable not only for jenkins user.

not everybody uses jenkins ... make it "CI user"

> Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
> ---
>  testsuite/vm_boot_test/vm_boot_test.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/testsuite/vm_boot_test/vm_boot_test.py
> b/testsuite/vm_boot_test/vm_boot_test.py index 01623c5c..69e7f4ae
> 100644 --- a/testsuite/vm_boot_test/vm_boot_test.py
> +++ b/testsuite/vm_boot_test/vm_boot_test.py
> @@ -46,6 +46,7 @@ class VmBase(Test):
>          fd, output_file = tempfile.mkstemp(suffix='_log.txt',
>                                             prefix='vm_start_' +
> distro + '_' + arch + '_', dir=build_dir, text=True)
> +        os.chmod(output_file, 0o644)

This smells like other output files could also have wrong permissions
while being relevant.
If that might be the case, maybe centrally adjust umask for all of the
testsuite.

Henning

>          cmdline = start_vm.format_qemu_cmdline(arch, build_dir,
> distro, output_file, None)

Re: [PATCH] ci: make vm_start logs readable by all users

[Uladzimir Bely]

In mail from вторник, 4 января 2022 г. 22:58:39 +03 user Henning Schild wrote:
> Am Thu, 23 Dec 2021 06:57:20 +0100
> 
> schrieb Uladzimir Bely <ubely@ilbers.de>:
> > This changes permissions for ${BUILD_DIR}/vm_start_.*_log.txt from 600
> > to 644 to make them readable not only for jenkins user.
> 
> not everybody uses jenkins ... make it "CI user"
> 

OK.

> > Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
> > ---
> > 
> >  testsuite/vm_boot_test/vm_boot_test.py | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/testsuite/vm_boot_test/vm_boot_test.py
> > b/testsuite/vm_boot_test/vm_boot_test.py index 01623c5c..69e7f4ae
> > 100644 --- a/testsuite/vm_boot_test/vm_boot_test.py
> > +++ b/testsuite/vm_boot_test/vm_boot_test.py
> > 
> > @@ -46,6 +46,7 @@ class VmBase(Test):
> >          fd, output_file = tempfile.mkstemp(suffix='_log.txt',
> >          
> >                                             prefix='vm_start_' +
> > 
> > distro + '_' + arch + '_', dir=build_dir, text=True)
> > +        os.chmod(output_file, 0o644)
> 
> This smells like other output files could also have wrong permissions
> while being relevant.

It looks like a specific behaviour of tempfile.mkstemp() function, that always 
creates file readable/writable only by the user ID. So, only vm logs are 
affected.

> If that might be the case, maybe centrally adjust umask for all of the
> testsuite.

I've just played a bit with umask and it seems to work nice with something 
like 'touch', but python's tempfile.mkstemp() still always creates files with 
0600 permissions, regardless of umask set for the same directory.

> 
> Henning
> 
> >          cmdline = start_vm.format_qemu_cmdline(arch, build_dir,
> > 
> > distro, output_file, None)

Re: [PATCH] ci: make vm_start logs readable by all users

[Henning Schild]

Am Mon, 10 Jan 2022 15:50:00 +0300
schrieb Uladzimir Bely <ubely@ilbers.de>:

> In mail from вторник, 4 января 2022 г. 22:58:39 +03 user Henning
> Schild wrote:
> > Am Thu, 23 Dec 2021 06:57:20 +0100
> > 
> > schrieb Uladzimir Bely <ubely@ilbers.de>:  
> > > This changes permissions for ${BUILD_DIR}/vm_start_.*_log.txt
> > > from 600 to 644 to make them readable not only for jenkins user.  
> > 
> > not everybody uses jenkins ... make it "CI user"
> >   
> 
> OK.
> 
> > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
> > > ---
> > > 
> > >  testsuite/vm_boot_test/vm_boot_test.py | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/testsuite/vm_boot_test/vm_boot_test.py
> > > b/testsuite/vm_boot_test/vm_boot_test.py index 01623c5c..69e7f4ae
> > > 100644 --- a/testsuite/vm_boot_test/vm_boot_test.py
> > > +++ b/testsuite/vm_boot_test/vm_boot_test.py
> > > 
> > > @@ -46,6 +46,7 @@ class VmBase(Test):
> > >          fd, output_file = tempfile.mkstemp(suffix='_log.txt',
> > >          
> > >                                             prefix='vm_start_' +
> > > 
> > > distro + '_' + arch + '_', dir=build_dir, text=True)
> > > +        os.chmod(output_file, 0o644)  
> > 
> > This smells like other output files could also have wrong
> > permissions while being relevant.  
> 
> It looks like a specific behaviour of tempfile.mkstemp() function,
> that always creates file readable/writable only by the user ID. So,
> only vm logs are affected.

Yes, apparently that one is trying especially hard to make things
"secure". You could also switch to 

with tempfile.TemporaryFile() as fp

in which case you should not have such problems.

Henning

> > If that might be the case, maybe centrally adjust umask for all of
> > the testsuite.  
> 
> I've just played a bit with umask and it seems to work nice with
> something like 'touch', but python's tempfile.mkstemp() still always
> creates files with 0600 permissions, regardless of umask set for the
> same directory.
> 
> > 
> > Henning
> >   
> > >          cmdline = start_vm.format_qemu_cmdline(arch, build_dir,
> > > 
> > > distro, output_file, None)  
> 
> 
> 
> 

Re: [PATCH] ci: make vm_start logs readable by all users

[Uladzimir Bely]

In mail from понедельник, 10 января 2022 г. 16:32:46 +03 user Henning Schild 
wrote:
> Am Mon, 10 Jan 2022 15:50:00 +0300
> 
> schrieb Uladzimir Bely <ubely@ilbers.de>:
> > In mail from вторник, 4 января 2022 г. 22:58:39 +03 user Henning
> > 
> > Schild wrote:
> > > Am Thu, 23 Dec 2021 06:57:20 +0100
> > > 
> > > schrieb Uladzimir Bely <ubely@ilbers.de>:
> > > > This changes permissions for ${BUILD_DIR}/vm_start_.*_log.txt
> > > > from 600 to 644 to make them readable not only for jenkins user.
> > > 
> > > not everybody uses jenkins ... make it "CI user"
> > 
> > OK.
> > 
> > > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
> > > > ---
> > > > 
> > > >  testsuite/vm_boot_test/vm_boot_test.py | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/testsuite/vm_boot_test/vm_boot_test.py
> > > > b/testsuite/vm_boot_test/vm_boot_test.py index 01623c5c..69e7f4ae
> > > > 100644 --- a/testsuite/vm_boot_test/vm_boot_test.py
> > > > +++ b/testsuite/vm_boot_test/vm_boot_test.py
> > > > 
> > > > @@ -46,6 +46,7 @@ class VmBase(Test):
> > > >          fd, output_file = tempfile.mkstemp(suffix='_log.txt',
> > > >          
> > > >                                             prefix='vm_start_' +
> > > > 
> > > > distro + '_' + arch + '_', dir=build_dir, text=True)
> > > > +        os.chmod(output_file, 0o644)
> > > 
> > > This smells like other output files could also have wrong
> > > permissions while being relevant.
> > 
> > It looks like a specific behaviour of tempfile.mkstemp() function,
> > that always creates file readable/writable only by the user ID. So,
> > only vm logs are affected.
> 
> Yes, apparently that one is trying especially hard to make things
> "secure". You could also switch to
> 
> with tempfile.TemporaryFile() as fp
> 
> in which case you should not have such problems.
> 
> Henning
> 

Yes, I looked for some alternatives. Probably, we could use 
tempfile.NamedTemporaryFile() instead, because it allows to keep temporary 
file in the system (if argument delete=False).

> > > If that might be the case, maybe centrally adjust umask for all of
> > > the testsuite.
> > 
> > I've just played a bit with umask and it seems to work nice with
> > something like 'touch', but python's tempfile.mkstemp() still always
> > creates files with 0600 permissions, regardless of umask set for the
> > same directory.
> > 
> > > Henning
> > > 
> > > >          cmdline = start_vm.format_qemu_cmdline(arch, build_dir,
> > > > 
> > > > distro, output_file, None)