zypper_repository: Add option to allow repository metadata to be unsigned

PrimaryCanary commented 3 years ago

Summary

RPM repositories can optionally sign the repository metadata (repomd.xml). The zypper_repository module's inability to disable metadata signing makes it difficult to add repos that don't sign their metadata. While this can be worked around by disabling gpgcheck, doing so is ill-advised.

Issue Type

Feature Idea

Component Name

zypper_repository

Additional Information

Zypper supports skipping the metadata signing check with zypper addrepo --gpgcheck-allow-unsigned-repo ..... This is more secure than zypper addrepo --no-gpgcheck .... because individual packages are still verified. The Ansible's builtin yum_repository module supports this use case with its repo_gpgcheck parameter.

I propose two solutions:

Adding a repo_gpgcheck parameter like that found in yum_repository.
Adding an extra_args parameter like that found in the zypper module.

I'm willing to implement one or both of these solutions. Which would you prefer?

- name: Add ZeroTier repo for SUSE/openSUSE
  community.general.zypper_repository:
    name: zerotier
    description: ZeroTier, Inc. RPM Release Repository
    repo: https://download.zerotier.com/redhat/el/8/
    repo_gpgcheck: no
    state: present

- name: Add ZeroTier repo for SUSE/openSUSE
  community.general.zypper_repository:
    name: zerotier
    description: ZeroTier, Inc. RPM Release Repository
    repo: https://download.zerotier.com/redhat/el/8/
    extra_args: "--gpgcheck-allow-unsigned-repo"
    state: present

Code of Conduct

[X] I agree to follow the Ansible Code of Conduct

ansibullbot commented 3 years ago

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 3 years ago

cc @AnderEnder @alxgu @andytom @commel @dcermak @evrardjp @lrupp @matze @sealor @toabctl click here for bot help

felixfontein commented 3 years ago

I'm against adding something general like extra_args. Having a specific parameter for this makes more sense IMO.

sealor commented 3 years ago

@felixfontein @PrimaryCanary I agree that adding extra_args is too general. I'm fine with a new parameter.

What do you think about adding a parameter for --no-gpgcheck and --gpgcheck-allow-unsigned-repo? This approach allows us to use the most secure way which is available.

How do we want to name the new parameters? gpgcheck: no and gpgcheck_allow_unsigned_repo: yes? IMHO we do not need the repo_ prefix here which is used in the yum configuration.

- name: Add ZeroTier repo for SUSE/openSUSE
  community.general.zypper_repository:
    name: zerotier
    description: ZeroTier, Inc. RPM Release Repository
    repo: https://download.zerotier.com/redhat/el/8/
    gpgcheck: no
    state: present

- name: Add ZeroTier repo for SUSE/openSUSE
  community.general.zypper_repository:
    name: zerotier
    description: ZeroTier, Inc. RPM Release Repository
    repo: https://download.zerotier.com/redhat/el/8/
    gpgcheck_allow_unsigned_repo: yes
    state: present

Useful links: Zypper manual man yum.conf

felixfontein commented 3 years ago

@sealor instead of adding two boolean parameters, you could also add a choices parameter, especially since not all combinations of the booleans make sense (no GPG check while allowing unsigned repos for example). So you could have an option gpg_signature_check with three choices always (default), allow_unsigned_repos, and never. (Or with some better names :) )

sealor commented 3 years ago

Good idea! What do you think about the following choices for gpg_signature_check?

choice	zypper parameter	description
`default`	`--default-gpgcheck`	use global GPG check settings in zypp.conf
`no`	`--no-gpgcheck`	disable all checks
`allow_unsigned`	`--gpgcheck-allow-unsigned`	Short hand for `--gpgcheck-allow-unsigned-repo` and `--gpgcheck-allow-unsigned-package`
`allow_unsigned_repo`	`--gpgcheck-allow-unsigned-repo`	allow the repository metadata to be unsigned
`allow_unsigned_package`	`--gpgcheck-allow-unsigned-package`	allow installing unsigned packages

felixfontein commented 3 years ago

I would avoid no, since boolean processing could change that to false. How about disable instead? Besides that, sounds good!

sealor commented 3 years ago

:+1: disable is fine!

PrimaryCanary commented 3 years ago

I've been implementing this on a local branch and I've run into a zypper bug preventing me from implementing idempotency. The XML output from zypper doesn't seem to be correct with version 1.14.43.

Start by adding any repo with a GPG option: sudo zypper addrepo --gpgcheck-allow-unsigned https://ftp.gwdg.de/pub/linux/misc/packman/suse/openSUSE_Tumbleweed/ packman

Verify its correctness:


$ zypper repos
#  | Alias                     | Name                               | Enabled | GPG Check | Refresh
---+---------------------------+------------------------------------+---------+-----------+--------
1 | packman                   | packman                            | Yes     | ( p) Yes  | No
<more below>

$ cat /etc/zypp/repos.d/packman.repo [packman] enabled=1 autorefresh=0 baseurl=https://ftp.gwdg.de/pub/linux/misc/packman/suse/openSUSE_Tumbleweed/ gpgcheck=1 repo_gpgcheck=0 pkg_gpgcheck=0

3. Verify its XML:

$ zypper --xmlout repos <?xml version='1.0'?>

https://ftp.gwdg.de/pub/linux/misc/packman/suse/openSUSE_Tumbleweed/ ``` Note that the XML GPG options are all one. I could switch the module over to parsing the repo files directly for GPG options (or everything) but it'd increase code complexity by a fair bit.

sealor commented 3 years ago

I notice the same behavior.

$ docker run -ti --rm opensuse/leap /bin/bash -c '
set -x
zypper rr -a

for PARAM in \
  --default-gpgcheck \
  --gpgcheck-strict \
  --no-gpgcheck \
  --gpgcheck-allow-unsigned \
  --gpgcheck-allow-unsigned-repo \
  --gpgcheck-allow-unsigned-package; do
zypper ar $PARAM http://download.opensuse.org/distribution/leap/15.2/repo/oss/ $PARAM
done

zypper --version
zypper repos
zypper --xmlout repos | grep "<repo" | grep -o name.* | sed 's/priority.*autorefresh....//'
'
...
+ zypper --version
zypper 1.14.43
+ zypper repos
Repository priorities are without effect. All enabled repositories share the same priority.

# | Alias                             | Name                              | Enabled | GPG Check | Refresh
--+-----------------------------------+-----------------------------------+---------+-----------+--------
1 | --default-gpgcheck                | --default-gpgcheck                | Yes     | ( p) Yes  | No
2 | --gpgcheck-allow-unsigned         | --gpgcheck-allow-unsigned         | Yes     | ( p) Yes  | No
3 | --gpgcheck-allow-unsigned-package | --gpgcheck-allow-unsigned-package | Yes     | ( p) Yes  | No
4 | --gpgcheck-allow-unsigned-repo    | --gpgcheck-allow-unsigned-repo    | Yes     | ( p) Yes  | No
5 | --gpgcheck-strict                 | --gpgcheck-strict                 | Yes     | ( p) Yes  | No
6 | --no-gpgcheck                     | --no-gpgcheck                     | Yes     | (  ) No   | No
+ zypper --xmlout repos
+ grep '<repo'
+ grep -o 'name.*'
+ sed 's/priority.*autorefresh.....//'
name="--default-gpgcheck" gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="1">
name="--gpgcheck-allow-unsigned" gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="1">
name="--gpgcheck-allow-unsigned-package" gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="1">
name="--gpgcheck-allow-unsigned-repo" gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="1">
name="--gpgcheck-strict" gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="1">
name="--no-gpgcheck" gpgcheck="0" repo_gpgcheck="0" pkg_gpgcheck="0">

sealor commented 3 years ago

After zypper refresh I get the following repository list:

+ zypper repos
Repository priorities are without effect. All enabled repositories share the same priority.

# | Alias                             | Name                              | Enabled | GPG Check | Refresh
--+-----------------------------------+-----------------------------------+---------+-----------+--------
1 | --default-gpgcheck                | --default-gpgcheck                | Yes     | (r ) Yes  | No
2 | --gpgcheck-allow-unsigned         | --gpgcheck-allow-unsigned         | Yes     | (r ) Yes  | No
3 | --gpgcheck-allow-unsigned-package | --gpgcheck-allow-unsigned-package | Yes     | (r ) Yes  | No
4 | --gpgcheck-allow-unsigned-repo    | --gpgcheck-allow-unsigned-repo    | Yes     | (r ) Yes  | No
5 | --gpgcheck-strict                 | --gpgcheck-strict                 | Yes     | (rp) Yes  | No
6 | --no-gpgcheck                     | --no-gpgcheck                     | Yes     | (  ) No   | No
+ zypper --xmlout repos
+ grep '<repo'
+ grep -o 'name.*'
+ sed 's/priority.*autorefresh....//'
name="--default-gpgcheck" type="rpm-md"  gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="0">
name="--gpgcheck-allow-unsigned" type="rpm-md"  gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="0">
name="--gpgcheck-allow-unsigned-package" type="rpm-md"  gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="0">
name="--gpgcheck-allow-unsigned-repo" type="rpm-md"  gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="0">
name="--gpgcheck-strict" type="rpm-md"  gpgcheck="1" repo_gpgcheck="1" pkg_gpgcheck="1">
name="--no-gpgcheck" type="rpm-md"  gpgcheck="0" repo_gpgcheck="0" pkg_gpgcheck="0">

PrimaryCanary commented 3 years ago

I've submitted a bug report to Zypper. In the meantime, a partially complete implementation is available https://github.com/PrimaryCanary/community.general/tree/zypper/gpg-checking. I'll wait to see what the Zypper maintainers say then finish implementing idempotency. Perhaps we can issue a warning for Zypper versions less than X.Y.Z saying Ansible will incorrectly report changed status.

aminvakil commented 3 years ago

As https://github.com/openSUSE/zypper/issues/390 has been closed with a corresponding PR, would you like to open a PR for this issue?

Although I'm not sure when will libzypp=17.27.0 hit opensuse 15.2 and when will ansible test containers get updated, but we can definitely create a PR meanwhile.

ansibullbot commented 3 years ago

Files identified in the description:

plugins/modules/packaging/os/zypper_repository.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 1 year ago

Files identified in the description:

plugins/modules/zypper_repository.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansible-collections / community.general