From 9b5b1612208659707231a19020e9f2023e45e127 Mon Sep 17 00:00:00 2001 From: Krista Bennett Date: Sun, 4 Feb 2018 03:36:08 +0100 Subject: [PATCH] A whole horde of trust fixes (mistrust list added, tests fixed, and sneaky null string in elect key removed) --- src/keymanagement.c | 126 +++++++++++++++++++++++--- src/keymanagement.h | 5 + src/pEpEngine.c | 52 ++++++++--- src/pEp_internal.h | 5 + test/mistrust_undo_test.cc | 6 +- test/new_update_id_and_myself_test.cc | 2 +- 6 files changed, 171 insertions(+), 25 deletions(-) diff --git a/src/keymanagement.c b/src/keymanagement.c index 99cd00e2..6cc12b5d 100644 --- a/src/keymanagement.c +++ b/src/keymanagement.c @@ -77,8 +77,11 @@ PEP_STATUS elect_pubkey( _comm_type_key > identity->comm_type) { bool blacklisted; - status = blacklist_is_listed(session, _keylist->value, &blacklisted); - if (status == PEP_STATUS_OK && !blacklisted) { + bool mistrusted; + status = is_mistrusted_key(session, _keylist->value, &mistrusted); + if (status == PEP_STATUS_OK) + status = blacklist_is_listed(session, _keylist->value, &blacklisted); + if (status == PEP_STATUS_OK && !mistrusted && !blacklisted) { identity->comm_type = _comm_type_key; _fpr = _keylist->value; } @@ -88,10 +91,14 @@ PEP_STATUS elect_pubkey( } free(identity->fpr); - identity->fpr = strdup(_fpr); - if (identity->fpr == NULL) { - free_stringlist(keylist); - return PEP_OUT_OF_MEMORY; + if (!_fpr || _fpr[0] == '\0') + identity->fpr = NULL; + else { + identity->fpr = strdup(_fpr); + if (identity->fpr == NULL) { + free_stringlist(keylist); + return PEP_OUT_OF_MEMORY; + } } free_stringlist(keylist); @@ -370,14 +377,15 @@ static PEP_STATUS prepare_updated_identity(PEP_SESSION session, stored_ident->comm_type = ct; } } + else { + if (stored_ident->comm_type == PEP_ct_unknown) + 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); - // This is outside the if block ON PURPOSE - we return an empty FPR + - // the reason why a key wasn't used in the comm_type string if we can't - // find or use one. return_id->comm_type = stored_ident->comm_type; // We patch the DB with the input username, but if we didn't have @@ -1065,6 +1073,8 @@ DYNAMIC_API PEP_STATUS key_mistrusted( status = mark_as_compromized(session, ident->fpr); if (status == PEP_STATUS_OK) status = remove_fpr_as_default(session, ident->fpr); + if (status == PEP_STATUS_OK) + status = add_mistrusted_key(session, ident->fpr); } return status; @@ -1083,12 +1093,15 @@ DYNAMIC_API PEP_STATUS undo_last_mistrust(PEP_SESSION session) { if (!cached_ident) status = PEP_CANNOT_FIND_IDENTITY; else { - status = set_identity(session, cached_ident); - free_identity(session->cached_mistrusted); + status = delete_mistrusted_key(session, cached_ident->fpr); + if (status == PEP_STATUS_OK) { + status = set_identity(session, cached_ident); + free_identity(session->cached_mistrusted); + } } session->cached_mistrusted = NULL; - + return status; } @@ -1129,7 +1142,19 @@ DYNAMIC_API PEP_STATUS key_reset_trust( if (status != PEP_STATUS_OK) goto pep_free; + bool mistrusted_key = false; + status = is_mistrusted_key(session, ident->fpr, &mistrusted_key); + + if (status != PEP_STATUS_OK) + goto pep_free; + + if (mistrusted_key) + status = delete_mistrusted_key(session, ident->fpr); + + if (status != PEP_STATUS_OK) + goto pep_free; + input_copy->comm_type = new_trust; tmp_ident = new_identity(ident->address, NULL, ident->user_id, NULL); @@ -1569,3 +1594,80 @@ PEP_STATUS contains_priv_key(PEP_SESSION session, const char *fpr, return session->cryptotech[PEP_crypt_OpenPGP].contains_priv_key(session, fpr, has_private); } + +PEP_STATUS add_mistrusted_key(PEP_SESSION session, const char* fpr) +{ + int result; + + assert(!EMPTYSTR(fpr)); + + if (!(session) || EMPTYSTR(fpr)) + return PEP_ILLEGAL_VALUE; + + sqlite3_reset(session->add_mistrusted_key); + sqlite3_bind_text(session->add_mistrusted_key, 1, fpr, -1, + SQLITE_STATIC); + + result = sqlite3_step(session->add_mistrusted_key); + sqlite3_reset(session->add_mistrusted_key); + + if (result != SQLITE_DONE) + return PEP_CANNOT_SET_PGP_KEYPAIR; // FIXME: Better status? + + return PEP_STATUS_OK; +} + +PEP_STATUS delete_mistrusted_key(PEP_SESSION session, const char* fpr) +{ + int result; + + assert(!EMPTYSTR(fpr)); + + if (!(session) || EMPTYSTR(fpr)) + return PEP_ILLEGAL_VALUE; + + sqlite3_reset(session->delete_mistrusted_key); + sqlite3_bind_text(session->delete_mistrusted_key, 1, fpr, -1, + SQLITE_STATIC); + + result = sqlite3_step(session->delete_mistrusted_key); + sqlite3_reset(session->delete_mistrusted_key); + + if (result != SQLITE_DONE) + return PEP_UNKNOWN_ERROR; // FIXME: Better status? + + return PEP_STATUS_OK; +} + +PEP_STATUS is_mistrusted_key(PEP_SESSION session, const char* fpr, + bool* mistrusted) +{ + PEP_STATUS status = PEP_STATUS_OK; + + assert(session); + assert(!EMPTYSTR(fpr)); + + if (!(session && fpr)) + return PEP_ILLEGAL_VALUE; + + *mistrusted = false; + + sqlite3_reset(session->is_mistrusted_key); + sqlite3_bind_text(session->is_mistrusted_key, 1, fpr, -1, SQLITE_STATIC); + + int result; + + result = sqlite3_step(session->is_mistrusted_key); + switch (result) { + case SQLITE_ROW: + *mistrusted = sqlite3_column_int(session->is_mistrusted_key, 0); + status = PEP_STATUS_OK; + break; + + default: + status = PEP_UNKNOWN_ERROR; + } + + sqlite3_reset(session->is_mistrusted_key); + return status; +} diff --git a/src/keymanagement.h b/src/keymanagement.h index 8ed21d28..3ec19cc7 100644 --- a/src/keymanagement.h +++ b/src/keymanagement.h @@ -344,6 +344,11 @@ DYNAMIC_API PEP_STATUS set_own_key( PEP_STATUS _myself(PEP_SESSION session, pEp_identity * identity, bool do_keygen, bool ignore_flags); +PEP_STATUS add_mistrusted_key(PEP_SESSION session, const char* fpr); +PEP_STATUS delete_mistrusted_key(PEP_SESSION session, const char* fpr); +PEP_STATUS is_mistrusted_key(PEP_SESSION session, const char* fpr, bool* mistrusted); + + #ifdef __cplusplus } #endif diff --git a/src/pEpEngine.c b/src/pEpEngine.c index 10685858..951abb97 100644 --- a/src/pEpEngine.c +++ b/src/pEpEngine.c @@ -294,6 +294,17 @@ static const char *sql_get_userid_alias_default = "select default_id from alternate_user_id " " where alternate_id = ?1 ; "; +// Revocation tracking +static const char *sql_add_mistrusted_key = + "insert or replace into mistrusted_keys (fpr) " + " values (upper(replace(?1,' ',''))) ;"; + +static const char *sql_delete_mistrusted_key = + "delete from blacklist_keys where fpr = upper(replace(?1,' ','')) ;"; + +static const char *sql_is_mistrusted_key = + "select count(*) from mistrusted_keys where fpr = upper(replace(?1,' ','')) ;"; + static const char *sql_add_userid_alias = "insert or replace into alternate_user_id (default_id, alternate_id) " "values (?1, ?2) ;"; @@ -781,17 +792,17 @@ DYNAMIC_API PEP_STATUS init(PEP_SESSION *session) ); assert(int_result == SQLITE_OK); } - } - if (version < 7) { - int_result = sqlite3_exec( - _session->db, - "create table if not exists mistrusted_keys (\n" - " fpr text primary key\n" - ");\n" - NULL, - NULL, - NULL - ); + if (version < 7) { + int_result = sqlite3_exec( + _session->db, + "create table if not exists mistrusted_keys (\n" + " fpr text primary key\n" + ");\n", + NULL, + NULL, + NULL + ); + } } else { // Version from DB was 0, it means this is initial setup. @@ -1015,6 +1026,18 @@ DYNAMIC_API PEP_STATUS init(PEP_SESSION *session) (int)strlen(sql_get_revoked), &_session->get_revoked, NULL); assert(int_result == SQLITE_OK); + int_result = sqlite3_prepare_v2(_session->db, sql_add_mistrusted_key, + (int)strlen(sql_add_mistrusted_key), &_session->add_mistrusted_key, NULL); + assert(int_result == SQLITE_OK); + + int_result = sqlite3_prepare_v2(_session->db, sql_delete_mistrusted_key, + (int)strlen(sql_delete_mistrusted_key), &_session->delete_mistrusted_key, NULL); + assert(int_result == SQLITE_OK); + + int_result = sqlite3_prepare_v2(_session->db, sql_is_mistrusted_key, + (int)strlen(sql_is_mistrusted_key), &_session->is_mistrusted_key, NULL); + assert(int_result == SQLITE_OK); + status = init_cryptotech(_session, in_first); if (status != PEP_STATUS_OK) goto pep_error; @@ -1174,6 +1197,13 @@ DYNAMIC_API void release(PEP_SESSION session) if (session->get_revoked) sqlite3_finalize(session->get_revoked); + if (session->add_mistrusted_key) + sqlite3_finalize(session->add_mistrusted_key); + if (session->delete_mistrusted_key) + sqlite3_finalize(session->delete_mistrusted_key); + if (session->is_mistrusted_key) + sqlite3_finalize(session->is_mistrusted_key); + if (session->db) sqlite3_close_v2(session->db); if (session->system_db) diff --git a/src/pEp_internal.h b/src/pEp_internal.h index e234f141..65a32740 100644 --- a/src/pEp_internal.h +++ b/src/pEp_internal.h @@ -173,6 +173,11 @@ struct _pEpSession { sqlite3_stmt *set_revoked; sqlite3_stmt *get_revoked; + // mistrusted + sqlite3_stmt* add_mistrusted_key; + sqlite3_stmt* is_mistrusted_key; + sqlite3_stmt* delete_mistrusted_key; + // aliases sqlite3_stmt *get_userid_alias_default; sqlite3_stmt *add_userid_alias; diff --git a/test/mistrust_undo_test.cc b/test/mistrust_undo_test.cc index 0a4c1d02..0ef50a77 100644 --- a/test/mistrust_undo_test.cc +++ b/test/mistrust_undo_test.cc @@ -58,8 +58,12 @@ int main() { assert(status == PEP_STATUS_OK); status = update_identity(session,recip1); assert(status == PEP_STATUS_OK); + assert(recip1->comm_type == PEP_ct_key_not_found); + recip1->fpr = strdup("BACC7A60A88A39A25D99B4A545D7542F39E5DAB5"); + status = get_trust(session, recip1); assert(recip1->comm_type == PEP_ct_mistrusted); - cout << "Mistrusted mistrust.undo.test@pep-project.org (BACC7A60A88A39A25D99B4A545D7542F39E5DAB5) and comm_type set to PEP_ct_mistrusted)." << endl << endl; + + cout << "Mistrusted mistrust.undo.test@pep-project.org (BACC7A60A88A39A25D99B4A545D7542F39E5DAB5) and comm_type IN DB set to PEP_ct_mistrusted)." << endl << endl; cout << "Undo mistrust (restore identity and trust in DB)" << endl; // Undo it diff --git a/test/new_update_id_and_myself_test.cc b/test/new_update_id_and_myself_test.cc index 5dc2f8fe..f1d78cd0 100644 --- a/test/new_update_id_and_myself_test.cc +++ b/test/new_update_id_and_myself_test.cc @@ -601,7 +601,7 @@ int main() { assert(!revokemaster_3000->fpr); assert(revokemaster_3000->username); assert(strcmp(revokemaster_3000->user_id, revoke_uuid) == 0); - assert(revokemaster_3000->comm_type == PEP_ct_key_revoked || revokemaster_3000->comm_type == PEP_ct_mistrusted); + assert(revokemaster_3000->comm_type == PEP_ct_key_not_found); cout << "Success! No key found. The comm_status error was " << revokemaster_3000->comm_type << "and the return status was " << tl_status_string(status) << endl; free_identity(revokemaster_3000);