pEp.foundation
/
pEpEngine
16
2
Fork 17
Code Issues 22 Pull Requests 2 Releases 48 Wiki Activity

ENGINE-633: Fixed error with update_identity - now, if a default key is unsuitable and there is no other available key in the trust db (meaning it's been assigned as a default at some time) for this identity, we return no key, PEP_ct_key_not_found, and PEP_STATUS_OK, *not* PEP_KEY_UNSUITABLE, since there is no key returned. key-related functions will still return that status.

Browse Source
doxygen_doc
Krista Bennett 2 years ago
parent de876f5fad
commit 2ce7baab83
6 changed files with 66 additions and 40 deletions
Show Stats Download Patch File Download Diff File

30
src/keymanagement.c
Unescape View File

@ -640,22 +640,27 @@ static PEP_STATUS prepare_updated_identity(PEP_SESSION session,
bool is_pEp = false;
switch (status) {
// FIXME: can we get memory or DB errors from the above? If so, handle it.
case PEP_STATUS_OK:
if (!EMPTYSTR(stored_ident->fpr)) {
// set identity comm_type from trust db (user_id, FPR)
status = get_trust(session, stored_ident);
if (status == PEP_CANNOT_FIND_IDENTITY || stored_ident->comm_type == PEP_ct_unknown) {
PEP_comm_type ct = stored_ident->comm_type;
if (status == PEP_CANNOT_FIND_IDENTITY || ct == PEP_ct_unknown || ct == PEP_ct_key_not_found) {
// This is OK - there is no trust DB entry, but we
// found a key. We won't store this, but we'll
// use it.
PEP_comm_type ct = PEP_ct_unknown;
ct = PEP_ct_unknown;
status = get_key_rating(session, stored_ident->fpr, &ct);
stored_ident->comm_type = ct;
stored_ident->comm_type = (ct == PEP_ct_unknown ? PEP_ct_key_not_found : ct);
}
}
else if (stored_ident->comm_type == PEP_ct_unknown)
stored_ident->comm_type = PEP_ct_key_not_found;
break;
break;
case PEP_KEY_UNSUITABLE:
status = PEP_STATUS_OK;
// explicit fallthrough
default:
is_pEp_user(session, stored_ident, &is_pEp);
if (is_pEp) {
@ -671,14 +676,14 @@ static PEP_STATUS prepare_updated_identity(PEP_SESSION session,
free(stored_ident->fpr);
stored_ident->fpr = NULL;
stored_ident->comm_type = PEP_ct_key_not_found;
stored_ident->comm_type = PEP_ct_key_not_found;
}
free(return_id->fpr);
return_id->fpr = NULL;
if (status == PEP_STATUS_OK && !EMPTYSTR(stored_ident->fpr))
return_id->fpr = strdup(stored_ident->fpr);
return_id->comm_type = stored_ident->comm_type;
// We patch the DB with the input username, but if we didn't have
@ -1011,6 +1016,9 @@ DYNAMIC_API PEP_STATUS update_identity(
identity->comm_type = PEP_ct_unknown;
adjust_pEp_trust_status(session, identity);
status = set_identity(session, identity);
// This is ONLY for the return value - VB confirms we should tell the user we didn't find a key
if (identity->comm_type == PEP_ct_unknown)
identity->comm_type = PEP_ct_key_not_found;
}
// VB says, and I quote, "that is not implemented and no one is using it right now"
@ -2264,7 +2272,7 @@ PEP_STATUS is_mistrusted_key(PEP_SESSION session, const char* fpr,
/**
* @internal
*
* <!-- _wipe_default_key_if_invalid() -->
* <!-- _wipe_own_default_key_if_invalid() -->
*
* @brief TODO
*
@ -2276,7 +2284,7 @@ PEP_STATUS is_mistrusted_key(PEP_SESSION session, const char* fpr,
* @retval PEP_OUT_OF_MEMORY out of memory
* @retval any other value on error
*/
static PEP_STATUS _wipe_default_key_if_invalid(PEP_SESSION session,
static PEP_STATUS _wipe_own_default_key_if_invalid(PEP_SESSION session,
pEp_identity* ident) {
if (!(session && ident))
@ -2319,6 +2327,8 @@ static PEP_STATUS _wipe_default_key_if_invalid(PEP_SESSION session,
// This may have been for a user default, not an identity default.
if (status == PEP_STATUS_OK && !(EMPTYSTR(ident->address)))
status = myself(session, ident);
else
status = PEP_STATUS_OK; // Once we've wiped it, since password errors are already handled, we're fine here.
return status;
}
@ -2348,7 +2358,7 @@ DYNAMIC_API PEP_STATUS clean_own_key_defaults(PEP_SESSION session) {
if (!ident)
continue;
status = _wipe_default_key_if_invalid(session, ident);
status = _wipe_own_default_key_if_invalid(session, ident);
if (PASS_ERROR(status))
return status;
}
@ -2375,7 +2385,7 @@ DYNAMIC_API PEP_STATUS clean_own_key_defaults(PEP_SESSION session) {
}
else if (user_default_key) {
pEp_identity* empty_user = new_identity(NULL, user_default_key, own_id, NULL);
status = _wipe_default_key_if_invalid(session, empty_user);
status = _wipe_own_default_key_if_invalid(session, empty_user);
if (PASS_ERROR(status))
return status;

4
src/keymanagement.h
Unescape View File

@ -30,10 +30,6 @@ extern "C" {
* @retval PEP_ILLEGAL_VALUE if called with illegal inputs, including an identity
* with .me set or with an own user_id specified in the
* *input* (see caveats)
* @retval PEP_KEY_UNSUITABLE if a default key was found for this identity, no
* other acceptable keys were found; if this is returned,
* the reason for rejecting the first default key found
* may be found in the comm_type
* @retval any other value on error
*
* @warning at least identity->address must be a non-empty UTF-8 string as input

14
test/src/CheckRenewedExpiredKeyTrustStatusTest.cc
Unescape View File

@ -175,8 +175,9 @@ TEST_F(CheckRenewedExpiredKeyTrustStatusTest, check_renewed_expired_key_trust_st
const string msg = slurp("test_mails/ENGINE-463-attempt-numero-dos.eml");
status = update_identity(session, expired_inquisitor);
ASSERT_EQ(status, PEP_KEY_UNSUITABLE);
ASSERT_EQ(status, PEP_STATUS_OK); // We don't need to be informed that there's now no key - update_identity not having one is enough
ASSERT_NULL(expired_inquisitor->fpr);
ASSERT_EQ(expired_inquisitor->comm_type, PEP_ct_key_not_found);
char* decrypted_msg = NULL;
stringlist_t* keylist = NULL;
@ -207,6 +208,12 @@ TEST_F(CheckRenewedExpiredKeyTrustStatusTest, check_renewed_expired_key_trust_st
status = outgoing_message_rating(session, msg2, &rating);
ASSERT_OK;
ASSERT_EQ(rating, PEP_rating_reliable);
status = update_identity(session, expired_inquisitor);
ASSERT_EQ(status, PEP_STATUS_OK); // We don't need to be informed that there's now no key - update_identity not having one is enough
ASSERT_NE(expired_inquisitor->fpr, nullptr);
ASSERT_STREQ(expired_inquisitor->fpr, inq_fpr);
ASSERT_EQ(expired_inquisitor->comm_type, PEP_ct_OpenPGP_unconfirmed);
}
// If we updated an OpenPGP identity in the meantime, we will have removed the key. Too bad.
@ -247,7 +254,7 @@ TEST_F(CheckRenewedExpiredKeyTrustStatusTest, check_renewed_expired_key_trust_st
// Ok, now update_identity - we'll discover it's expired
status = update_identity(session, expired_inquisitor);
ASSERT_EQ(status , PEP_KEY_UNSUITABLE);
ASSERT_EQ(status , PEP_STATUS_OK); // Key is expired, but we don't return it. So... byebye.
PEP_comm_type ct = expired_inquisitor->comm_type;
ASSERT_EQ(ct , PEP_ct_key_not_found);
ASSERT_NULL(expired_inquisitor->fpr);
@ -303,6 +310,7 @@ TEST_F(CheckRenewedExpiredKeyTrustStatusTest, check_renewed_expired_key_trust_st
ASSERT_EQ(rating, PEP_rating_trusted);
free_message(msg2);
}
TEST_F(CheckRenewedExpiredKeyTrustStatusTest, check_renewed_expired_key_trust_status_pEp_user) {
@ -403,7 +411,7 @@ TEST_F(CheckRenewedExpiredKeyTrustStatusTest, check_renewed_expired_key_trust_st
// Ok, now update_identity - we'll discover it's expired
status = update_identity(session, expired_inquisitor);
ASSERT_EQ(status , PEP_KEY_UNSUITABLE);
ASSERT_OK; // key is expired, but we don't return it, so we're OK.
PEP_comm_type ct = expired_inquisitor->comm_type;
ASSERT_EQ(ct, PEP_ct_key_not_found);
ASSERT_NULL(expired_inquisitor->fpr);

28
test/src/ExpiredSubkeyTest.cc
Unescape View File

@ -108,11 +108,17 @@ TEST_F(ExpiredSubkeyTest, check_expired_subkey_with_valid_subkeys_expired_main)
ASSERT_OK;
status = update_identity(session, expired_0);
ASSERT_OK;
ASSERT_NOTNULL(expired_0->fpr);
PEP_rating rating;
status = identity_rating(session, expired_0, &rating);
ASSERT_EQ(status , PEP_KEY_UNSUITABLE);
ASSERT_EQ(rating , PEP_rating_undefined);
ASSERT_OK;
ASSERT_EQ(rating , PEP_rating_have_no_key);
PEP_comm_type ct = PEP_ct_unknown;
status = get_key_rating(session, fpr, &ct);
ASSERT_OK;
ASSERT_EQ(ct, PEP_ct_key_expired);
status = get_key_rating_for_user(session, expired_0->user_id, fpr, &rating);
ASSERT_OK;
ASSERT_EQ(rating, PEP_rating_unreliable);
}
TEST_F(ExpiredSubkeyTest, check_all_valid_with_leftover_expired_subkeys) {
@ -140,9 +146,19 @@ TEST_F(ExpiredSubkeyTest, check_no_valid_encryption_subkey) {
ASSERT_OK;
status = update_identity(session, expired_0);
ASSERT_OK;
ASSERT_NOTNULL(expired_0->fpr);
status = update_identity(session, expired_0);
ASSERT_OK;
ASSERT_NULL(expired_0->fpr);
PEP_rating rating;
status = identity_rating(session, expired_0, &rating);
ASSERT_EQ(status , PEP_KEY_UNSUITABLE);
ASSERT_EQ(rating , PEP_rating_undefined);
ASSERT_OK;
ASSERT_EQ(rating , PEP_rating_have_no_key);
PEP_comm_type ct = PEP_ct_unknown;
status = get_key_rating(session, fpr, &ct);
ASSERT_OK;
ASSERT_EQ(ct, PEP_ct_key_expired);
status = get_key_rating_for_user(session, expired_0->user_id, fpr, &rating);
ASSERT_OK;
ASSERT_EQ(rating, PEP_rating_unreliable);
}

20
test/src/RemoveKeyElectionTest.cc
Unescape View File

@ -92,7 +92,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_unstored_not_foun
PEP_STATUS status = update_identity(session, alice);
ASSERT_OK;
ASSERT_NULL(alice->fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_key_not_found); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_key_not_found); // This is desired. The DB, however, must have "unknown"
}
TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_not_found) {
@ -104,7 +104,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_not_found)
status = update_identity(session, alice);
ASSERT_OK;
ASSERT_NULL(alice->fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_key_not_found); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_key_not_found); // This is desired. The DB, however, must have "unknown"
}
TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found) {
@ -120,7 +120,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found) {
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
}
TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_no_input_username) {
@ -138,7 +138,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_no_i
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
ASSERT_STREQ(alice_name, alice->username);
}
@ -166,7 +166,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_stor
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
ASSERT_STREQ(alice_name, alice->username);
free_identity(alice2);
alice2 = NULL;
@ -190,7 +190,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_no_u
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
}
TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_no_userid_no_uname) {
@ -210,7 +210,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_no_u
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
}
TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_stored_TOFU_input_TOFU_names_match) {
@ -226,7 +226,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_stor
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
}
TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_stored_TOFU_input_TOFU_names_no_match) {
@ -244,7 +244,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_stor
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
}
TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_stored_TOFU_input_no_userid_names_no_match) {
@ -264,7 +264,7 @@ TEST_F(RemoveKeyElectionTest, check_remove_key_election_simple_stored_found_stor
ASSERT_OK;
ASSERT_NOTNULL(alice->fpr);
ASSERT_STREQ(alice->fpr, alice_fpr);
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed); // This might be undesired. Maybe "unknown"
ASSERT_EQ(alice->comm_type, PEP_ct_OpenPGP_unconfirmed);
ASSERT_STREQ(alice->username, "Cheese");
pEp_identity* alice2 = NULL;
status = get_identity(session, alice_addr, alice_TOFU, &alice2);

10
test/src/RevocationTest.cc
Unescape View File

@ -111,10 +111,7 @@ TEST_F(RevocationTest, check_revocation) {
const char* linda_fpr = "ABC96B3B4BAFB57DC45D81B56A48221A903A158B";
pEp_identity* pre = new_identity("linda@example.org", NULL, NULL, NULL);
status = update_identity(session, pre);
ASSERT_OK;
pre->fpr = strdup(linda_fpr);
status = set_identity(session, pre);
status = set_fpr_preserve_ident(session, pre, linda_fpr, false);
ASSERT_OK;
status = update_identity(session, pre);
ASSERT_OK;
@ -136,10 +133,9 @@ TEST_F(RevocationTest, check_revocation) {
ASSERT_OK;
status = update_identity(session, post);
// PEP_KEY_UNSUITABLE => revoked (or something similar).
ASSERT_EQ(status , PEP_KEY_UNSUITABLE);
ASSERT_EQ(status , PEP_STATUS_OK); // We don't return revoked keys on update_identity, we just return nothing, and this is OK.
ASSERT_EQ(post->comm_type , PEP_ct_key_not_found);
free(post->fpr);
ASSERT_NULL(post->fpr);
post->fpr = strdup(keylist->value);
status = get_trust(session, post);
ASSERT_OK;

Write Preview
Loading…
Cancel
Save