Fixed hole in update_identity spec for extant identities without saved fprs. ENGINE-581
authorKrista 'DarthMama' Bennett <krista@pep.foundation>
Thu, 27 Jun 2019 15:12:42 +0200
branchENGINE-581
changeset 3886ebd61a1cf6db
parent 3885 843f7c599a97
child 3887 a3f479737dc4
Fixed hole in update_identity spec for extant identities without saved fprs.
src/keymanagement.c
src/pEpEngine.c
test/src/engine_tests/CheckRenewedExpiredKeyTrustStatusTests.cc
test/src/engine_tests/ExternalRevokeTests.cc
test/src/engine_tests/NewUpdateIdAndMyselfTests.cc
test/src/engine_tests/RevocationTests.cc
     1.1 --- a/src/keymanagement.c	Thu Jun 27 13:12:07 2019 +0200
     1.2 +++ b/src/keymanagement.c	Thu Jun 27 15:12:42 2019 +0200
     1.3 @@ -470,6 +470,8 @@
     1.4      PEP_STATUS status;
     1.5      
     1.6      bool is_identity_default, is_user_default, is_address_default;
     1.7 +    bool no_stored_default = EMPTYSTR(stored_ident->fpr);
     1.8 +    
     1.9      status = get_valid_pubkey(session, stored_ident,
    1.10                                  &is_identity_default,
    1.11                                  &is_user_default,
    1.12 @@ -488,6 +490,11 @@
    1.13              stored_ident->comm_type = ct;
    1.14          }
    1.15      }
    1.16 +    else if (status != PEP_STATUS_OK) {
    1.17 +        free(stored_ident->fpr);
    1.18 +        stored_ident->fpr = NULL;
    1.19 +        stored_ident->comm_type = PEP_ct_key_not_found;        
    1.20 +    }
    1.21      else {
    1.22          if (stored_ident->comm_type == PEP_ct_unknown)
    1.23              stored_ident->comm_type = PEP_ct_key_not_found;
    1.24 @@ -536,8 +543,17 @@
    1.25           // or identity AND is valid for this address, set in DB
    1.26           // as default
    1.27           status = set_identity(session, return_id);
    1.28 +    } 
    1.29 +    else if (no_stored_default && !EMPTYSTR(return_id->fpr) 
    1.30 +             && return_id->comm_type != PEP_ct_key_revoked
    1.31 +             && return_id->comm_type != PEP_ct_key_expired
    1.32 +             && return_id->comm_type != PEP_ct_key_expired_but_confirmed
    1.33 +             && return_id->comm_type != PEP_ct_mistrusted 
    1.34 +             && return_id->comm_type != PEP_ct_key_b0rken) { 
    1.35 +        // We would have stored this anyway for a first-time elected key. We just have an ident w/ no default already.
    1.36 +        status = set_identity(session, return_id);
    1.37      }
    1.38 -    else {
    1.39 +    else { // this is a key other than the default, but there IS a default (FIXME: fdik, do we really want behaviour below?)
    1.40          // Store without default fpr/ct, but return the fpr and ct 
    1.41          // for current use
    1.42          char* save_fpr = return_id->fpr;
     2.1 --- a/src/pEpEngine.c	Thu Jun 27 13:12:07 2019 +0200
     2.2 +++ b/src/pEpEngine.c	Thu Jun 27 15:12:42 2019 +0200
     2.3 @@ -4267,8 +4267,8 @@
     2.4              identity->username))
     2.5          return PEP_ILLEGAL_VALUE;
     2.6  
     2.7 -    const char* saved_username = NULL;
     2.8 -    const char* at = NULL;
     2.9 +    char* saved_username = NULL;
    2.10 +    char* at = NULL;
    2.11      size_t uname_len = strlen(identity->username);
    2.12      
    2.13      if (uname_len > 0)
     3.1 --- a/test/src/engine_tests/CheckRenewedExpiredKeyTrustStatusTests.cc	Thu Jun 27 13:12:07 2019 +0200
     3.2 +++ b/test/src/engine_tests/CheckRenewedExpiredKeyTrustStatusTests.cc	Thu Jun 27 15:12:42 2019 +0200
     3.3 @@ -108,6 +108,13 @@
     3.4      status = update_identity(session, expired_inquisitor);
     3.5      TEST_ASSERT_MSG((status == PEP_KEY_UNSUITABLE), tl_status_string(status));
     3.6      PEP_comm_type ct = expired_inquisitor->comm_type;    
     3.7 +    TEST_ASSERT_MSG(ct == PEP_ct_key_not_found, tl_ct_string(ct));
     3.8 +    TEST_ASSERT(!expired_inquisitor->fpr);
     3.9 +    
    3.10 +    expired_inquisitor->fpr = strdup(inquisitor_fpr);
    3.11 +    status = get_trust(session, expired_inquisitor);
    3.12 +    ct = expired_inquisitor->comm_type;    
    3.13 +    TEST_ASSERT(status == PEP_STATUS_OK);
    3.14      TEST_ASSERT_MSG(ct == PEP_ct_key_expired_but_confirmed, tl_ct_string(ct));
    3.15      
    3.16      // Ok, so I want to make sure we make an entry, so I'll try to decrypt the message WITH
    3.17 @@ -250,6 +257,13 @@
    3.18      status = update_identity(session, expired_inquisitor);
    3.19      TEST_ASSERT_MSG((status == PEP_KEY_UNSUITABLE), tl_status_string(status));
    3.20      PEP_comm_type ct = expired_inquisitor->comm_type;    
    3.21 +    TEST_ASSERT_MSG(ct == PEP_ct_key_not_found, tl_ct_string(ct));
    3.22 +    TEST_ASSERT(!expired_inquisitor->fpr);
    3.23 +    
    3.24 +    expired_inquisitor->fpr = strdup(inquisitor_fpr);
    3.25 +    status = get_trust(session, expired_inquisitor);
    3.26 +    ct = expired_inquisitor->comm_type;    
    3.27 +    TEST_ASSERT(status == PEP_STATUS_OK);
    3.28      TEST_ASSERT_MSG(ct == PEP_ct_key_expired_but_confirmed, tl_ct_string(ct));
    3.29      
    3.30      // Ok, so I want to make sure we make an entry, so I'll try to decrypt the message WITH
     4.1 --- a/test/src/engine_tests/ExternalRevokeTests.cc	Thu Jun 27 13:12:07 2019 +0200
     4.2 +++ b/test/src/engine_tests/ExternalRevokeTests.cc	Thu Jun 27 15:12:42 2019 +0200
     4.3 @@ -255,8 +255,9 @@
     4.4      
     4.5      status = get_trust(session, recip1);
     4.6  
     4.7 -    cout << "Recip's trust DB comm_type (should be unknown, as we're using a keyring-only key, not in DB) = "  << tl_ct_string(recip1->comm_type) << endl;
     4.8 -    TEST_ASSERT_MSG((recip1->comm_type != PEP_ct_OpenPGP_unconfirmed), "recip1->comm_type != PEP_ct_OpenPGP_unconfirmed");
     4.9 +//    cout << "Recip's trust DB comm_type (should be unknown, as we're using a keyring-only key, not in DB) = "  << tl_ct_string(recip1->comm_type) << endl;
    4.10 +    cout << "Recip's trust DB comm_type (should PEP_ct_OpenPGP_unconfirmed), as we now record this when using update_identity on no-default idents = "  << tl_ct_string(recip1->comm_type) << endl;
    4.11 +    TEST_ASSERT_MSG((recip1->comm_type == PEP_ct_OpenPGP_unconfirmed), tl_ct_string(recip1->comm_type));
    4.12  
    4.13      // decrypt message
    4.14  //    free_message(outgoing_msg);
    4.15 @@ -285,8 +286,9 @@
    4.16      
    4.17      status = get_trust(session, recip1);
    4.18      
    4.19 -    cout << "Recip's trust DB comm_type (should be unknown - there's nothing in the DB) = "  << tl_ct_string(recip1->comm_type) << endl;
    4.20 -    TEST_ASSERT_MSG((recip1->comm_type == PEP_ct_unknown), "recip1->comm_type == PEP_ct_unknown");
    4.21 +//    cout << "Recip's trust DB comm_type (should be unknown - there's nothing in the DB) = "  << tl_ct_string(recip1->comm_type) << endl;
    4.22 +    cout << "Recip's trust DB comm_type (should be PEP_ct_OpenPGP_unconfirmed, as we now store it.) = "  << tl_ct_string(recip1->comm_type) << endl;
    4.23 +    TEST_ASSERT_MSG((recip1->comm_type == PEP_ct_OpenPGP_unconfirmed), tl_ct_string(recip1->comm_type));
    4.24  
    4.25      free_message(encrypted_outgoing_msg);
    4.26      free_message(decrypted_msg);
     5.1 --- a/test/src/engine_tests/NewUpdateIdAndMyselfTests.cc	Thu Jun 27 13:12:07 2019 +0200
     5.2 +++ b/test/src/engine_tests/NewUpdateIdAndMyselfTests.cc	Thu Jun 27 15:12:42 2019 +0200
     5.3 @@ -500,10 +500,10 @@
     5.4      TEST_ASSERT_MSG((bernd->user_id), "bernd->user_id");
     5.5      TEST_ASSERT_MSG((strcmp(bernd->user_id, bernd_userid) == 0), "strcmp(bernd->user_id, bernd_userid) == 0"); // ???
     5.6      TEST_ASSERT_MSG((!bernd->me), "!bernd->me"); 
     5.7 -    TEST_ASSERT_MSG((bernd->comm_type == PEP_ct_key_expired), "bernd->comm_type == PEP_ct_key_expired");
     5.8 +    TEST_ASSERT_MSG((bernd->comm_type == PEP_ct_key_not_found), tl_ct_string(bernd->comm_type));
     5.9      TEST_ASSERT_MSG((strcmp(bernd->address, bernd_address) == 0), "strcmp(bernd->address, bernd_address) == 0");
    5.10  
    5.11 -    cout << "PASS: update_identity() correctly rejected expired key with PEP_KEY_UNSUITABLE and PEP_ct_key_expired" << endl << endl;
    5.12 +    cout << "PASS: update_identity() correctly rejected expired key with PEP_KEY_UNSUITABLE and PEP_ct_key_not_found" << endl << endl;
    5.13      free_identity(bernd);
    5.14      
    5.15  }
     6.1 --- a/test/src/engine_tests/RevocationTests.cc	Thu Jun 27 13:12:07 2019 +0200
     6.2 +++ b/test/src/engine_tests/RevocationTests.cc	Thu Jun 27 15:12:42 2019 +0200
     6.3 @@ -52,8 +52,24 @@
     6.4      TEST_ASSERT_MSG((status == PEP_TEST_KEY_IMPORT_SUCCESS), "status == PEP_STATUS_OK");
     6.5  
     6.6      pEp_identity* post = new_identity("linda@example.org", NULL, NULL, NULL);
     6.7 +    
     6.8 +//    string save_fpr = post->fpr;
     6.9 +
    6.10 +    stringlist_t* keylist = NULL;
    6.11 +    
    6.12 +    status = find_keys(session, "linda@example.org", &keylist);
    6.13 +    TEST_ASSERT(status == PEP_STATUS_OK);
    6.14 +    
    6.15      status = update_identity(session, post);
    6.16      // PEP_KEY_UNSUITABLE => revoked (or something similar).
    6.17      TEST_ASSERT_MSG((status == PEP_KEY_UNSUITABLE), tl_status_string(status));
    6.18 +    TEST_ASSERT_MSG((post->comm_type == PEP_ct_key_not_found), tl_ct_string(post->comm_type));
    6.19 +    free(post->fpr);
    6.20 +    post->fpr = strdup(keylist->value);
    6.21 +    status = get_trust(session, post);
    6.22 +    TEST_ASSERT(status == PEP_STATUS_OK);
    6.23      TEST_ASSERT_MSG((post->comm_type == PEP_ct_key_revoked), tl_ct_string(post->comm_type));
    6.24 +    free_identity(pre);
    6.25 +    free_identity(post);
    6.26 +    free_stringlist(keylist);    
    6.27  }