isislovecruft / python-gnupg

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

Handle non-existant KeyID/Fingerprint when encrypting #179

Open flexd opened 7 years ago

flexd commented 7 years ago

Hi there! Please correct me if I am doing this wrong.

gpg.encrypt(message, *pgp_keys) where pgp_keys is a list of KeyIDs or Fingerprints.

If I pgp_keys = ['valid key'] or ['valid key 1', 'valid key 2'], it works as expected. Crypt.ok is true, meaning the operation was successful, and Crypt.data has the encrypted data.

However, if pgp_keys = ['valid key1', 'invalid key here'], Crypt.ok is false (which is correct), but Crypt.status is still empty. Does this library not handle a invalid key at all? It seems to me there should be an error message here.

PS: Why can't we use a email or other identifier here to encrypt a message here? GPG allows that on the command line as far as I know.

I notice this is actually not failing in Crypt._handle_status at all, but somewhere else. I have not located where yet. Using Python 3.4.3, latest python-gnupg (fetched yesterday), and gpg 1.4.16.

flexd commented 7 years ago

Right, so I turned on debug logging and figure out what is going on.. It's just passing a empty --recipients to gpg, which is failing.

PGP encrypting 'fooo' to fingerprints: ['<valid key here>', 'foo']
DEBUG:gnupg:GPG.encrypt() called for recipients '('<valid key here>', 'foo')' with type '<class 'tuple'>'
DEBUG:gnupg:Got data '<_io.BytesIO object at 0x7f23331c0888>' with type '<class '_io.BytesIO'>'.
DEBUG:gnupg:Got arg string: --armor
DEBUG:gnupg:Got groups: {'--armor': ''}
DEBUG:gnupg:Appending option: --armor
DEBUG:gnupg:Got arg string: --always-trust
DEBUG:gnupg:Got groups: {'--always-trust': ''}
DEBUG:gnupg:Appending option: --always-trust
DEBUG:gnupg:Got arg string: --cipher-algo AES256
DEBUG:gnupg:_make_filo(): Converted to reverse list: ['AES256', '--cipher-algo']
DEBUG:gnupg:Got arg: --cipher-algo
DEBUG:gnupg:Got value: AES256
DEBUG:gnupg:Got groups: {'--cipher-algo': 'AES256'}
DEBUG:gnupg:Appending option: --cipher-algo AES256
DEBUG:gnupg:Got arg string: --compress-algo ZLIB
DEBUG:gnupg:_make_filo(): Converted to reverse list: ['ZLIB', '--compress-algo']
DEBUG:gnupg:Got arg: --compress-algo
DEBUG:gnupg:Got value: ZLIB
DEBUG:gnupg:Got groups: {'--compress-algo': 'ZLIB'}
DEBUG:gnupg:Appending option: --compress-algo ZLIB
DEBUG:gnupg:Got arg string: --encrypt
DEBUG:gnupg:Got groups: {'--encrypt': ''}
DEBUG:gnupg:Appending option: --encrypt
DEBUG:gnupg:Got arg string: --recipient <valid key here>
DEBUG:gnupg:_make_filo(): Converted to reverse list: ['<valid key here>', '--recipient']
DEBUG:gnupg:Got arg: --recipient
DEBUG:gnupg:Got value: <valid key here>
DEBUG:gnupg:Got groups: {'--recipient': '<valid key here>'}
DEBUG:gnupg:Appending option: --recipient <valid key here>
DEBUG:gnupg:Got arg string: --recipient foo
DEBUG:gnupg:_make_filo(): Converted to reverse list: ['foo', '--recipient']
DEBUG:gnupg:Got arg: --recipient
DEBUG:gnupg:Got value: foo
DEBUG:gnupg:Got groups: {'--recipient': 'foo'}
DEBUG:gnupg:'--recipient foo' not hex.
DEBUG:gnupg:Appending option: --recipient
DEBUG:gnupg:Sending command to GnuPG process:
['/usr/bin/gpg', '--no-options', '--no-emit-version', '--no-tty', '--status-fd', '2', '--homedir', '/home/kristoffer/.gnupg/', '--no-default-keyring', '--keyring', '/home/kristoffer/.gnupg/pubring.gpg', '--secret-keyring', '/home/kristoffer/.gnupg/secring.gpg', '--no-use-agent', '--armor', '--always-trust', '--cipher-algo', 'AES256', '--compress-algo', 'ZLIB', '--encrypt', '--recipient', '3FEB367482588811', '--recipient']
DEBUG:gnupg:<Thread(Thread-4, initial daemon)>, <_io.BytesIO object at 0x7f23331c0888>, <_io.BufferedWriter name=7>
DEBUG:gnupg:Sending 4 bytes of data...
DEBUG:gnupg:stderr reader: <Thread(Thread-5, initial daemon)>
DEBUG:gnupg:Encoded data (type <class 'bytes'>):
b'fooo'
DEBUG:gnupg:stdout reader: <Thread(Thread-6, initial daemon)>
DEBUG:gnupg:Wrote data type <class 'bytes'> to outstream.
DEBUG:gnupg:Reading data from stream '<_io.BufferedReader name=8>'...
DEBUG:gnupg:Finishing reading from stream '<_io.BufferedReader name=8>'...
DEBUG:gnupg:Read    0 bytes total
ERROR:gn[Errno 32] Broken pipetstream <_io.BufferedWriter name=7>:
DEBUG:gnupg:

I notice DEBUG:gnupg:'--recipient foo' not hex.. Could that be a returned error instead? That way this will fail when you give it bad keys. It seems like the point where the second key 'foo' just disappears, but we continue on anyway.

flexd commented 7 years ago

gpg will just return gpg: Missing argument for option "--recipient" for this call.

flexd commented 7 years ago

gpg also supports passing an email (or anything else that identifies a pubkey) for --recipient, is there any particular reason it was decided to only allow KeyIDs and Fingerprints?

flexd commented 7 years ago

@isislovecruft would love your input on this..

isislovecruft commented 7 years ago

Hi @flexd! I would consider a patch which returns an error in some way that is more visible to the programmer; that's a great idea.

gpg also supports passing an email (or anything else that identifies a pubkey) for --recipient, is there any particular reason it was decided to only allow KeyIDs and Fingerprints?

Because you should be specific about which key you want to encrypt to. Also for safety reasons I'd rather not parse nearly completely arbitrary inputs, such as email addresses.