search

rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
194 stars 55 forks source link

Log offending key ID on import failure #1162

Closed kaie closed 4 years ago

kaie commented 4 years ago

If import of a key block with rnp_import_keys with many keys is failing (because some key in the middle causes the import to return with failure), then it's currently impossible to find out which key was the offending key.

I suggest to dump the key ID of the offending key next to the error log describing the failure.

kaie commented 4 years ago

Suggested patch: https://gist.github.com/kaie/e87898c4a0de65569ab49e148fc26b25

Please let me know if you like the idea. Please feel free to take that gist and simply add it to your own patch or pull request, it always takes me a lot of work to fork/clean/push/pull-request. But let me know if you want me to do that, and I will.

kaie commented 4 years ago

Ideally, it would be good it the API could also offer the application to obtain the failing key ID.

If I understand correctly, currently, if rnp_import_keys fails, the output json parameter is undefined (and you don't set it).

How about abusing that output string parameter, and simply set the output string failing to the hex key id (or fingerprint), without a json structure? Or, if you feel this is hacky, having the error output as JSON would be acceptable, too.

But having just the RNP_LOG would be a helpful first step, if that can be done more quickly.

kaie commented 4 years ago

tracking this at https://bugzilla.mozilla.org/show_bug.cgi?id=1637508

ni4 commented 4 years ago

@kaie Thanks, I also had suggestion that logging should be improved somehow. Updated it in #1166. I did it in slightly other way via define, so correct file name/line numbers will be printed.

Also will file separate issue to return errored key(s) fingerprints during import. Currently it is hard to implement quickly since would require a lot of internal code changes.

kaie commented 4 years ago

Thanks for this improvement. However, when importing the (failing) key from issue https://github.com/rnpgp/rnp/issues/1163 then the key 6E620C6562D66B3D isn't printed (as with my original patch), it only prints the id of the failing subkey (00a117805ea58ed0).

I think it would be helpful to also print the primary key ID of the failing key.

ni4 commented 4 years ago

@kaie I believed that it is possible to easily locate primary key via subkey's keyid. However it seems not an easy task, so updated dumping with PR #1169