Re: virDomainCoreDumpWithFormat files created as root

noxdafox <noxdafox@xxxxxxxxx> · Sun, 08 Nov 2015 13:43:38 +0200



    I've been spending a bit of time looking into libvirt's code and I
    believe this is not implemented as Daniel first said.

    
    The issue is in the qemuOpenFileAs function in
    src/qemu/qemu_driver.c which ignores the dynamic ownership flag and
    does not set correct ownership on the file. 

    
    The qemuOpenFileAs function is used by other ones as well, so I
    wonder if this affects other QEMU features.

    
    I tried to fix and test it in different use cases and I didn't
    notice any side effect, yet I'd like to provide tests as this
    function is quite a core one and it's implementation is a bit
    confusing (maybe a refactoring would simplify its logic).

    Unfortunately the function is not exposed, therefore unittests are a
    bit challenging. Higher level tests seem to mock the driver
    (src/test/test_driver.c) as well.

    
    Here's the patch fixing the issue. I set the correct uid and gid
    only if the file is being created and dynamic ownership is set.

    
    diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

    index a2cc002..1b47dc6 100644

    --- a/src/qemu/qemu_driver.c

    +++ b/src/qemu/qemu_driver.c

    @@ -2932,6 +2932,11 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t
    fallback_gid,

             if (path_shared <= 0 || dynamicOwnership)

                 vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;

     
    +        if (dynamicOwnership) {

    +            uid = fallback_uid;

    +            gid = fallback_gid;

    +        }

    +

             if (stat(path, &sb) == 0) {

                 /* It already exists, we don't want to delete it on
    error */

                 need_unlink = false;

    
    Please let me know what do you think about it and if we can somehow
    include it in the sources. I'd be glad to offer further help if
    necessary.

    
    On 29/05/15 16:41, Mathew Moon wrote:

    
      Just a hunch but is the binary that creates the dump suid?
        You may want to check the perms on it. Not sure if it is
        libvirtd or some other binary.

        
        Sent from my iPhone
      

        On May 29, 2015, at 3:39 AM, NoxDaFox <noxdafox@xxxxxxxxx>
        wrote:

        
              2015-05-28 12:04 GMT+03:00 Michal
                Privoznik <mprivozn@xxxxxxxxxx>:

                
                    On 28.05.2015 09:29, NoxDaFox wrote:

                      > Greetings,

                      >

                      > I am dumping a guest VM memory for inspection
                      using the command

                      > "virDomainCoreDumpWithFormat" and the created
                      files appear to belong to

                      > root (both user and group).

                      >

                      > I have searched around but didn't find any
                      answer. Is there a way to

                      > instruct QEMU to create those files under a
                      different user?

                      >

                      
                  The coredump should belong to the user:group that the
                  domain in question

                  is running under. If (by default) your qemu processes
                  run under

                  root:root so will the coredump.

                
                I changed default settings but it didn't work. I
                  can see the qemu processes now belonging to my user
                  but yet the core dump is root:root. I used to generate
                  such file with plain QMP 'pmemsave' command and it was
                  creating it under libvirt-qemu group which was a bit
                  better. This new command is way more handy but if I
                  can't sort this issue out I'll need to fallback to the
                  previous one.

                  
                I set in /etc/libvirt/qemu.conf the keys

                  
                  user = "myuser"

                  group = "mygroup"

                  dynamic_ownership = 1

                  
                And restarted libvirtd.

                
                  Although, I'm wondering if we should not disregard
                  this and make

                  coredump be always owned by root:root since a coredump
                  may contain

                  sensitive info, e.g. all kinds of cipher keys. We do
                  that with disk

                  images, so maybe we should reconsider our politics
                  with coredumps.

                  
                      Michal

                    
            This would limit the API quite
              much. There are use cases in which the need is to dump the
              memory for inspection and forcing it to be root would
              cripple the design quite a lot. A policy and an expert
              SysAdmin is all you need in these cases.

              
            In my specific case I'm building a
              test automation system and I was forced to run everything
              as admin or spawn a separate process, with admin rights,
              which would reset the correct user:group at every core
              dump taken. Quite clumsy!

            
        _______________________________________________

          libvirt-users mailing list

          libvirt-users@xxxxxxxxxx

          https://www.redhat.com/mailman/listinfo/libvirt-users
      
    
_______________________________________________
libvirt-users mailing list
libvirt-users@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvirt-users