fixed gpg.decrypt function

[leifliddy]

What does this PR do?

gpg.decrypt now works when use_passphrase=True

What issues does this PR fix or reference?

Fixes: #62806

Previous Behavior

gpg.decrypt did not function correctly when use_passphrase=True was passed

New Behavior

gpg.decrypt now functions correctly when use_passphrase=True was passed

Merge requirements satisfied?

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

Yes

[leifliddy]

I'll rework the gpg.encrypt function (when use_passphrase=True) once this PR is approved #62896
so we can remove those GPG_1_3_1 checks.
Those version checks reference the unsupported (and dead) gnupg library -- and it's really confusing trying to code around that
especially considering that the salt gpg unit tests don't support the gnupg library.
That PR has already been approved, someone just needs to merge it.

[MKLeb]

Hi @leifliddy, it seems like there was some discussion on the fix in #62896. If this PR depends on that one, I will ask to get more eyes on the other one to see how we want to handle it.

[leifliddy]

@MKLeb Cool, thanks! It seems to be a bit of a divisive topic. If Gareth wants to "follow our normal deprecation path" I'd just like some clarification on what that means. There's no salt documentation that lists gnupg as a supported library so I'm sure how to deprecate something that's not supported. Could you please see what you can do?

[MKLeb]

@leifliddy The GPG version check PR has been merged... were you planning on adding the encryption changes to this PR or a separate one?

[leifliddy]

I can do it on this one. I just need a day or so to rebase the current changes and to add the encryption changes.

[leifliddy]

I realize that this PR is all over the place. It's technically resolving an issue with the decrypt and encrypt functions. But, it also introduces the new argument always_trust
So without always_trust, gpg.encrypt will not encrypt a message with a key that's not ultimately trusted without prompting the user. So it's not possible to automate that without always_trust=True or by trusting the key via the trust_key function
So I needed that always_trust in order for the unit test to complete. Plus, it's a useful argument to have anyway.

I couldn't for the life of me develop a unit test that ran through the trust_key function --which uses cmd.run_all to run gpg to trust a key

res = __salt__["cmd.run_all"](cmd, stdin=stdin, runas=_user, python_shell=False)

I just didn't know how to mock that -- in any case it's probably better testing-wise to isolate functions as much as possible (in the unit tests) to avoid the need for dependent functions.

On another note, I don't see why we can't just use the trust_keys function from the python-gnupg module
https://github.com/vsajip/python-gnupg/blob/master/gnupg.py
That function exists for a reason ; )

That would provide the added functionality of being able to process multiple keys at once while not relying on cmd.run_all to do that.
I'll save that for a future update though...

Anyways, let me know what you think...

[leifliddy]

I see that four people had already approved this...and so I really hate to do this, but I needed to make one last change.

    if output:
        result = gpg.encrypt_file(
            _fp,
            recipients,
            passphrase=gpg_passphrase,
            sign=sign,
            always_trust=always_trust,
        )

There's no need to do a conditional check on output here. If output=None then it's ignored by gpg.encrypt.
I just happened to miss that. We just need this

    elif filename:
        with salt.utils.files.flopen(filename, "rb") as _fp:
            result = gpg.encrypt_file(
                _fp,
                recipients,
                sign=sign,
                passphrase=gpg_passphrase,
                always_trust=always_trust,
                output=output,
            )

[Ch3LL]

No worries. We will wait to re-review after you push your changes

[leifliddy]

Thanks. The change has already been pushed.

[Ch3LL]

Oh I see, The commit was before your comment. Thanks, i'll review right now

[Ch3LL]

Just a minor request