Is this fixed now?

@aheinecke thanks for the merge of my other branch! sadly, that branch does *not* address this issue yet. It doesn't even test for it. :( I can work on trying to fix it (and test it) if there's a consensus that we want this particular change in behavior.

One more semantic question about how folks think Context.decrypt(verify=True) should work: if the decrypted thing has no signature at all, should the function succeed without throwing an exception? it currently does, but the returned verify_result has its signatures member set to the empty list.

I think this is correct ("i verified all signatures i found, and here they are (all none of them)"), but it might suprise someone who expects verify=True to throw an error if the data wasn't signed.

Anyway, if this is correct behavior (i think it is), then why should the existence of an invalid signature cause an exception to be thrown at all? If the bad signature was just stripped, no error would be thrown. I'm inclined to say that Context.decrypt() should just filter out (ignore) all bad signatures from its returned verify_result. That will avoid changing the current semantics too drastically -- the current function only ever returns lists of valid signatures -- while making it less brittle in the face of unevaluable signatures.

@dkg If you propose a patch here I'm pretty sure that we will accept it. As one of our Python binding users you know better then us how the API should behave.

I've just published a branch dkg/fix-T4276 (with commit 4100794e305ba22241ea5a4f7b42bb5189fbd948) which i think resolves this issue.

The earlier three patches in the series extend the test suite to make sure that it's doing the right thing

I'm for merging this as I understand the rationale. In Kleo / GpgOL I also only need one valid signature.

But I think as this is an API break, a software that currently relies on the exception might get a security problem if the exception is no longer raised. So I'm a bit cautions to make this change if we don't also bump the python gpgme major version.
Adding a different function like Context.decrypt_ex or something like that also seems bad though. :-/

What is your opinion on that?

I agree that this is a minor API shift, but i *don't* think it's a security problem, because i was particularly careful to maintain the invariant that decrypt(verify=True) will only ever return valid signatures.

Can you describe the logic of a system that depends on the exception?

Note that any tool that depends on a BadSignatures exception is already vulnerable (in the currently-distributed version of python-gpg) to a malicious attacker that removes *all* signatures. in that case, no BadSignatures exception will be raised, and the verify_result will be empty. If you can point me to any downstream projects that depend on the BadSignatures exception, i'd be happy to try an attack and file a bug report if it's successful.

So i do not think that a change to the major version is necessary, and i think this can be merged without any additional security concerns.

Thanks for the explanation. That addresses my concerns.

Using several python versions?

yes, python2.7 and python3.7

So i've been able to (intermittently) reproduce the failures that i think @werner was alluding to here, but not under any circumstances where i can get them to happen reliably to understand what's going on.

The error is (occasionally):

Traceback (most recent call last):
  File "./t-decrypt-verify.py", line 87, in <module>
    assert len(verify_result.signatures) == 1
AssertionError
FAIL: t-decrypt-verify.py

i'm looking for a way to better understand this failure, but currently it only happens every once in a while, and each run of make -j3 distcheck takes minutes to complete, which makes it tough to narrow down the problem.

i'm actually running make -j3 check, since make -j3 distcheck has the problems described in T4688.