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

Log offending key ID on import failure #1162

Closed
kaie opened this issue Jun 4, 2020 · 6 comments · Fixed by #1166
Closed

Log offending key ID on import failure #1162

kaie opened this issue Jun 4, 2020 · 6 comments · Fixed by #1166

Comments

@kaie
Copy link
Contributor

kaie commented Jun 4, 2020

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
Copy link
Contributor Author

kaie commented Jun 4, 2020

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
Copy link
Contributor Author

kaie commented Jun 4, 2020

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
Copy link
Contributor Author

kaie commented Jun 4, 2020

@ni4
Copy link
Contributor

ni4 commented Jun 5, 2020

@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
Copy link
Contributor Author

kaie commented Jun 6, 2020

Thanks for this improvement. However, when importing the (failing) key from issue #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
Copy link
Contributor

ni4 commented Jun 8, 2020

@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

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

Successfully merging a pull request may close this issue.

2 participants