CZ-NIC / envelope

Insert a message and attachments and send e-mail / sign / encrypt contents by a single line.
Other
171 stars 12 forks source link

GPG: Make add from address to decipherers configurable #14

Closed unicode-it closed 2 years ago

unicode-it commented 3 years ago

In method _get_decipherers the from address is always added to the list and thus a pub key for the sender address is needed.

This isn't wanted in szenarios a system sender account (eg in a web application) is used to send encrypted mail out, put will never ever want or be able to decrypt this mail again.

There should be an Option to set this in encryption method. ATM only overridding method is possible.

unicode-it commented 3 years ago

I can provide a pull request, if wanted...

e3rd commented 3 years ago

Show me an example of code that implements the library but does not work yet please so that I can think over it.

unicode-it commented 3 years ago

I don't think code helps here. In _get_decipherers the sender address is added: return set(x.address for x in self._to + self._cc + self._bcc + ([self.__from] if self.__from else [])). So the sender always needs a pgp pub key. I just think this should be configurable and not assumed. Eg: A website sends encrypted notifications with this library. Then the notifications should not be encrypted for the sender.

e3rd commented 3 years ago

I get the point now. Its true there is a valid use case of not encrypting for the sender, yet having sender in the headers. However it seems a bit difficult to me to design a concise and intuitive API extension.

How the API extension would look like according to your needs please? (If you are making a pull request, please mind to cover both bash and python, write a test and put a line in the CHANGELOG.)

hege-li commented 2 years ago

Yes this is frustrating feature.

For example, if I'm explicitly calling encryption(key="some@recipient") with a certain key, why is self._gnupg.encrypt(recipients=self._get_decipherers()) calling _get_decipherers() in it's current form? It should feed the key I specified as --recipient and nothing more. No To, Cc, Bccs, especially no Froms, I think these should be left to the "auto" mode.

e3rd commented 2 years ago

I agree calling encryption(key=True) (default) would encrypt the message for sender and recipients, calling encryption(key=AUTO) would start encrypting only if all keys are available and calling encryption(key=[]) would encrypt the message only for the given keys.

Since we can pass fingerprints, key IDs, key contents or path names to the encrypt method and since we can call Envelope(encrypt=) or $ envelope --encrypt too, it is not trivial to change the code to reflect the change. Besides, the change should reflect the same behaviours for signing methods too.

(I cannot guarantee the time for this, I'm open to pull requests.)

hege-li commented 2 years ago

I just patched it with a quick hack, probably not correct solution for all cases, but works for now..

    def _get_decipherers(self) -> Set[str]:
        """
        :return: Set of e-mail addresses
        """
        if is_gpg_fingerprint(self._encrypt):
            return self._encrypt
        return set(x.address for x in self._to + self._cc + self._bcc + ([self.__from] if self.__from else []))
clonejo commented 2 years ago

I just patched it with a quick hack, probably not correct solution for all cases, but works for now..

I am now using this patch to override the list of decipherers: https://github.com/clonejo/envelope/commit/354fccba041fc668d70bb555d04da29b02957c6b

e3rd commented 2 years ago

You can now put any keys/fingerprits/addresses/raw keys into the encrypt and friends method. Would you please test that the current implementation covers your use case?

e3rd commented 2 years ago

I'll assigned you, if you do not react I'll take it as solved for you.