Salt: cmd.run does not run as the user's default gid in saltstack 2019.2

Created on 2 Sep 2019 · 17Comments · Source: saltstack/salt

Description of Issue

With salt 2018.3.4, using runas in a cmd.run would run the command with the uid and gid of the user account requested.

In 2019.2.0, it runs with the uid of the user account, but the gid is set to root.

Adding the kwarg group to the state resolves this - but this is not mentioned in the docs.

Setup

# cat > /etc/salt/minion.d/localminion.conf <<EOF
use_master_when_local: False
file_client: local
EOF

# mkdir -p /srv/salt
# cat > /srv/salt/test.sls <<EOF
runas salt:
  cmd.run:
    - name: id
    - runas: salt


runas salt with group:
  cmd.run:
    - name: id
    - runas: salt
    - group: salt
EOF

# useradd  -m -k /etc/skel -s /bin/bash salt

Steps to Reproduce Issue

# salt-call state.apply test
local:
----------
          ID: runas salt
    Function: cmd.run
        Name: id
      Result: True
     Comment: Command "id" run
     Started: 10:11:12.416785
    Duration: 53.337 ms
     Changes:
              ----------
              pid:
                  2082
              retcode:
                  0
              stderr:
              stdout:
                  uid=1009(salt) gid=0(root) groups=0(root),1010(salt) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
----------
          ID: runas salt with group
    Function: cmd.run
        Name: id
      Result: True
     Comment: Command "id" run
     Started: 10:11:12.470421
    Duration: 56.659 ms
     Changes:
              ----------
              pid:
                  2098
              retcode:
                  0
              stderr:
              stdout:
                  uid=1009(salt) gid=1010(salt) groups=1010(salt) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

Summary for local
------------
Succeeded: 2 (changed=2)
Failed:    0
------------
Total states run:     2
Total run time: 109.996 ms

Versions Report

# salt-call --versions-report
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Jun 20 2019, 20:27:34)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.6.1810 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-957.27.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.6.1810 Core

Aluminium Bug CS-R2 CS-S3 Confirmed Documentation ZD doc-rework phase-plan status-to-do

Source

OrlandoArcapix

Most helpful comment

Looks like original fix was here - https://github.com/saltstack/salt/pull/53681

Then it got ported to master here - https://github.com/saltstack/salt/pull/56891

barneysowood on 29 Jul 2020

👍2

All 17 comments

Thanks for submitting! @OrlandoArcapix I don't see a mention of group in the code block either, but perhaps it is just a documentation error.

xeacott on 5 Sep 2019

Shouldn't runas then be named user as well?

srkunze on 11 Sep 2019

related #54457

srkunze on 11 Sep 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

stale[bot] on 7 Jan 2020

Thank you for updating this issue. It is no longer marked as stale.

stale[bot] on 8 Jan 2020

ZD-5064

oeuftete on 7 May 2020

Related: #57223

oeuftete on 14 May 2020

Also ZD-5097.

oeuftete on 14 May 2020

To be clear, there is more than just a documentation issue here. There is a documentation issue for sure, the group argument is not documented. But I'm going to take the doc tags off, since I think that's creating confusion about the larger issue. @sagetherage please correct me if I'm overstepping on that.

The pre-2019 behaviour with just runas: someuser appears to have implied also running as someuser's group. The old behaviour seems like a reasonable expectation; the new behaviour (which may not be intentional) has been unexpected to several users.

cc: @DmitryKuzmenko

oeuftete on 14 May 2020

Just updated to version 3000.2 and ran into the same issue. When using cmd directly I can workaround the issue by providing the desired group.
But not all modules which makes use of cmd module (e.g. git) have the possibilty to provide group information and therefore do their "work" using group "root".

conmarti on 22 Jun 2020

👍1

Was this fixed in Sodium/3001?

I don't see a PR that references this bug, but when when I try it on my Sodium/3001 master/minion, I get the following....

# salt-master --version
salt-master 3001

# cat /srv/salt/show-54378.sls
seems right:
  cmd.run:
    - name: id && id -gn
    - runas: postgres

bug?:
  cmd.run:
    - name: id && id -gn
    - runas: postgres
    - group: redis

workaround?:
  cmd.run:
    - name: id && id -gn
    - runas: postgres
    - kwargs:
      group: redis

# salt saltmaster state.apply show-54378

saltmaster:
----------
          ID: seems right
    Function: cmd.run
        Name: id && id -gn
      Result: True
     Comment: Command "id && id -gn" run
     Started: 14:49:20.840070
    Duration: 72.616 ms
     Changes:
              ----------
              pid:
                  1505
              retcode:
                  0
              stderr:
              stdout:
                  uid=26(postgres) gid=26(postgres) groups=26(postgres) context=system_u:system_r:unconfined_service_t:s0
                  postgres
----------
          ID: bug?
    Function: cmd.run
        Name: id && id -gn
      Result: True
     Comment: Command "id && id -gn" run
     Started: 14:49:20.912972
    Duration: 33.389 ms
     Changes:
              ----------
              pid:
                  1511
              retcode:
                  0
              stderr:
              stdout:
                  uid=26(postgres) gid=994(redis) groups=994(redis),26(postgres) context=system_u:system_r:unconfined_service_t:s0
                  redis
----------
          ID: workaround?
    Function: cmd.run
        Name: id && id -gn
      Result: True
     Comment: Command "id && id -gn" run
     Started: 14:49:20.946595
    Duration: 33.144 ms
     Changes:
              ----------
              pid:
                  1516
              retcode:
                  0
              stderr:
              stdout:
                  uid=26(postgres) gid=994(redis) groups=994(redis),26(postgres) context=system_u:system_r:unconfined_service_t:s0
                  redis

Summary for saltmaster
------------
Succeeded: 3 (changed=3)
Failed:    0
------------
Total states run:     3
Total run time: 139.149 ms

In the above example:

the first state (seems right), the command is run with the gid that matches the user's primary gid.
the second state (bug?) shows cmd.run honouring the -group option
the third state (workaround) shows the proposed workaround that's listed in the OP's issue description

Is anyone still seeing this issue in 3001?

dkfsalt on 29 Jul 2020

@oeuftete can you respond with what you are seeing to DKFsalt's questions?

alexpeay on 29 Jul 2020

Looks like it was still broken on 3000, but it's working for me on 3001:

[root@centos-s-1vcpu-1gb-lon1-01 salt]# salt --version
salt 3001
[root@centos-s-1vcpu-1gb-lon1-01 salt]# salt-call state.apply show-54378 
local:
----------
          ID: Seems right
    Function: cmd.run
        Name: id && id -gn
      Result: True
     Comment: Command "id && id -gn" run
     Started: 15:32:47.409953
    Duration: 75.973 ms
     Changes:   
              ----------
              pid:
                  8005
              retcode:
                  0
              stderr:
              stdout:
                  uid=1001(postgres) gid=1001(postgres) groups=1001(postgres) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
                  postgres
----------
          ID: bug?
    Function: cmd.run
        Name: id && id -gn
      Result: True
     Comment: Command "id && id -gn" run
     Started: 15:32:47.486273
    Duration: 36.726 ms
     Changes:   
              ----------
              pid:
                  8010
              retcode:
                  0
              stderr:
              stdout:
                  uid=1001(postgres) gid=1002(redis) groups=1002(redis),1001(postgres) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
                  redis
----------
          ID: workaround?
    Function: cmd.run
        Name: id && id -gn
      Result: True
     Comment: Command "id && id -gn" run
     Started: 15:32:47.523348
    Duration: 36.748 ms
     Changes:   
              ----------
              pid:
                  8015
              retcode:
                  0
              stderr:
              stdout:
                  uid=1001(postgres) gid=1002(redis) groups=1002(redis),1001(postgres) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
                  redis

Summary for local
------------
Succeeded: 3 (changed=3)
Failed:    0
------------
Total states run:     3
Total run time: 149.447 ms

barneysowood on 29 Jul 2020

Looks like original fix was here - https://github.com/saltstack/salt/pull/53681

Then it got ported to master here - https://github.com/saltstack/salt/pull/56891

barneysowood on 29 Jul 2020

👍2

Nice finds, @barneysowood and @dkfsalt.

What's left for this issue is to get the group argument for the cmd.run state documented, or maybe spun off into a different issue with a clean start.

Supporting the group option in other modules/states that leverage the cmd module (like git as mentioned in https://github.com/saltstack/salt/issues/54378#issuecomment-647360485) probably should have new issues as well.

oeuftete on 30 Jul 2020

👍1

since we didn't use this ticket for the original regression/fix in Magnesium and we committed to the work, we can update this to be a documentation ticket. @ScriptAutomate likely can help, here, too. I prefer smaller issues written for a small PR, but with docs that can be a bit different, it may make sense to keep it here in this ticket with the history of the comments and all.