Skip to content

Conversation

@andyfoston
Copy link

SUMMARY

Make apt_repository use HTTPS instead of HTTP when receiving keys from Ubuntu PPA repositories.

This more closely aligns with Debian's/Ubuntu's apt-add-repository CLI tool, and helps ensure apt_repository PPA installations can be used by organisations that block outbound unsecured HTTP traffic.

ISSUE TYPE
  • Feature Pull Request
ADDITIONAL INFORMATION

Debian's software-properties/ HTTPS usage commit (I'm not able to find the main repo for software-properties)


@ansibot ansibot added feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. module This issue/PR relates to a module. labels Aug 13, 2024
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Aug 13, 2024
@s-hertel
Copy link
Contributor

s-hertel commented Aug 13, 2024

Snippet from the commit linked:

 -- Andrea Azzarone <andrea.azzarone@canonical.com>  Tue, 03 Apr 2018 16:04:32 +0200

software-properties (0.96.24.25) bionic; urgency=medium

  * ppa.py:
   - rework key retrieval, instead of using hkp & gnupg/dirmngr, use https
     & python's built in urllib.
   - thus, add-apt-key for PPAs observes https_proxy for key retrieval
   - simplify gnupg operations, depend on gpg package only, and use
     import/public key operations only.
   - fix unicode process output bugs, when operating in a non-UTF-8
     locale, thus enabling to import keys for my ppas in C locale.
   - avoid creating trustdb, or requiring any gpg-agent systemd socket to
     be activated
   - update tests to execute key addition fully with less things stubbed
     out with mock
   - stop using apt-key for installing keys
   - dirmngr is a heavy dependency and not used, and it is hard to pass
     proxy information to it when invoking gpg from a non-standard homedir
   - deprecate --keyserver option, making HTTPS access to
     keyserver.ubuntu.com required
   - LP: #1755192, LP: #1713962, LP: #1699086, LP: #1510220, LP: #1433761,
     LP: #1395321, LP: #1312267

In the main repo I see https://salsa.debian.org/pkgutopia-team/software-properties/-/commit/a3731eb8d161a305e4e48413504a984687f620cf.

It looks like --keyserver was removed in https://salsa.debian.org/pkgutopia-team/software-properties/-/commit/bed1f867edd228b025dfae20c88009c0d2d9b0e0.

This could be a breaking change. I'd be in favor of changing the default but making it configurable for backwards compatibility.

Since apt-key (excluding apt-key del) is deprecated too, the module should manage the keyring with gpg instead.

@andyfoston
Copy link
Author

Thank-you for the feedback. I've made a start on this, but I'll have to pick this up next week now

@andyfoston andyfoston force-pushed the make-add-repository-use-https-ppa branch from cc5b08b to e6a0b71 Compare August 20, 2024 15:31
@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Aug 21, 2024
@ansibot
Copy link
Contributor

ansibot commented Aug 21, 2024

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/apt_repository.py:569:28: E111: indentation is not a multiple of 4
lib/ansible/modules/apt_repository.py:571:161: E501: line too long (209 > 160 characters)
lib/ansible/modules/apt_repository.py:574:161: E501: line too long (165 > 160 characters)

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/apt_repository.py:567:28: disallowed-name: Disallowed name "_"

click here for bot help

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 21, 2024
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 21, 2024
@andyfoston
Copy link
Author

Hey @s-hertel thanks for the suggestions. I've re-ordered things now so that this tries using GPG first and falls back to apt-key.

I found that gpg didn't work for me as it was, so I've changed some things to get this working. Among other things, I've changed the ordering of the APT_KEY_DIRS as this only seems to work if the key is written to /etc/apt/trusted.gpg.d.

As apt-key is deprecated, and so is gpg --keyserver from what I've read, so I've created another PR that stops using these deprecated options/binaries for PPA repos. This is a bigger change and restructures how the apt repository files are written, so not sure if this is too big of a breaking change to get merged. I've raised this as a draft to try and avoid people reviewing it unnecessarily but I can put this in for review if you'd prefer?

@s-hertel
Copy link
Contributor

@andyfoston Good catch, I missed that --keyserver is deprecated in gpg. I tested it with 2.4.0, and it worked and didn't emit a deprecation warning, but it looks like it was deprecated in favor of configuring the keyserver with dirmngr in gnupg >= 2.1.9. I can't find when it is supposed to be removed though...

gnupg 2.1.10 adds support for configuring multiple keyservers.

gnupg 2.3.2 changes the default keyserver from hkps://hkps.pool.sks-keyservers.net (deprecated) to hkps://keyserver.ubuntu.com (which is still what it is today).

What do you think about checking the gpg version, and passing --keyserver for versions < 2.3.2 (since the default keyserver is broken on earlier versions), and otherwise not passing --keyserver, so gpg can use the configured keyservers (which is secure by default)? I think this would eliminate the need to use fetch_file in the other PR.

The backwards incompatibility for old content that relies on the http keyserver still seems like an issue (for both PRs), but I'm not sure about adding a module option that's only applicable for old versions of gpg to provide a way to restore the insecure behavior, so I'm curious what others think about that. Maybe it just needs to be pointed out as a potential breaking change in the porting guide.

The change in this PR to reorder the APT_KEY_DIRS also seems like a backward incompatible change. I found this to be a helpful explanation about the apt-key deprecation https://askubuntu.com/questions/1286545/what-commands-exactly-should-replace-the-deprecated-apt-key/1307181#1307181, and in light of that, I think preferring /etc/apt/trusted.gpg.d is a step in the wrong direction.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 29, 2024
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jun 10, 2025
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year. stale_review Updates were made after the last review and the last review is more than 7 days old.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants