git module verifies signatures after the repo has already been updated

[spanezz]

Summary

I would expect that if some changes are not signed, ansible would leave my working copy unchanged to the last verified version.

Instead, since verify_commit_sign is called at the end of all operations, even if it will fail, working copies will have been updated.

This may mean that a failed playbook might cause production code to get updated to an untrusted/unverified version. Depending on setups, that may have serious consequences.

Issue Type

Bug Report

Component Name

git

Ansible Version

$ ansible --version
ansible 2.10.8

Configuration

$ ansible-config dump --only-changed
ANSIBLE_NOCOWS(/home/enrico/dev/transilience/ansible.cfg) = True

OS / Environment

Debian Bullseye

Steps to Reproduce

---
- hosts: localhost
  tasks:
   - name: test git
     git:
        repo: https://github.com/spanezz/transilience.git
        dest: /tmp/test
        verify_commit: yes
        version: main

It will make a checkout. If a checkout is present to an earlier commit in main, it will update it to the last commit in main

Expected Results

I would expect that if a signature fails to verify, nothing is changed in the filesystem

Actual Results

PLAY [localhost] *************************************************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************************************************
ok: [localhost]

TASK [test git] **************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Failed to verify GPG signature of commit/tag \"main\"", "rc": 1, "stderr": "gpg: Signature made Fri Jul  2 23:30:54 2021 CEST\ngpg:                using RSA key 4AEE18F83AFDEB23\ngpg: Can't check signature: No public key\n", "stderr_lines": ["gpg: Signature made Fri Jul  2 23:30:54 2021 CEST", "gpg:                using RSA key 4AEE18F83AFDEB23", "gpg: Can't check signature: No public key"], "stdout": "", "stdout_lines": []}

PLAY RECAP *******************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   



This error is as expected. The problem is the unexpected side effect on the destination

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibot]

Files identified in the description:

• lib/ansible/modules/git.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

[konstruktoid]

This is due to the fact that fetch doesn't support verification, so the verification is done after the repo has been cloned or updated.

git pull --verify-signatures would propably be an alternative.

https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/git.py#L830
https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/git.py#L1021

[spanezz]

git fetch without signature verification is probably not an issue, while checkout out before verifying is, as it may leave an unverified checkout around.

A possible strategy to deal with this can be to fetch no matter what; then verify the signature; then checkout if the signature is sound

[konstruktoid]

I might be missing something but how to verify the commits without altering unless you're using git pull --verify-signatures?

[spanezz]

Git does offer some way to check signatures without checkout out code, I guess the trick is to find a convenient one for integration in the module, and to use that right give the intrications of signature verification.

I know that at least git show --show-signature can be used to verify the signature on a commit without needing the files to be checked out, although it does need the commit to be fetched into the local repo. I would guess it's fine (and unavoidable, given how git works) to fetch untrusted commits, and that the critical point is the decision what can be safely checked out of the repo