SEMANTIC CHANGE in myself(): .fpr is never used; to set fpr now use set_own_key()

src/keymanagement.c

@@ -864,6 +864,7 @@ PEP_STATUS _myself(PEP_SESSION session, pEp_identity * identity, bool do_keygen,
 
     pEp_identity *stored_identity = NULL;
     char* revoked_fpr = NULL; 
+    bool valid_key_found = false;
         
     char* default_own_id = NULL;
     status = get_default_own_userid(session, &default_own_id);
@@ -905,8 +906,10 @@ PEP_STATUS _myself(PEP_SESSION session, pEp_identity * identity, bool do_keygen,
                           &stored_identity);
 
     assert(status != PEP_OUT_OF_MEMORY);
-    if (status == PEP_OUT_OF_MEMORY)
-        return PEP_OUT_OF_MEMORY;
+    if (status == PEP_OUT_OF_MEMORY) {
+        status = PEP_OUT_OF_MEMORY;
+        goto pep_free;
+    }
 
     // Set usernames - priority is input username > stored name > address
     // If there's an input username, we always patch the username with that
@@ -916,50 +919,49 @@ PEP_STATUS _myself(PEP_SESSION session, pEp_identity * identity, bool do_keygen,
         char* uname = (stored_uname ? stored_identity->username : identity->address);
         free(identity->username);
         identity->username = strdup(uname);
-        if (identity->username == NULL)
-            return PEP_OUT_OF_MEMORY;
+        if (identity->username == NULL) {
+            status = PEP_OUT_OF_MEMORY;
+            goto pep_free;
+        }
     }
 
-    bool valid_key_found = false;
-    
-    // Now deal with keys.
-    // Different from update_identity(), the input fpr here
-    // MATTERS. 
-    // If the input fpr is invalid, we return, giving the reason why.
-    if (!EMPTYSTR(identity->fpr)) {
-        status = validate_fpr(session, identity);
-    
-        if (status != PEP_STATUS_OK || 
-            identity->comm_type < PEP_ct_strong_but_unconfirmed) {
-            if (identity->comm_type != PEP_ct_key_expired)
-                goto pep_free;
-            // Otherwise, it was expired and key renewal failed
-            // and we take the stored one or do keygen. 
-        } 
-        else
-            valid_key_found = true;
-    }    
-    
-    // Ok, if there wasn't a valid input fpr, check stored identity
-    if (!valid_key_found && stored_identity && 
-        (EMPTYSTR(identity->fpr) || strcmp(stored_identity->fpr, identity->fpr) != 0)) {
-        
+    // ignore input fpr
+
+    if (identity->fpr) {
+        free(identity->fpr);
+        identity->fpr = NULL;
+    }
+
+    // check stored identity
+    if (stored_identity && !EMPTYSTR(stored_identity->fpr)) {
         // Fall back / retrieve
         status = validate_fpr(session, stored_identity);
-        if (status == PEP_STATUS_OK && 
-            stored_identity->comm_type >= PEP_ct_strong_but_unconfirmed) {
-          
-            free(identity->fpr);
-            identity->fpr = strdup(stored_identity->fpr);
-            valid_key_found = true;            
-        }
-        else {
-            bool revoked = false;
-            if (!EMPTYSTR(stored_identity->fpr)) {
+        if (status == PEP_OUT_OF_MEMORY)
+            goto pep_free;
+        if (status == PEP_STATUS_OK) {
+            if (stored_identity->comm_type >= PEP_ct_strong_but_unconfirmed) {
+                identity->fpr = strdup(stored_identity->fpr);
+                assert(identity->fpr);
+                if (!identity->fpr) {
+                    status = PEP_OUT_OF_MEMORY;
+                    goto pep_free;
+                }
+                valid_key_found = true;            
+            }
+            else {
+                bool revoked = false;
                 status = key_revoked(session, stored_identity->fpr, &revoked);
-                if (revoked)
+                if (status)
+                    goto pep_free;
+                if (revoked) {
                     revoked_fpr = strdup(stored_identity->fpr);
-            }        
+                    assert(revoked_fpr);
+                    if (!revoked_fpr) {
+                        status = PEP_OUT_OF_MEMORY;
+                        goto pep_free;
+                    }
+                }
+            }
         }
     }
     
@@ -1011,31 +1013,31 @@ pep_free:
     return status;
 }
 
-DYNAMIC_API PEP_STATUS initialise_own_identities(PEP_SESSION session,
-                                                 identity_list* my_idents) {
-    PEP_STATUS status = PEP_STATUS_OK;
-    if (!session)
-        return PEP_ILLEGAL_VALUE;
-        
-    if (!my_idents)
-        return PEP_STATUS_OK;
-            
-    identity_list* ident_curr = my_idents;
-    while (ident_curr) {
-        pEp_identity* ident = ident_curr->ident;
-        if (!ident || !ident->address) {
-            status = PEP_ILLEGAL_VALUE;
-            goto pep_error;
-        }
-
-        status = _myself(session, ident, false, false);
-        
-        ident_curr = ident_curr->next;
-    }
-    
-pep_error:
-    return status;
-}
+// DYNAMIC_API PEP_STATUS initialise_own_identities(PEP_SESSION session,
+//                                                  identity_list* my_idents) {
+//     PEP_STATUS status = PEP_STATUS_OK;
+//     if (!session)
+//         return PEP_ILLEGAL_VALUE;
+//         
+//     if (!my_idents)
+//         return PEP_STATUS_OK;
+//             
+//     identity_list* ident_curr = my_idents;
+//     while (ident_curr) {
+//         pEp_identity* ident = ident_curr->ident;
+//         if (!ident || !ident->address) {
+//             status = PEP_ILLEGAL_VALUE;
+//             goto pep_error;
+//         }
+// 
+//         status = _myself(session, ident, false, false);
+//         
+//         ident_curr = ident_curr->next;
+//     }
+//     
+// pep_error:
+//     return status;
+// }
 
 DYNAMIC_API PEP_STATUS myself(PEP_SESSION session, pEp_identity * identity)
 {
@@ -1618,69 +1620,43 @@ DYNAMIC_API PEP_STATUS own_keys_retrieve(PEP_SESSION session, stringlist_t **key
     return _own_keys_retrieve(session, keylist, 0);
 }
 
-// FIXME: should it be be used when sync receive old keys ? (ENGINE-145)
 DYNAMIC_API PEP_STATUS set_own_key(
        PEP_SESSION session,
-       const char *address,
+       pEp_identity *me,
        const char *fpr
     )
 {
     PEP_STATUS status = PEP_STATUS_OK;
     
-    assert(session &&
-           address &&
-           fpr && fpr[0]
-          );
-    
-    if (!(session &&
-          address &&
-          fpr && fpr[0]
-         ))
+    assert(session && me);
+    assert(!EMPTYSTR(fpr));
+    assert(!EMPTYSTR(me->address));
+    assert(!EMPTYSTR(me->user_id));
+    assert(!EMPTYSTR(me->username));
+
+    if (!session || !me || EMPTYSTR(fpr) || EMPTYSTR(me->address) ||
+            EMPTYSTR(me->user_id) || EMPTYSTR(me->username))
         return PEP_ILLEGAL_VALUE;
-                        
-    // First see if we have it in own identities already, AND we retrieve
-    // our own user_id
-    char* my_user_id = NULL;
-    status = get_default_own_userid(session, &my_user_id);
-    if (status != PEP_STATUS_OK)
+
+    status = _myself(session, me, false, true);
+    // we do not need a valid key but dislike other errors
+    if (status != PEP_STATUS_OK && status != PEP_GET_KEY_FAILED && status != PEP_KEY_UNSUITABLE)
         return status;
-        
-    if (!my_user_id) {
-        // We have no own user_id. So we cannot set it for an identity.
-        return PEP_CANNOT_FIND_IDENTITY;
-    }
-    
-    pEp_identity* my_id = NULL;
-    
-    status = get_identity(session, address, my_user_id, &my_id);
+    status = PEP_STATUS_OK;
+ 
+    if (me->fpr)
+        free(me->fpr);
+    me->fpr = strdup(fpr);
+    assert(me->fpr);
+    if (!me->fpr)
+        return PEP_OUT_OF_MEMORY;
 
-    if (status == PEP_STATUS_OK && my_id) {
-        if (my_id->fpr && strcasecmp(my_id->fpr, fpr) == 0) {
-            // We're done. It was already here.
-            goto pep_free;
-        }           
-    }
-                
-    // If there's an id w/ user_id + address
-    if (my_id) {
-        free(my_id->fpr);
-        my_id->fpr = my_user_id;
-        my_id->comm_type = PEP_ct_pEp;
-        my_id->me = true;
-    }
-    else { // Else, we need a new identity
-        my_id = new_identity(address, fpr, my_user_id, NULL); 
-        if (status != PEP_STATUS_OK)
-            goto pep_free; 
-        my_id->me = true;
-        my_id->comm_type = PEP_ct_pEp;
-    }
-        
-    status = set_identity(session, my_id);
-    
-pep_free:
-    free(my_id);
-    free(my_user_id);
+    status = validate_fpr(session, me);
+    if (status)
+        return status;
+
+    me->comm_type = PEP_ct_pEp;
+    status = set_identity(session, me);
     return status;
 }
 

src/keymanagement.h

@@ -54,6 +54,7 @@ DYNAMIC_API PEP_STATUS update_identity(
         PEP_SESSION session, pEp_identity * identity
     );
 
+// TODO: remove
 // initialise_own_identities () - ensures that an own identity is complete
 //
 //  parameters:
@@ -78,8 +79,8 @@ DYNAMIC_API PEP_STATUS update_identity(
 //      N.B. to adapter devs - this function is likely unnecessary, so please
 //      do not put work into exposing it yet. Tickets will be filed if need be.
 
-DYNAMIC_API PEP_STATUS initialise_own_identities(PEP_SESSION session,
-                                                 identity_list* my_idents);
+// DYNAMIC_API PEP_STATUS initialise_own_identities(PEP_SESSION session,
+//                                                  identity_list* my_idents);
 
 // myself() - ensures that an own identity is complete
 //
@@ -336,9 +337,24 @@ DYNAMIC_API PEP_STATUS own_keys_retrieve(
         stringlist_t **keylist
       );
 
+// set_own_key() - mark a key as own key
+//
+//  parameters:
+//      session (in)            session to use
+//      me (inout)              own identity this key is used for
+//      fpr (in)                fingerprint of the key to mark as own key
+//
+//  caveat:
+//      the key has to be in the key ring already
+//      me->address, me->user_id and me->username must be set to valid data
+//      myself() is called by set_own_key() without key generation
+//      me->flags are ignored
+//      me->address must not be an alias
+//      me->fpr will be ignored and replaced by fpr
+
 DYNAMIC_API PEP_STATUS set_own_key(
        PEP_SESSION session,
-       const char *address,
+       pEp_identity *me,
        const char *fpr
     );
 

test/apple_mail_test.cc

@@ -35,7 +35,7 @@ int main(int argc, char** argv) {
     const string mailtext = slurp(mailfile);
     pEp_identity * me = new_identity("pep.test.recip@kgrothoff.org", "93D19F24AD6F4C4BA9134AAF84D9217908DB0AEE", PEP_OWN_USERID, "pEp Test Recipient");    
     me->me = true;    
-    PEP_STATUS status = myself(session, me);
+    PEP_STATUS status = set_own_key(session, me, "93D19F24AD6F4C4BA9134AAF84D9217908DB0AEE");
     
     pEp_identity * you = new_identity("pep.test.apple@pep-project.org", NULL, "pep.test.apple@pep-project.org", "pEp Apple Test");    
     you->me = false;    

test/encrypt_for_identity_test.cc

@@ -40,7 +40,7 @@ int main() {
     pEp_identity* bob = new_identity("pep.test.bob@pep-project.org", NULL, "42", "Bob Test");
     alice->me = true;
 
-    PEP_STATUS mystatus = myself(session, alice);
+    PEP_STATUS mystatus = set_own_key(session, alice, "4ABE3AAF59AC32CFE4F86500A9411D176FF00E97");
     assert(mystatus == PEP_STATUS_OK);
 
     identity_list* to_list = new_identity_list(bob); // to bob

test/new_update_id_and_myself_test.cc

@@ -132,9 +132,7 @@ int main() {
 
     char* new_fpr = strdup(new_me->fpr);
 
-    new_me = new_identity(uniqname, new_fpr, alias_id, NULL);
-
-    status = myself(session, new_me);
+    status = set_own_key(session, new_me, new_fpr);
     assert(status == PEP_STATUS_OK);
     assert(new_me->fpr);
     assert(strcmp(new_me->fpr, generated_fpr) != 0);
@@ -153,7 +151,7 @@ int main() {
     free(new_me->fpr);
     new_me->fpr = strdup(generated_fpr);
     new_me->comm_type = PEP_ct_unknown;
-    status = myself(session, new_me);
+    status = set_own_key(session, new_me, generated_fpr);
     assert(status == PEP_STATUS_OK);
     assert(strcmp(new_me->fpr, generated_fpr) == 0);
     assert(new_me->comm_type == PEP_ct_pEp);
@@ -165,9 +163,9 @@ int main() {
     status = revoke_key(session, generated_fpr, "Because it's fun");
     assert (status == PEP_STATUS_OK);
     
-    new_me = new_identity(uniqname, new_fpr, alias_id, NULL);
+    new_me = new_identity(uniqname, NULL, alias_id, start_username);
     
-    status = myself(session, new_me);
+    status = set_own_key(session, new_me, new_fpr);
     assert(status == PEP_STATUS_OK);
     assert(new_me->fpr);
     assert(strcmp(new_me->fpr, generated_fpr) != 0);

test/revoke_regen_attach_test.cc

@@ -46,6 +46,7 @@ int main() {
 
     cout << "re-generated fingerprint \n";
     free(me->fpr);
+    me->fpr = NULL;
     status = myself(session, me);
     assert(status == PEP_STATUS_OK);
     cout << me->fpr << "\n";