ENGINE-847: incremental repair of er... yeah. I think the ratings calculus needs a little more thought. Will do what I can before I go, but I am not sure this is really going to end up being the elegant solution intended, so pls look carefully at it, fdik.

src/message_api.c

@@ -4447,7 +4447,7 @@ static void get_protocol_version_from_headers(
         pEp_version_major_minor(pEp_protocol_version->value->value, major_ver, minor_ver);           
 }
 
-static void get_message_version_from_headers(
+void get_message_version_from_headers(
         stringpair_list_t* field_list,
         unsigned int* major_ver,
         unsigned int* minor_ver
@@ -5373,18 +5373,14 @@ static PEP_STATUS _decrypt_message(
                         free_message(inner_message);
                     }
                 } // end if (message_blob)
-                
-                //  else if (strcmp(wrap_info, "TRANSPORT") == 0) {
-                //      // FIXME: this gets even messier.
-                //      // (TBI in ENGINE-278)
-                //  }
-                //  else {} // shouldn't be anything to be done here
-    
             } // end if (has_inner || wrap_info)
             else {
-                
-            } // this we do if this isn't an inner message
-            
+                // this we do if this isn't an inner message
+                if (_keylist && !EMPTYSTR(_keylist->value)) {
+                    msg->_sender_fpr = strdup(_keylist->value);
+                }
+            }
+
             pEp_identity* msg_from = msg->from;
             if (msg_from && !EMPTYSTR(msg_from->address)) {
                 if (!is_me(session, msg_from)) {
@@ -5489,12 +5485,6 @@ static PEP_STATUS _decrypt_message(
             if (!msg->recv_by)
                 goto enomem;
         }
-        
-        // Adjust the incoming message rating? I think we have a problem here with reencrypted messages,
-        // but I don't know what vb changed in this branch here...
-        status = incoming_message_rating(session, src, msg, _keylist, extra, decrypt_status, rating);
-
-        decorate_message(session, msg, *rating, _keylist, false, true);
 
         // Maybe unnecessary
         // if (keys_were_imported)
@@ -5703,7 +5693,8 @@ static PEP_STATUS _decrypt_message(
     
     if (reencrypt && session->unencrypted_subject && !has_extra_keys && subjects_match) 
         reencrypt = false;
-    
+
+
     if (reencrypt) {
         if (decrypt_status == PEP_DECRYPTED || decrypt_status == PEP_DECRYPTED_AND_VERIFIED
             || decrypt_status == PEP_VERIFY_SIGNER_KEY_REVOKED) {
@@ -5793,25 +5784,64 @@ static PEP_STATUS _decrypt_message(
         }
     }
 
-    // 5. Set up return values
-    *dst = msg;
-    *keylist = _keylist;        
-    
+    // 5. Double-check sender key info
+
     // Double-check for message 2.1+: (note, we don't do this for already-reencrypted-messages)
-    if (!(reencrypt && reenc_signer_key_is_own_key)) { 
-        if (major_ver > 2 || (major_ver == 2 && minor_ver > 0)) {
-            if (msg_major_ver > 2 || (msg_major_ver == 2 && msg_minor_ver > 0)) {
-                if (EMPTYSTR((*dst)->_sender_fpr) || 
-                (!EMPTYSTR(_keylist->value) && (strcasecmp((*dst)->_sender_fpr, _keylist->value) != 0))) {
-                    if (decrypt_status == PEP_DECRYPTED_AND_VERIFIED)
-                        decrypt_status = PEP_DECRYPTED;
-                    if (*rating > PEP_rating_unreliable)
-                        *rating = PEP_rating_unreliable;
-                }
+
+    // Rating, so far, is based on the decrypt rating. We need to adjust a little and then update
+    // the rating with the function.
+
+    if (!(reencrypt && reenc_signer_key_is_own_key) && rating > PEP_rating_unreliable) {
+        // We should now ALWAYS have sender_fpr filled in
+        if (!EMPTYSTR(msg->_sender_fpr)) {
+            // Is the sender key the signer key? They should be.
+            if (!EMPTYSTR(_keylist->value) && (strcasecmp(msg->_sender_fpr, _keylist->value) != 0)) {
+                if (decrypt_status == PEP_DECRYPTED_AND_VERIFIED)
+                    decrypt_status = PEP_DECRYPTED;
+                *rating = PEP_rating_unreliable;
             }
+//            else {
+//                const pEp_identity *msg_from = msg->from;
+//                const char *sender_user_id = msg_from->user_id;
+//                if (*rating > PEP_rating_unreliable) {
+//                    if (msg_from->fpr && strcasecmp(msg_from->fpr, msg->_sender_fpr) != 0) {
+//                        // check to see if the sender OWNS this key and we have a trust entry
+//                        // for it. Otherwise... PROBLEMS.
+//                        pEp_identity *temp_from = identity_dup(msg_from);
+//                        if (!temp_from)
+//                            goto enomem;
+//                        free(temp_from->fpr);
+//                        temp_from->fpr = strdup(msg->_sender_fpr);
+//                        if (!temp_from->fpr)
+//                            goto enomem;
+//                        status = get_trust(session, temp_from);
+//                        if (status == PEP_CANNOT_FIND_IDENTITY) {
+//                            if (decrypt_status == PEP_DECRYPTED_AND_VERIFIED)
+//                                decrypt_status = PEP_DECRYPTED;
+//                            *rating = PEP_rating_unreliable;
+//                        }
+//                    }
+//                }
+//            }
+// Covered by incoming message rating, I hope.
         }
     }
-        
+
+    // Adjust the incoming message rating? I think we have a problem here with reencrypted messages,
+    // but I don't know what vb changed in this branch here...
+    status = incoming_message_rating(session, src, msg, _keylist, extra, decrypt_status, rating);
+
+    if (*rating == PEP_rating_have_no_key)
+        *rating = PEP_rating_unreliable;
+
+    // 6. Put this stuff on the message
+    decorate_message(session, msg, *rating, _keylist, false, true);
+
+
+    // 7. Set up return values
+    *dst = msg;
+    *keylist = _keylist;
+
     if (imported_key_fprs)
         *imported_key_fprs = _imported_key_list;
     if (changed_public_keys)
@@ -5879,11 +5909,14 @@ DYNAMIC_API PEP_STATUS decrypt_message(
                                          rating, flags, NULL,
                                          &imported_key_fprs, &changed_key_bitvec);
 
+    /*
     if (src->dir == PEP_dir_incoming) {
         PEP_rating rating2;
         PEP_STATUS status2 = incoming_message_rating(session, src, *dst, *keylist, NULL, status, rating);
         assert(status2 != PEP_ILLEGAL_VALUE);
     }
+    */
+    // This gets done internally anyway. Whyyyyy?
 
     message *msg = *dst ? *dst : src;
 
@@ -6784,3 +6817,38 @@ PEP_STATUS try_encrypt_message(
 
     return status;
 }
+
+PEP_STATUS update_identity_list(PEP_SESSION session, identity_list* idents) {
+    PEP_STATUS status = PEP_STATUS_OK;
+    if (idents) {
+        identity_list* il = idents;
+        for ( ; il && il->ident; il = il->next) {
+            if (is_me(session, il->ident))
+                status = myself(session, il->ident);
+            else
+                status = update_identity(session, il->ident);
+        }
+    }
+    return status;
+}
+
+PEP_STATUS update_message_identities(PEP_SESSION session, message* msg) {
+    PEP_STATUS status = PEP_STATUS_OK;
+
+    if (msg->from) {
+        if (is_me(session, msg->from))
+            status = myself(session, msg->from);
+        else
+            status = update_identity(session, msg->from);
+    }
+    if (status == PEP_STATUS_OK) {
+        status = update_identity_list(session, msg->to);
+    }
+    if (status == PEP_STATUS_OK) {
+        status = update_identity_list(session, msg->cc);
+    }
+    if (status == PEP_STATUS_OK) {
+        status = update_identity_list(session, msg->bcc);
+    }
+    return status;
+}

src/message_api_internal.h

@@ -106,6 +106,37 @@ PEP_STATUS try_encrypt_message(
         PEP_encrypt_flags_t flags
     );
 
+/**
+ *  @internal
+ *
+ *  @param session
+ *  @param idents
+ *  @return
+ */
+PEP_STATUS update_identity_list(PEP_SESSION session, identity_list* idents);
+
+/**
+ *  @internal
+ *
+ *  @param session
+ *  @param msg
+ *  @return
+ */
+PEP_STATUS update_message_identities(PEP_SESSION session, message* msg);
+
+/**
+ *  @internal
+ *
+ *  @param field_list
+ *  @param major_ver
+ *  @param minor_ver
+ */
+void get_message_version_from_headers(
+        stringpair_list_t* field_list,
+        unsigned int* major_ver,
+        unsigned int* minor_ver
+    );
+
 #ifdef __cplusplus
 }
 #endif

src/rating_api.c

@@ -11,6 +11,7 @@
 #include "baseprotocol.h"
 #include "KeySync_fsm.h"
 #include "sync_codec.h"
+#include "message_api_internal.h"
 
 const char * rating_to_string(PEP_rating rating)
 {
@@ -398,7 +399,7 @@ static PEP_STATUS message_rating_for_identities(
     *rating = PEP_rating_undefined;
 
     if (msg->dir == PEP_dir_incoming) {
-        if (msg->from->me)
+        if (is_me(session, msg->from))
             status = myself(session, msg->from);
         else
             status = update_identity(session, msg->from);
@@ -420,6 +421,9 @@ static PEP_STATUS message_rating_for_identities(
 
     *rating = add_rating(from_rating, _rating);
 
+    if (*rating == PEP_rating_have_no_key)
+        *rating = PEP_rating_unreliable;
+    
 the_end:
     return status;
 }
@@ -518,11 +522,16 @@ static PEP_STATUS incoming_message_crypto_rating(
 
     if (dst) {
         PEP_rating sender_rating = PEP_rating_undefined;
-        PEP_STATUS status = sender_fpr_rating(session, src->from, dst->_sender_fpr, &sender_rating);
-        if (status)
+        PEP_STATUS status = sender_fpr_rating(session, dst->from, dst->_sender_fpr, &sender_rating);
+        if (status == PEP_CANNOT_FIND_IDENTITY)
+            status = PEP_STATUS_OK; // this is legit if we don't have the key
+        if (status != PEP_STATUS_OK)
             return status;
 
-        *rating = add_rating(enc_rating, sender_rating);
+        if (sender_rating == PEP_rating_undefined)
+            *rating = enc_rating > PEP_rating_unreliable ? PEP_rating_unreliable : enc_rating;
+        else
+            *rating = add_rating(enc_rating, sender_rating);
     }
 
     return PEP_STATUS_OK;
@@ -681,41 +690,6 @@ PEP_STATUS set_receiverRating(PEP_SESSION session, message *msg, PEP_rating rati
     return base_decorate_message(session, msg, BASE_SYNC, payload, size, msg->recv_by->fpr);
 }
 
-static PEP_STATUS _update_identity_list(PEP_SESSION session, identity_list* idents) {
-    PEP_STATUS status = PEP_STATUS_OK;
-    if (idents) {
-        identity_list* il = idents;
-        for ( ; il && il->ident; il = il->next) {
-            if (is_me(session, il->ident))
-                status = myself(session, il->ident);
-            else
-                status = update_identity(session, il->ident);
-        }
-    }
-    return status;
-}
-
-static PEP_STATUS _update_message_identities(PEP_SESSION session, message* msg) {
-    PEP_STATUS status = PEP_STATUS_OK;
-
-    if (msg->from) {
-        if (is_me(session, msg->from))
-            status = myself(session, msg->from);
-        else
-            status = update_identity(session, msg->from);
-    }
-    if (status == PEP_STATUS_OK) {
-        status = _update_identity_list(session, msg->to);
-    }
-    if (status == PEP_STATUS_OK) {
-        status = _update_identity_list(session, msg->cc);
-    }
-    if (status == PEP_STATUS_OK) {
-        status = _update_identity_list(session, msg->bcc);
-    }
-    return status;
-}
-
 DYNAMIC_API PEP_STATUS incoming_message_rating(
         PEP_SESSION session,
         const message *src,
@@ -743,6 +717,8 @@ DYNAMIC_API PEP_STATUS incoming_message_rating(
     if (!dst && known_keys && known_keys->value && known_keys->next)
         return PEP_ILLEGAL_VALUE;
 
+    PEP_STATUS status = PEP_STATUS_OK;
+
     *rating = PEP_rating_undefined;
     PEP_rating _rating = decrypt_rating(decrypt_status);
 
@@ -762,16 +738,27 @@ DYNAMIC_API PEP_STATUS incoming_message_rating(
     }
 
     PEP_rating crypto_rating = PEP_rating_undefined;
-    PEP_STATUS status = incoming_message_crypto_rating(session, src, dst,
+    if (src->from && is_me(session, src->from))
+        status = myself(session, src->from);
+    else
+        status = update_identity(session, src->from);
+
+    if (status)
+        return status;
+
+    if (dst)
+        status = update_message_identities(session, dst);
+
+    if (status)
+        return status;
+
+    status = incoming_message_crypto_rating(session, src, dst,
             &crypto_rating);
     if (status)
         return status;
     _rating = add_rating(_rating, crypto_rating);
 
     if (dst) {
-        status = _update_message_identities(session, dst);
-        if (status)
-            return status;
         PEP_rating identities_rating = PEP_rating_undefined;
         status = message_rating_for_identities(session, dst,
                 &identities_rating);

test/src/HeaderKeyImportTest.cc

@@ -719,6 +719,8 @@ TEST_F(HeaderKeyImportTest, check_header_key_import) {
     const char* alice_fpr = "4ABE3AAF59AC32CFE4F86500A9411D176FF00E97";
     slurp_and_import_key(session, "test_keys/pub/pep-test-bob-0xC9C2EE39_pub.asc");
     slurp_and_import_key(session, "test_keys/priv/pep-test-bob-0xC9C2EE39_priv.asc");
+    pEp_identity* bob = new_identity("pep.test.bob@pep-project.org", NULL, "BOB", "Bob Dog");
+    PEP_STATUS status = set_own_key(session, bob, "BFCDB7F301DEEEBBF947F29659BFF488C9C2EE39");
 
     string strmsg = slurp("test_mails/Header_key_import.eml");
 
@@ -731,7 +733,7 @@ TEST_F(HeaderKeyImportTest, check_header_key_import) {
 
     flags = 0;
     message* enc_msg = string_to_msg(strmsg);
-    PEP_STATUS status = decrypt_message(session, enc_msg, &dec_msg, &keylist, &rating, &flags);
+    status = decrypt_message(session, enc_msg, &dec_msg, &keylist, &rating, &flags);
     ASSERT_EQ(rating , PEP_rating_reliable);
     ASSERT_OK;