encryption privacy fix: do not add keys with partial address match #84
Conversation
Fixes a privacy issue caused by a partial match of a recipient address with a public key uid in your keyring. For example, when sending an encrypted mail to eve@example.org, and there is a public key for steve@example.org in your keyring, steve will be able to decrypt the message.
dodo/pgp_util.py
Outdated
| # "mail@example.org <mail@example.org>" are skipped. These uids are considered | ||
| # malformed, but they occur. | ||
| recipients_keys = [key['fingerprint'] for key in Gpg.list_keys() | ||
| if any(addr == email.utils.parseaddr(uid, strict=False)[1] |
There was a problem hiding this comment.
| if any(addr == email.utils.parseaddr(uid, strict=False)[1] | |
| if any(addr == utils.strip_email_address(uid, strict=False) |
from here maybe?
Lines 282 to 287 in 194fb49
Would probably have to add strict=True to that, so feel free to ignore if you think your approach is simpler here.
There was a problem hiding this comment.
Oh, I notice now that strict=True was only added in Python 3.12.6 (released not even a month ago), while Dodo claims to support older versions.
I suppose something like kwargs = {"strict": True} if strict and sys.version_info[:3] >= (3, 12, 6) else {} followed by email.utils.parseaddr(e, **kwargs) would do? Or catching TypeError I guess.
| # malformed, but they occur. | ||
| recipients_keys = [key['fingerprint'] for key in Gpg.list_keys() | ||
| if any(addr == email.utils.parseaddr(uid, strict=False)[1] | ||
| for uid in key['uids'] for addr in recipients)] |
There was a problem hiding this comment.
I feel like this gets quite cramped with [... for ... in ... if any(... for ... in ... for ... in ...)]. That's a lot going on in one list comprehension. What do you think about a (untested):
def find_gpg_keys(recipients: list[str]) -> list[str]:
keys = []
for key in Gpg.list_keys():
for uid in key["uid"]:
address = utils.strip_email_address(uid, strict=False)
if address in recipients:
keys.append(key["fingerprint"])or maybe
def find_gpg_keys(recipients: list[str]) -> list[str]:
keys = []
for key in Gpg.list_keys():
addresses = {utils.strip_email_address(uid, strict=False) for uid in key["uids"]}
if addresses & recipients:
keys.append(key["fingerprint"])There was a problem hiding this comment.
I removed the strict parameter and used the util.strip_email_address instead. I will address the case where gnupg.list_keys emits malformed email addresses later, together with your suggestions above. Note that the behavior of util.strip_email_address changes in python 3.12.6 but I think this will not have a big impact on dodo because, at first sight, notmuch transforms mail addresses such as mail@example.com <mail@example.com> to "mail@example.com" <mail@example.com> which are parsed correctly by the new email.utils.parseaddr (and also by the golang validation mentioned on issue #102988)
The strict parameter for email.utils.parseaddr is only available in python version 3.12.6 and above.
Fixes a privacy issue caused by a partial match of a recipient address with a public key uid in your key ring. For example, when sending an encrypted mail to eve@example.org, and there is a public key for steve@example.org in your key ring, Steve will be able to decrypt the message.
Fixes #70.
Thanks @The-Compiler for spotting this.