Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] disable key lookup #55

Open
robsdedude opened this issue Dec 5, 2017 · 10 comments
Open

[Feature Request] disable key lookup #55

robsdedude opened this issue Dec 5, 2017 · 10 comments

Comments

@robsdedude
Copy link

robsdedude commented Dec 5, 2017

Hi,

in my app I only want to use keys. So I do not want to look up keys given a mail address. If the key (given as string) does not work I would like an exception to be raised.

I think to implement this one would need to warp this line in a options check and raise some error, if the key couldn't be imported. It might well be that there are other things that have to be changed for this.

@robsdedude robsdedude changed the title [Feature Request] disable key-chain [Feature Request] disable key lookup Dec 5, 2017
@duckdalbe
Copy link

In the line you reference, k is the fingerprint(s) from the imported key-material you provided. If that lookup results in an empty set, GpgmeHelper#17 raises an exception.

To me that looks like just what you want — right?

@robsdedude
Copy link
Author

Nope. If I call GPGME::Key.import(k).imports.map(&:fpr) with an invalid key (pub-key as string) I get an empty Array. Then

GPGME::Key.find(:public, k || r, :encrypt)

Will look up the recipient as fallback.

@duckdalbe
Copy link

As far as I understand, the code of GpgmeHelper doesn't work as you described, because

[] || 'something'
=> []

But the code actually does behave similar to your description (and not like I described), because
GPGME::Key.find() returns all known keys if given an empty array:

GPGME::Key.find(:public, [], :encrypt)
=> [#<GPGME::Key pub ...]

So apparently a new option is required to stop the code-flow if no (usable) key could be imported from the given material.

It would also make the code's behaviour less surprising, in my eyes. So I second this request.

@robsdedude
Copy link
Author

robsdedude commented Dec 7, 2017

Sorry, I'm actually a Pythonist and thought

[] || 'something'
=> 'something'

But anyway, just as you said I noticed that (only on some systems) mail-gpg encrypts a mail with all pub-keys in the key ring if given an invalid key (#31)

jkraemer added a commit that referenced this issue Mar 30, 2018
@jkraemer
Copy link
Owner

Does that commit above (which is also part of just released v0.3.2) solve your problem or would you still require an explicit option?

@jkraemer
Copy link
Owner

I think that might not have been enough yet to really raise an error in case of no keys. please try it out with master to include the compact call I just added.

@robsdedude
Copy link
Author

robsdedude commented May 18, 2018

Ok, sorry for taking so long to give you feedback. When given an invalid key Mail::Gpg::GpgmeHelper.keys_for_data([mail_address], {mail_address => broken_key}) will lookup the mail address in the key chain and return only that key. That's an improvement over returning all keys in the key chain but as said, I'd expect an error to be raised here. Or at least have an option to disable the lookup and get an error instead.

Note: The result will be an empty array when the mail address is not stored in the key chain.

Thanks for keeping up the good work. I really appreciate it!

jkraemer added a commit that referenced this issue May 19, 2018
- if the :keys option is present, do not fall back to previously
  imported for any recipient address without a specified key
@jkraemer
Copy link
Owner

Still no error but the just-released version 0.4 should not use any keys you did not specify in the :keys option.

Regarding the option to raise an error, just to make sure I get this right: you want an error to be raised whenever there is a recipient for whom no valid key was present in the :keys argument, correct?

@robsdedude
Copy link
Author

That would be one option (like force_key_import: true or so). The other way (which I thought of in the first place) is to raise the error, when there is a key for a recipient specified for in :keys but it cannot be imported.

Like so:
mail to: [mail_a, mail_b], gpg: {encrypt: true, keys: {mail_a => valid_key}} would use that valid_key for mail_a and look up the key for mail_b.
However, mail to: [mail_a, mail_b], gpg: {encrypt: true, keys: {mail_a => invalid_key}, validate_key: true} (I just made up that option and it's name) would still import the key for mail_b but raise an exception as soon as it tries to import the invalid_key and fails in doing so. I even think, that this behavior should be the default. Silently using fallbacks when the parameters are invalid isn't good programming style I guess. So probably there should be an option keychain_fallback or so which makes gem behave like it does now.

@robsdedude
Copy link
Author

Actually, I have to correct that. I just tested the examples with version 0.4 and it behaves just like I wanted it to. The only thing that could be improved though is that given an invalid key a Mail::Gpg::MissingKeysError: No keys to encrypt to! will be raised. This is a little hard to debug I guess. As a key was specified, it just couldn't be imported (as it's invalid).

azul pushed a commit to riseuplabs/crabgrass-core that referenced this issue Sep 28, 2018
jkraemer/mail-gpg#55
now mail-gpg raises an exception when dealing with invalid keys
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants