ymasson commented 6 months ago

Description I use hardened Debian servers. This include a default UMASK enforced to 027 (0640 for files). This hardening is executed after the Debian installation. So, /etc/apt/sources.list mode is 0644.

I have a state managing /etc/apt/sources.list.d/salt.list using pkgrepo.managed. On every state.highstate the mode of /etc/apt/sources.list is modified to 0640 without any change reported.

Setup use a debian:bullseye Docker container. install Salt via Salt's repositories and apt-get install salt-minion. delete all .list files in /etc/apt/sources.list.d/.

create a state like

root@813ebf88c190:/# cat srv/salt/test/init.sls
test:
  pkgrepo.managed:
    - comments: salt
    - name: deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main
    - file: /etc/apt/sources.list.d/salt.list
    - key_url: https://repo.saltproject.io/salt/py3/debian/11/amd64/SALT-PROJECT-GPG-PUBKEY-2023.gpg
    - gpgcheck: 1
    - aptkey: False
    - refresh: True
    - clean_file: True

change the /etc/apt/sources.list mode to 0640 (simulate a different mode than the default UMASK). run a salt-call --local state.apply test check the /etc/apt/sources.list mode.

[ x] on-prem machine
[x ] VM (Virtualbox, KVM, etc. please specify)
[x ] VM running on a cloud service, please be explicit and add details
[x ] container (Kubernetes, Docker, containerd, etc. please specify)
[ ] or a combination, please be explicit
[ ] jails if it is FreeBSD
[ ] classic packaging
[x ] onedir packaging
[ ] used bootstrap to install

Steps to Reproduce the behavior I use docker run -it --rm debian:bullseye , install Salt and remove the .list file

mkdir /etc/apt/keyrings
curl -fsSL -o /etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg https://repo.saltproject.io/salt/py3/debian/11/amd64/SALT-PROJECT-GPG-PUBKEY-2023.gpg
echo "deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main" >/etc/apt/sources.list.d/saltstack.list
apt-get update
apt-get install salt-minion
rm /etc/apt/sources.list.d/saltstack.list

Then create the state and

root@813ebf88c190:/# chmod 640 /etc/apt/sources.list
root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r----- 1 root root 430 Mar 11 18:37 /etc/apt/sources.list

root@813ebf88c190:/# salt-call --local state.apply test
/opt/saltstack/salt/lib/python3.10/site-packages/salt/ext/tornado/netutil.py:493: DeprecationWarning: ssl.PROTOCOL_TLS is deprecated
  context = ssl.SSLContext(
local:
----------
          ID: test
    Function: pkgrepo.managed
        Name: deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main
      Result: True
     Comment: Configured package repo 'deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main'
     Started: 18:40:45.495649
    Duration: 1914.183 ms
     Changes:   
              ----------
              repo:
                  deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   1.914 s

root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r--r-- 1 root root 430 Mar 11 18:40 /etc/apt/sources.list

root@813ebf88c190:/# chmod 640 /etc/apt/sources.list
root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r----- 1 root root 430 Mar 11 18:40 /etc/apt/sources.list

root@813ebf88c190:/# salt-call --local state.apply test
/opt/saltstack/salt/lib/python3.10/site-packages/salt/ext/tornado/netutil.py:493: DeprecationWarning: ssl.PROTOCOL_TLS is deprecated
  context = ssl.SSLContext(
local:
----------
          ID: test
    Function: pkgrepo.managed
        Name: deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main
      Result: True
     Comment: Configured package repo 'deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main'
     Started: 18:41:14.794378
    Duration: 859.441 ms
     Changes:   

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time: 859.441 ms

root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r--r-- 1 root root 430 Mar 11 18:41 /etc/apt/sources.list

Versions Report

Salt Version:
          Salt: 3006.7

Python Version:
        Python: 3.10.13 (main, Feb 19 2024, 03:31:20) [GCC 11.2.0]

Dependency Versions:
          cffi: 1.14.6
      cherrypy: 18.6.1
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.3
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.15.1
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.10.0-28-amd64
        system: Linux
       version: Debian GNU/Linux 11 bullseye

Additional context If I add

print(reposplit)
print(sanitizedsplit)

just before the ifstatement here https://github.com/saltstack/salt/blob/master/salt/states/pkgrepo.py#L521 if sanitizedsplit != reposplit:

I have

['deb', '[signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg', 'arch=amd64]', 'bullseye', 'https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7', 'main']
['deb', '[arch=amd64', 'signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg]', 'bullseye', 'https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7', 'main']

[] are not excluded.

So, 1: the file mode of /etc/apt/sources.list is modified even though it is not managed by the state. 2: it seems a change is detected due to the bad .split(), but nothing is reported.

fun fact, if I sort [arch=amd64 signed-by=xxxx], nothing append. no mode change, nothing.

ymasson commented 6 months ago

@whytewolf maybe it is similar to https://github.com/saltstack/salt/issues/65703 ?

whytewolf commented 6 months ago

i don't think it is related.

66164 by switching to an OrderedDict and just dropping the weird indexing that was implemented before.

jg-basis commented 5 months ago

I note this issue on Debian 3006.7

saltstack / salt

[BUG] pkgrepo.managed silently change file mode to UMASK #66199

65703 is caused by indexing issues. that issue is going to be fixed in https://github.com/saltstack/salt/pull/66164 by switching to an OrderedDict and just dropping the weird indexing that was implemented before.