rnp_key_is_compromised/retired/superseded return RNP_ERROR_BAD_PARAMETERS for keys that are not revoked

[ronaldtse]

rnp_key_is_compromised() / rnp_key_is_retired() / rnp_key_is_superseded() return RNP_ERROR_BAD_PARAMETERS when the key is not revoked

Description

All three predicates go through rnp_key_is_revoked_with_code() (src/lib/rnp.cpp, around line 7659 on main):

static rnp_result_t
rnp_key_is_revoked_with_code(rnp_key_handle_t handle, bool *result, int code)
{
    if (!handle || !result) {
        return RNP_ERROR_NULL_POINTER;
    }
    auto *key = get_key_prefer_public(handle);
    if (!key || !key->revoked()) {
        return RNP_ERROR_BAD_PARAMETERS;
    }
    *result = key->revocation().code == code;
    return RNP_SUCCESS;
}

"Not revoked" is a perfectly normal key state, but it is answered with an error. Callers of e.g. rnp_key_is_compromised() have no way to distinguish "the key is fine, just not revoked as compromised" from an actual problem — every caller must special-case the error code and map it to false by hand.

The sibling predicates rnp_key_is_revoked(), rnp_key_is_valid(), rnp_key_is_expired(), ... all succeed with a boolean for the same key state, so this trio is also inconsistent within the API.

Suggested behavior

Return RNP_SUCCESS with *result = false when the key exists but is not revoked, reserving RNP_ERROR_BAD_PARAMETERS for an unusable handle (and RNP_ERROR_KEY_NOT_FOUND for the missing-key case if that needs to stay distinct).

Environment

Observed against librnp 0.18.1 and re-verified in current main (src/lib/rnp.cpp, rnp_key_is_revoked_with_code), while developing the Rust binding (rnpgp/rnp-rs).