rnpgp / rnp

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

Function `rnp_ffi_create()` does not accept `RNP_KEYSTORE_GPG21` format #1894

Open antonsviridenko opened 1 year ago

antonsviridenko commented 1 year ago

Description

Parameter description for rnp_ffi_create() says:

 *  @param pub_format the format of the public keyring, RNP_KEYSTORE_GPG or other
 *         RNP_KEYSTORE_* constant
 *  @param sec_format the format of the secret keyring, RNP_KEYSTORE_GPG or other
 *         RNP_KEYSTORE_* constant

so it is reasonable to assume that

/* Keystore format: GPG, KBX (pub), G10 (sec), GPG21 ( KBX for pub, G10 for sec) */
...
#define RNP_KEYSTORE_GPG21 ("GPG21")

should be accepted too. But CLI and test code replaces literal value "GPG21" passed as a keystore format to KBX for public and G10 for private instead, in these two places: https://github.com/rnpgp/rnp/blob/master/src/rnp/fficli.cpp#L1928 https://github.com/rnpgp/rnp/blob/master/src/tests/support.cpp#L537 So this way CLI can accept "GPG21" keystore format, but rnp_ffi_create() is never called or tested with RNP_KEYSTORE_GPG21 in current codebase.

Steps to Reproduce

Call rnp_ffi_create() with pub_fomat and sec_format set to RNP_KEYSTORE_GPG21.

Expected Behavior

Either function should accept this value, or documentation should be updated.

Actual Behavior

RNP_ERROR_BAD_PARAMETERS is returned. Helper function parse_ks_format recognizes only 3 keystore formats from 4 defined.

ni4 commented 1 year ago

@antonsviridenko Could you please clarify why this may be needed? I do not remember when this was added, maybe there were some misunderstanding or so on. There is no GPG21 key store format: GnuPG has 2 ways of storing keys. Old format via plain pubring.gpg/secring.gpg (i.e. _GPG/_GPG) and a new one, with pubring.kbx/private-keys-v1.d (i.e. _KBX/_G10)

P.S. So, as for me, more correct solution would be to remove _GPG21 define from the codebase :)

ni4 commented 1 year ago

Related: https://github.com/rnpgp/rnp/issues/1724

antonsviridenko commented 1 year ago

P.S. So, as for me, more correct solution would be to remove _GPG21 define from the codebase :)

I agree