Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] 3002.5 pkgrepo.managed accepts insecure key_url #59786

Closed
Tracked by #59408
OrangeDog opened this issue Mar 12, 2021 · 1 comment · Fixed by #63004
Closed
Tracked by #59408

[BUG] 3002.5 pkgrepo.managed accepts insecure key_url #59786

OrangeDog opened this issue Mar 12, 2021 · 1 comment · Fixed by #63004
Labels
Bug broken, incorrect, or confusing behavior debian affects this operating system severity-critical top severity, seen by most users, serious issues Sulfur v3006.0 release code name and version ubuntu affects this operating system

Comments

@OrangeDog
Copy link
Contributor

Description
From https://wiki.debian.org/DebianRepository/UseThirdParty

The key MUST be downloaded over a secure mechanism like HTTPS

From https://docs.saltproject.io/en/3002/ref/states/all/salt.states.pkgrepo.html#salt.states.pkgrepo.managed

URL to retrieve a GPG key from. Allows the usage of http://, https:// as well as salt://.

Setup

deb http://example.com/repo stable main:
  pkgrepo.managed:
    - file: /etc/apt/sources.list.d/example.list
    - key_url: http://example.com/repo/GPG_KEY.asc

Expected behavior
Salt should refuse to download keys over HTTP by default. It's possibly acceptable to allow forcing it, but that should trigger warnings.

Versions Report

salt --versions-report
Salt Version:
          Salt: 3002.5

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: Not Installed
         gitdb: 2.0.6
     gitpython: 3.0.7
        Jinja2: 2.10.1
       libgit2: 0.28.3
      M2Crypto: 0.31.0
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.6.1
        pygit2: 1.0.3
        Python: 3.8.5 (default, Jan 27 2021, 15:41:15)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: 2.0.5
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2

System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.4.0-66-generic
        system: Linux
       version: Ubuntu 20.04 focal

Additional context
This would probably qualify as a CVE.

@OrangeDog OrangeDog added Bug broken, incorrect, or confusing behavior needs-triage debian affects this operating system ubuntu affects this operating system labels Mar 12, 2021
@bryceml
Copy link
Contributor

bryceml commented Mar 12, 2021

an exception I would consider is if it's hitting localhost, 169.254.169.254 (ec2 metadata url, not sure if it's practical or not) or something like that.

@sagetherage sagetherage changed the title [BUG] pkgrepo.managed accepts insecure key_url [BUG] 3002.5 pkgrepo.managed accepts insecure key_url Mar 15, 2021
@sagetherage sagetherage added severity-critical top severity, seen by most users, serious issues and removed needs-triage labels Mar 31, 2021
@sagetherage sagetherage added this to the Silicon milestone Mar 31, 2021
@sagetherage sagetherage added the Silicon v3004.0 Release code name label Mar 31, 2021
@sagetherage sagetherage removed the Silicon v3004.0 Release code name label Aug 19, 2021
@sagetherage sagetherage modified the milestones: Silicon, Approved, Sulphur Aug 19, 2021
@sagetherage sagetherage added the Sulfur v3006.0 release code name and version label Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior debian affects this operating system severity-critical top severity, seen by most users, serious issues Sulfur v3006.0 release code name and version ubuntu affects this operating system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants