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

support import and skipping problematic keys #1145

Closed
kaie opened this issue May 20, 2020 · 10 comments · Fixed by #1159
Closed

support import and skipping problematic keys #1145

kaie opened this issue May 20, 2020 · 10 comments · Fixed by #1159
Assignees
Milestone

Comments

@kaie
Copy link
Contributor

kaie commented May 20, 2020

Users who migrate to RNP might want to import their complete public keyring.

Apparently RNP is more strict about keys, or, might not support some kinds of keys, so it refuses to import them.

The problem is:

When importing a large key block that contains many keys, RNP aborts the import at the first problem.

I'd like to request one of the following mechanisms:

  • add an option for rnp_import_keys that says "import what you can, ignore what you cannot import"
    or
  • add a new API that allows me to split a key block, and retrieve a list of individual key blocks for each of the contained keys. With, I could loop, call rnp_import_keys separately for each of the keys, and know directly which keys are problematic.
@ni4
Copy link
Contributor

ni4 commented May 20, 2020

@kaie Today I got to this idea as well, thanks to the data provided by Patrick. At the moment the most logical way seems to be to add flag like IGNORE_BAD_KEYS, and pass data about invalid keys back in results JSON.

@ni4 ni4 self-assigned this May 20, 2020
@ni4 ni4 added this to the v0.14.0 milestone May 20, 2020
@kaie
Copy link
Contributor Author

kaie commented May 20, 2020

yes that would work

@kaie
Copy link
Contributor Author

kaie commented May 20, 2020

I'd like to suggest an additional flag.

A key might fail to import, because it contains a signature that RNP is rejecting.

So another option for import could be: If invalid signature is found on key, try to import the stripped/minimized key.

@ni4
Copy link
Contributor

ni4 commented May 23, 2020

@kaie JFYI: While this is not 100% complete yet, since just merged PR #1149 RNP should be much more relaxed in signature consumption, and rnp --dump-packets more informative for easier debugging.

@ni4
Copy link
Contributor

ni4 commented Jun 3, 2020

@kaie As of now there is an additional RNP_LOAD_SAVE_PERMISSIVE flag for rnp_import_keys(), which should skip bad sigs/keys and dump some more detailed debug output.

@kaie
Copy link
Contributor Author

kaie commented Jun 4, 2020

Thanks. But if we import multiple keys at once, there's no output that says which key is failing. That makes it very difficult to diagnose, because you have no idea to which key an message refers to. I suggest to log the failing key ID to the console. I have a patch.

@kaie
Copy link
Contributor Author

kaie commented Jun 4, 2020

Because this issue is closed, I've filed new #1162

@kaie
Copy link
Contributor Author

kaie commented Jun 8, 2020

I'm trying to see the effect of RNP_LOAD_SAVE_PERMISSIVE.

Do you have an example key, which gets imported with RNP_LOAD_SAVE_PERMISSIVE, but which gets rejected without that flag? I couldn't find any.

@ni4
Copy link
Contributor

ni4 commented Jun 9, 2020

@kaie Together with RNP_LOAD_SAVE_PERMISSIVE, I fixed other loading issues, that's probably the reason. However there are manually modified keys with different errors in sig/key data, please see the test_ffi_malformed_keys_import() in ffi.cpp

@kaie
Copy link
Contributor Author

kaie commented Jun 9, 2020

Thanks @ni4 that is helpful and allowed me to test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants