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

[FEATURE REQUEST] Simple gpg signature verification of file downloads #63143

Closed
lkubb opened this issue Nov 30, 2022 · 6 comments · Fixed by #63611
Closed

[FEATURE REQUEST] Simple gpg signature verification of file downloads #63143

lkubb opened this issue Nov 30, 2022 · 6 comments · Fixed by #63611
Labels
Feature new functionality including changes to functionality and code refactors, etc. needs-triage

Comments

@lkubb
Copy link
Contributor

lkubb commented Nov 30, 2022

Is your feature request related to a problem? Please describe.
Sometimes I write formulae to install builds from Github releases (or similar). If they offer cryptographic signatures, I would like to be able to securely and easily verify them. This includes verifying the signature has been made by a particular (set of) key(s), not everything imported in my keyring (ref apt-key deprecation). (This has been split into a separate FR: #63166)

Describe the solution you'd like

# [...] use gpg.present to receive keys

Verify release:
  gpg.verified:
    - name: /my/file.tar.gz
    - signature: /my/file.tar.gz.asc
    - signed_by_any:  # or signed_by_all
      - deadbeef
      - cafebabe
      - d00d1337

Edit:
More semantical sense, with the caveat that an implementation needs to be properly validated because of how much those states already handle, would make to add signature andsource_hash_sig (+supporting) parameters to file.managed and archive.extracted:

Install latest release:
  file.managed:
    - name: /opt/awesome/software
    - source: https://github.com/awesome/software/releases/42/awesome-software-42-x84_64
    - source_hash: https://github.com/awesome/software/releases/42/awesome-software-42-x84_64.SHA256
    - source_hash_sig: True  # verifies an embedded signature in the source hash file
    - mode: '0755'
Install latest release:
  file.managed:
    - name: /opt/awesome/software
    - source: https://github.com/awesome/software/releases/42/awesome-software-42-x84_64
    - source_hash: https://github.com/awesome/software/releases/42/awesome-software-42-x84_64.SHA256
    - signature: https://github.com/awesome/software/releases/42/awesome-software-42-x84_64.asc
    - mode: '0755'

Describe alternatives you've considered
This monster [does not contain the limitation to a particular (set of) key(s)]:

# [...] some lvl4 incantation to make sure the following
# does not run without the key actually being present

Vaultwarden web vault signature is verified:
  test.configurable_test_state:
    - name: Check if the downloaded web vault archive has been signed by the author.
    - changes: False
    - result: >
        __slot__:salt:gpg.verify(filename=/tmp/web-vault-{{ warden.version_web_vault }}.tar.gz,
        signature=/tmp/web-vault-{{ warden.version_web_vault }}.tar.gz.asc).res
    - require:
      - Vaultwarden key is actually present
      - Vaultwarden web vault is downloaded

Additional context
Already wrote the implementation, PR pending me writing tests for it and fixing another related, quite aggravating issue with gpg.receive_keys / gpg.present.

@lkubb lkubb added Feature new functionality including changes to functionality and code refactors, etc. needs-triage labels Nov 30, 2022
@OrangeDog
Copy link
Contributor

OrangeDog commented Nov 30, 2022

I don't think this works as a state. It's a consistency check, not actually making any change if it fails.

Also, your workaround is unnecessarily complex. You can just use module.run:

Vaultwarden web vault signature is verified:
  module.run:
    - gpg.verify:
      - filename: /tmp/web-vault-{{ warden.version_web_vault }}.tar.gz
      - signature: /tmp/web-vault-{{ warden.version_web_vault }}.tar.gz.asc
    - require:
      - Vaultwarden key is actually present
      - Vaultwarden web vault is downloaded

All that's missing is the signed_by_any argument to gpg.verify.

@lkubb
Copy link
Contributor Author

lkubb commented Nov 30, 2022

That state will always pass regardless of the verification result since gpg.verify returns a result dict that evaluates to True {"res": False, "message": "something"}, rendering the check useless. You cannot inspect the result further in the sls context without using slots afaik.

It is a consistency check, yes, but you can act on its failure in subsequent states. I like that it's simple and explicit. An alternative would be to introduce a flag to gpg.verify to only return a boolean result.

@OrangeDog
Copy link
Contributor

OrangeDog commented Nov 30, 2022

Ah, I see.

If acting on the result, the unless/onlyif have a get_return parameter that lets you get to it. e.g.

Vaultwarden web vault signature is verified:
  test.fail_without_changes:
    - unless:
      - fun: gpg.verify:
        filename: /tmp/web-vault-{{ warden.version_web_vault }}.tar.gz
        signature: /tmp/web-vault-{{ warden.version_web_vault }}.tar.gz.asc
        get_return: res
    - require:
      - Vaultwarden key is actually present
      - Vaultwarden web vault is downloaded

@lkubb
Copy link
Contributor Author

lkubb commented Nov 30, 2022

Huh, I didn't know about that. Thanks for showing me! This seems a bit simpler than using slots.

I still think having a gpg.verified "state" makes sense. For security and usability reasons, being explicit is important imo. Your solution still requires quite a bit of knowledge about internal workings for a simple signature check.

@OrangeDog
Copy link
Contributor

file.managed already has a parameter to check hashes, possibly it could be extended to do signatures, but that's already a very busy state.

In any case, certainly the gpg.verify function can have the parameter to specify key ids.

@lkubb
Copy link
Contributor Author

lkubb commented Nov 30, 2022

Yup, file.managed would be the semantically correct place for this "state", but I have a bit of hesitation adapting that one since, as you mentioned, it's quite complicated already. Also, sometimes the hashes are signed, other times the file itself, which might complicate it further.

I will submit the signed_by_[any/all] and gpg.verified PRs separately once ready. Thanks again for your input, much appreciated!

@lkubb lkubb changed the title [FEATURE REQUEST] gpg.verified/gpg.signed_by state [FEATURE REQUEST] Simple gpg signature verification of file downloads Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. needs-triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants