search

isislovecruft / python-gnupg

A modified version of python-gnupg, including security patches, extensive documentation, and extra features.
Other
424 stars 172 forks source link

verify_file does not work in py3k with binary files #217

Open weatherhead99 opened 6 years ago

weatherhead99 commented 6 years ago

Hi, I was having trouble using the verify_file method on binary files with python3. A minimum working example which demonstrates the problem can be found at: https://gist.github.com/weatherhead99/22d0da1fe374ac538d3ead7cc23fdc81

These signatures validate running in python2 but not python3

(files used for signatures etc are attached, but it works for any binary vs text file)

test_message.tar.gz.asc.txt](https://github.com/isislovecruft/python-gnupg/files/1599605/test_message.tar.gz.asc.txt) test_message.txt.asc.txt pubkey.txt test_message.tar.gz test_message.txt

I also have a potential fix/workaround: commenting out the following two lines in _util.py (line 203) fixes this issue, at least on my machine. I'm not clear what the rationale of this code is, but it might be for different terminal encodings?

if _py3k and isinstance(data, bytes): encoded = coder.encode(data.decode(coder.name))[0]

I can't find anywhere else that the binary(data) function is used except _threaded_copy_data(), which in turn only seems to be used in verify_file()...

davidkhess commented 6 years ago

I ran into the same thing. If you are only dealing with binary files, this monkey patch works too:

import gnupg._util
def binary(data):
    return data
gnupg._util.binary = binary
isislovecruft commented 6 years ago

Hi @weatherhead99! Thanks for the patches!

I think there might be a typo in your https://gist.github.com/weatherhead99/22d0da1fe374ac538d3ead7cc23fdc81 script? Although (and my apologies if so) it might just be a version difference since it's unfortunately taken me a while to get to your patch. The problem is with the assert(import_key_result.results[0]["fingerprint"] is not None) line… since that key is expired, it needs to be changed to assert(import_key_result.results[2]["fingerprint"] is not None). This is entirely unintuitive and supremely unhelpful and we should totally fix this. Indexing should totally not depend on key validity.

weatherhead99 commented 6 years ago

@isislovecruft yes you're absolutely right - though I think the problem might not be the version but that I generated the keys with an expiry date before the date which you ran the example :-)