Warn when using insecure key_urls on apt based systems, and add a kwarg to control whether they are allowed or not

This commit is contained in:
MKLeb 2022-11-02 17:24:55 -04:00 committed by Megan Wilhite
parent bfd72b5186
commit 2e0bd6c5ad
2 changed files with 87 additions and 45 deletions

View file

@ -276,74 +276,81 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs):
purposes of this, ``comps`` should be a comma-separated list.
file
The filename for the ``*.list`` that the repository is configured in.
It is important to include the full-path AND make sure it is in
a directory that APT will look in when handling packages
The filename for the ``*.list`` that the repository is configured in.
It is important to include the full-path AND make sure it is in
a directory that APT will look in when handling packages
dist
This dictates the release of the distro the packages should be built
for. (e.g. ``unstable``). This option is rarely needed.
This dictates the release of the distro the packages should be built
for. (e.g. ``unstable``). This option is rarely needed.
keyid
The KeyID or a list of KeyIDs of the GPG key to install.
This option also requires the ``keyserver`` option to be set.
The KeyID or a list of KeyIDs of the GPG key to install.
This option also requires the ``keyserver`` option to be set.
keyserver
This is the name of the keyserver to retrieve GPG keys from. The
``keyid`` option must also be set for this option to work.
This is the name of the keyserver to retrieve GPG keys from. The
``keyid`` option must also be set for this option to work.
key_url
URL to retrieve a GPG key from. Allows the usage of ``http://``,
``https://`` as well as ``salt://``.
URL to retrieve a GPG key from. Allows the usage of ``http://``,
``https://`` as well as ``salt://``.
.. note::
.. note::
Use either ``keyid``/``keyserver`` or ``key_url``, but not both.
Use either ``keyid``/``keyserver`` or ``key_url``, but not both.
key_text
The string representation of the GPG key to install.
The string representation of the GPG key to install.
.. versionadded:: 2018.3.0
.. versionadded:: 2018.3.0
.. note::
.. note::
Use either ``keyid``/``keyserver``, ``key_url``, or ``key_text`` but
not more than one method.
Use either ``keyid``/``keyserver``, ``key_url``, or ``key_text`` but
not more than one method.
consolidate : False
If set to ``True``, this will consolidate all sources definitions to the
``sources.list`` file, cleanup the now unused files, consolidate components
(e.g. ``main``) for the same URI, type, and architecture to a single line,
and finally remove comments from the ``sources.list`` file. The consolidation
will run every time the state is processed. The option only needs to be
set on one repo managed by Salt to take effect.
If set to ``True``, this will consolidate all sources definitions to the
``sources.list`` file, cleanup the now unused files, consolidate components
(e.g. ``main``) for the same URI, type, and architecture to a single line,
and finally remove comments from the ``sources.list`` file. The consolidation
will run every time the state is processed. The option only needs to be
set on one repo managed by Salt to take effect.
clean_file : False
If set to ``True``, empty the file before configuring the defined repository
If set to ``True``, empty the file before configuring the defined repository
.. note::
Use with care. This can be dangerous if multiple sources are
configured in the same file.
.. note::
Use with care. This can be dangerous if multiple sources are
configured in the same file.
.. versionadded:: 2015.8.0
.. versionadded:: 2015.8.0
refresh : True
If set to ``False`` this will skip refreshing the apt package database
on Debian based systems.
If set to ``False`` this will skip refreshing the apt package database
on Debian based systems.
refresh_db : True
.. deprecated:: 2018.3.0
Use ``refresh`` instead.
.. deprecated:: 2018.3.0
Use ``refresh`` instead.
require_in
Set this to a list of :mod:`pkg.installed <salt.states.pkg.installed>` or
:mod:`pkg.latest <salt.states.pkg.latest>` to trigger the
running of ``apt-get update`` prior to attempting to install these
packages. Setting a require in the pkg state will not work for this.
Set this to a list of :mod:`pkg.installed <salt.states.pkg.installed>` or
:mod:`pkg.latest <salt.states.pkg.latest>` to trigger the
running of ``apt-get update`` prior to attempting to install these
packages. Setting a require in the pkg state will not work for this.
aptkey: Use the binary apt-key. If the command ``apt-key`` is not found
in the path, aptkey will be False, regardless of what is passed into
this argument.
aptkey:
Use the binary apt-key. If the command ``apt-key`` is not found
in the path, aptkey will be False, regardless of what is passed into
this argument.
allow_insecure_key : True
Whether to allow an insecure (e.g. http vs. https) key_url.
.. versionadded:: 3006
"""
if not salt.utils.path.which("apt-key"):
aptkey = False
@ -390,6 +397,21 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs):
# If neither argument was passed we assume the repo will be enabled
enabled = True
# To be changed in version 3008: default to False and still log a warning
allow_insecure_key = kwargs.pop("allow_insecure_key", True)
if allow_insecure_key:
salt.utils.versions.warn_until(
3008,
"allow_insecure_key will default to False starting in salt 3008.",
)
else:
if kwargs.get("key_url", "").strip().startswith("http:"):
ret["result"] = False
ret[
"comment"
] = "Cannot have 'key_url' using http with 'allow_insecure_key' set to True"
return ret
repo = name
if __grains__["os"] in ("Ubuntu", "Mint"):
if ppa is not None:

View file

@ -23,10 +23,10 @@ def test_new_key_url():
Test when only the key_url is changed that a change is triggered
"""
kwargs = {
"name": "deb http://mock/ sid main",
"name": "deb https://mock/ sid main",
"disabled": False,
}
key_url = "http://mock/changed_gpg.key"
key_url = "https://mock/changed_gpg.key"
with patch.dict(pkgrepo.__salt__, {"pkg.get_repo": MagicMock(return_value=kwargs)}):
ret = pkgrepo.managed(key_url=key_url, **kwargs)
@ -38,13 +38,13 @@ def test_update_key_url():
Test when only the key_url is changed that a change is triggered
"""
kwargs = {
"name": "deb http://mock/ sid main",
"name": "deb https://mock/ sid main",
"gpgcheck": 1,
"disabled": False,
"key_url": "http://mock/gpg.key",
"key_url": "https://mock/gpg.key",
}
changed_kwargs = kwargs.copy()
changed_kwargs["key_url"] = "http://mock/gpg2.key"
changed_kwargs["key_url"] = "https://mock/gpg2.key"
with patch.dict(pkgrepo.__salt__, {"pkg.get_repo": MagicMock(return_value=kwargs)}):
ret = pkgrepo.managed(**changed_kwargs)
@ -52,3 +52,23 @@ def test_update_key_url():
assert ret["changes"] == {
"key_url": {"old": kwargs["key_url"], "new": changed_kwargs["key_url"]}
}
def test_managed_insecure_key():
"""
Test when only the key_url is changed that a change is triggered
"""
kwargs = {
"name": "deb http://mock/ sid main",
"gpgcheck": 1,
"disabled": False,
"key_url": "http://mock/gpg.key",
"allow_insecure_key": False,
}
with patch.dict(pkgrepo.__salt__, {"pkg.get_repo": MagicMock(return_value=kwargs)}):
ret = pkgrepo.managed(**kwargs)
assert ret["result"] is False
assert (
ret["comment"]
== "Cannot have 'key_url' using http with 'allow_insecure_key' set to True"
)