ENGINE-669, probably - adds new status PGP_VERIFY_SENDER_KEY_REVOKED to decrypt_message returns (still to be implemented in pgp_gpg.c) and now avoids sending messages when a message from self contains a revoked own key sync
authorKrista 'DarthMama' Bennett <krista@pep.foundation>
Wed, 04 Dec 2019 15:28:34 +0100
branchsync
changeset 4234b720642cd9e0
parent 4231 52ef88f8da92
child 4235 1daffdbc64aa
ENGINE-669, probably - adds new status PGP_VERIFY_SENDER_KEY_REVOKED to decrypt_message returns (still to be implemented in pgp_gpg.c) and now avoids sending messages when a message from self contains a revoked own key
src/message_api.c
src/pEpEngine.h
src/pgp_sequoia.c
test/src/KeyResetMessageTest.cc
test/src/VerifyTest.cc
     1.1 --- a/src/message_api.c	Tue Dec 03 12:18:52 2019 +0100
     1.2 +++ b/src/message_api.c	Wed Dec 04 15:28:34 2019 +0100
     1.3 @@ -1208,6 +1208,7 @@
     1.4          return PEP_rating_unencrypted;
     1.5  
     1.6      case PEP_DECRYPTED:
     1.7 +    case PEP_VERIFY_SIGNER_KEY_REVOKED:
     1.8      case PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH:
     1.9          return PEP_rating_unreliable;
    1.10  
    1.11 @@ -3419,6 +3420,8 @@
    1.12      unsigned int major_ver = 0;
    1.13      unsigned int minor_ver = 0;
    1.14      
    1.15 +    stringpair_list_t* revoke_replace_pairs = NULL;
    1.16 +    
    1.17      // Grab input flags
    1.18      bool reencrypt = ((*flags & PEP_decrypt_flag_untrusted_server) &&
    1.19              (_have_extrakeys(*keylist) || session->unencrypted_subject));
    1.20 @@ -3546,9 +3549,7 @@
    1.21                  status = _mime_decode_message_internal(ptext, psize, &msg, &has_inner);
    1.22                  if (status != PEP_STATUS_OK)
    1.23                      goto pEp_error;
    1.24 -                
    1.25 -                /* Ensure messages whose maintext is in the attachments
    1.26 -                   move main text into message struct longmsg et al */
    1.27 +                                
    1.28                  /* KG: This IS a src modification of old - we're adding to it
    1.29                     w/ memhole subject, but the question is whether or not
    1.30                     this is OK overall... */
    1.31 @@ -3616,7 +3617,8 @@
    1.32          if (status != PEP_STATUS_OK)
    1.33              goto pEp_error;
    1.34  
    1.35 -        if (decrypt_status == PEP_DECRYPTED || decrypt_status == PEP_DECRYPTED_AND_VERIFIED) {
    1.36 +        if (decrypt_status == PEP_DECRYPTED || decrypt_status == PEP_DECRYPTED_AND_VERIFIED || 
    1.37 +            decrypt_status == PEP_VERIFY_SIGNER_KEY_REVOKED) {
    1.38              char* wrap_info = NULL;
    1.39              
    1.40              if (!has_inner) {
    1.41 @@ -3888,67 +3890,69 @@
    1.42      } // End prepare output message for return
    1.43  
    1.44      // 3. Check to see if the sender used any of our revoked keys
    1.45 -    stringpair_list_t* revoke_replace_pairs = NULL;
    1.46 -    status = check_for_own_revoked_key(session, _keylist, &revoke_replace_pairs);
    1.47 -
    1.48 -    //assert(status != PEP_STATUS_OK); // FIXME: FOR DEBUGGING ONLY DO NOT LEAVE IN    
    1.49 -    if (status != PEP_STATUS_OK) {
    1.50 -        // This should really never choke unless the DB is broken.
    1.51 -        status = PEP_UNKNOWN_DB_ERROR;
    1.52 -        goto pEp_error;
    1.53 -    }
    1.54 -    
    1.55 -    if (msg) {
    1.56 -        stringpair_list_t* curr_pair_node;
    1.57 -        stringpair_t* curr_pair;
    1.58 -
    1.59 -        for (curr_pair_node = revoke_replace_pairs; curr_pair_node; curr_pair_node = curr_pair_node->next) {
    1.60 -            curr_pair = curr_pair_node->value;
    1.61 -
    1.62 -            if (!curr_pair)
    1.63 -                continue; // Again, shouldn't occur
    1.64 -
    1.65 -            if (curr_pair->key && curr_pair->value) {
    1.66 -                status = create_standalone_key_reset_message(session,
    1.67 -                    &reset_msg,
    1.68 -                    msg->from,
    1.69 -                    curr_pair->key,
    1.70 -                    curr_pair->value);
    1.71 -
    1.72 -                // If we can't find the identity, this is someone we've never mailed, so we just
    1.73 -                // go on letting them use the wrong key until we mail them ourselves. (Spammers, etc)
    1.74 -                if (status != PEP_CANNOT_FIND_IDENTITY) {
    1.75 -                    if (status != PEP_STATUS_OK)
    1.76 -                        goto pEp_error;
    1.77 -
    1.78 -                    if (!reset_msg) {
    1.79 -                        status = PEP_OUT_OF_MEMORY;
    1.80 -                        goto pEp_error;
    1.81 -                    }
    1.82 -                    // insert into queue
    1.83 -                    if (session->messageToSend)
    1.84 -                        status = session->messageToSend(reset_msg);
    1.85 -                    else
    1.86 -                        status = PEP_SYNC_NO_MESSAGE_SEND_CALLBACK;
    1.87 -
    1.88 -
    1.89 -                    if (status == PEP_STATUS_OK) {
    1.90 -                        // Put into notified DB
    1.91 -                        status = set_reset_contact_notified(session, curr_pair->key, msg->from->user_id);
    1.92 -                        if (status != PEP_STATUS_OK) // It's ok to barf because it's a DB problem??
    1.93 +    if (!is_me(session, msg->from)) {
    1.94 +        status = check_for_own_revoked_key(session, _keylist, &revoke_replace_pairs);
    1.95 +
    1.96 +        //assert(status != PEP_STATUS_OK); // FIXME: FOR DEBUGGING ONLY DO NOT LEAVE IN    
    1.97 +        if (status != PEP_STATUS_OK) {
    1.98 +            // This should really never choke unless the DB is broken.
    1.99 +            status = PEP_UNKNOWN_DB_ERROR;
   1.100 +            goto pEp_error;
   1.101 +        }
   1.102 +        
   1.103 +        if (msg) {
   1.104 +            stringpair_list_t* curr_pair_node;
   1.105 +            stringpair_t* curr_pair;
   1.106 +
   1.107 +            for (curr_pair_node = revoke_replace_pairs; curr_pair_node; curr_pair_node = curr_pair_node->next) {
   1.108 +                curr_pair = curr_pair_node->value;
   1.109 +
   1.110 +                if (!curr_pair)
   1.111 +                    continue; // Again, shouldn't occur
   1.112 +
   1.113 +                if (curr_pair->key && curr_pair->value) {
   1.114 +                    status = create_standalone_key_reset_message(session,
   1.115 +                        &reset_msg,
   1.116 +                        msg->from,
   1.117 +                        curr_pair->key,
   1.118 +                        curr_pair->value);
   1.119 +
   1.120 +                    // If we can't find the identity, this is someone we've never mailed, so we just
   1.121 +                    // go on letting them use the wrong key until we mail them ourselves. (Spammers, etc)
   1.122 +                    if (status != PEP_CANNOT_FIND_IDENTITY) {
   1.123 +                        if (status != PEP_STATUS_OK)
   1.124                              goto pEp_error;
   1.125 -                    }
   1.126 -                    else {
   1.127 -                        // According to Volker, this would only be a fatal error, so...
   1.128 -                        free_message(reset_msg); // ??
   1.129 -                        reset_msg = NULL; // ??
   1.130 -                        goto pEp_error;
   1.131 +
   1.132 +                        if (!reset_msg) {
   1.133 +                            status = PEP_OUT_OF_MEMORY;
   1.134 +                            goto pEp_error;
   1.135 +                        }
   1.136 +                        // insert into queue
   1.137 +                        if (session->messageToSend)
   1.138 +                            status = session->messageToSend(reset_msg);
   1.139 +                        else
   1.140 +                            status = PEP_SYNC_NO_MESSAGE_SEND_CALLBACK;
   1.141 +
   1.142 +
   1.143 +                        if (status == PEP_STATUS_OK) {
   1.144 +                            // Put into notified DB
   1.145 +                            status = set_reset_contact_notified(session, curr_pair->key, msg->from->user_id);
   1.146 +                            if (status != PEP_STATUS_OK) // It's ok to barf because it's a DB problem??
   1.147 +                                goto pEp_error;
   1.148 +                        }
   1.149 +                        else {
   1.150 +                            // According to Volker, this would only be a fatal error, so...
   1.151 +                            free_message(reset_msg); // ??
   1.152 +                            reset_msg = NULL; // ??
   1.153 +                            goto pEp_error;
   1.154 +                        }
   1.155                      }
   1.156                  }
   1.157              }
   1.158          }
   1.159 -    }
   1.160 -
   1.161 +        free_stringpair_list(revoke_replace_pairs);
   1.162 +        revoke_replace_pairs = NULL;
   1.163 +    } // end !is_me(msg->from)    
   1.164  
   1.165      bool reenc_signer_key_is_own_key = false; // only matters for reencrypted messages 
   1.166      
   1.167 @@ -3964,7 +3968,8 @@
   1.168      }    
   1.169  
   1.170      if (reencrypt) {
   1.171 -        if (decrypt_status == PEP_DECRYPTED || decrypt_status == PEP_DECRYPTED_AND_VERIFIED) {
   1.172 +        if (decrypt_status == PEP_DECRYPTED || decrypt_status == PEP_DECRYPTED_AND_VERIFIED
   1.173 +            || decrypt_status == PEP_VERIFY_SIGNER_KEY_REVOKED) {
   1.174              const char* sfpr = NULL;
   1.175              if (has_extra_keys)
   1.176                  sfpr = _keylist->value;
   1.177 @@ -4049,6 +4054,7 @@
   1.178      free_message(msg);
   1.179      free_message(reset_msg);
   1.180      free_stringlist(_keylist);
   1.181 +    free_stringpair_list(revoke_replace_pairs);
   1.182  
   1.183      return status;
   1.184  }
     2.1 --- a/src/pEpEngine.h	Tue Dec 03 12:18:52 2019 +0100
     2.2 +++ b/src/pEpEngine.h	Wed Dec 04 15:28:34 2019 +0100
     2.3 @@ -84,8 +84,10 @@
     2.4      PEP_VERIFY_NO_KEY                               = 0x0407,
     2.5      PEP_VERIFIED_AND_TRUSTED                        = 0x0408,
     2.6      PEP_CANNOT_REENCRYPT                            = 0x0409,
     2.7 +    PEP_VERIFY_SIGNER_KEY_REVOKED                   = 0x040a,
     2.8      PEP_CANNOT_DECRYPT_UNKNOWN                      = 0x04ff,
     2.9  
    2.10 +
    2.11      PEP_TRUSTWORD_NOT_FOUND                         = 0x0501,
    2.12      PEP_TRUSTWORDS_FPR_WRONG_LENGTH                 = 0x0502,
    2.13      PEP_TRUSTWORDS_DUPLICATE_FPR                    = 0x0503,
     3.1 --- a/src/pgp_sequoia.c	Tue Dec 03 12:18:52 2019 +0100
     3.2 +++ b/src/pgp_sequoia.c	Wed Dec 04 15:28:34 2019 +0100
     3.3 @@ -1487,7 +1487,7 @@
     3.4              status = PEP_DECRYPTED_AND_VERIFIED;
     3.5          } else if (cookie.good_but_revoked) {
     3.6              // If there are any signatures from revoked keys, fail.
     3.7 -            status = PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH;
     3.8 +            status = PEP_VERIFY_SIGNER_KEY_REVOKED;
     3.9          } else if (cookie.bad_checksums) {
    3.10              // If there are any bad signatures, fail.
    3.11              status = PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH;
    3.12 @@ -1611,7 +1611,7 @@
    3.13              status = PEP_UNENCRYPTED;
    3.14          } else if (cookie.good_but_revoked) {
    3.15              // If there are any signatures from revoked keys, fail.
    3.16 -            status = PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH;
    3.17 +            status = PEP_VERIFY_SIGNER_KEY_REVOKED;
    3.18          } else if (cookie.bad_checksums) {
    3.19              // If there are any bad signatures, fail.
    3.20              status = PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH;
     4.1 --- a/test/src/KeyResetMessageTest.cc	Tue Dec 03 12:18:52 2019 +0100
     4.2 +++ b/test/src/KeyResetMessageTest.cc	Wed Dec 04 15:28:34 2019 +0100
     4.3 @@ -1217,6 +1217,36 @@
     4.4      myfile.close();      
     4.5  }
     4.6  
     4.7 +TEST_F(KeyResetMessageTest, check_no_reset_message_to_self) {
     4.8 +    pEp_identity* bob = NULL;
     4.9 +    PEP_STATUS status = set_up_preset(session, BOB,
    4.10 +                                      true, true, true, true, true, &bob);
    4.11 +                                                                
    4.12 +    slurp_and_import_key(session, "test_keys/pub/pep-test-bob-0xC9C2EE39_pub.asc");    
    4.13 +                                          
    4.14 +    message* bob_msg = new_message(PEP_dir_outgoing);
    4.15 +    bob_msg->from = identity_dup(bob);
    4.16 +    bob_msg->to = new_identity_list(identity_dup(bob));
    4.17 +    bob_msg->shortmsg = strdup("Engine bugs suck\n");
    4.18 +    bob_msg->longmsg = strdup("Everything is the engine's fault.\n");
    4.19 +    
    4.20 +    message* enc_msg = NULL;
    4.21 +    
    4.22 +    status = encrypt_message(session, bob_msg, NULL, &enc_msg, PEP_enc_PGP_MIME, 0);
    4.23 +    ASSERT_EQ(status, PEP_STATUS_OK);
    4.24 +
    4.25 +    key_reset_all_own_keys(session);
    4.26 +    
    4.27 +    message* dec_msg = NULL;
    4.28 +    stringlist_t* keylist = NULL;
    4.29 +    PEP_rating rating;
    4.30 +    PEP_decrypt_flags_t flags = 0;
    4.31 +    
    4.32 +    status = decrypt_message(session, enc_msg, &dec_msg, &keylist, &rating, &flags);
    4.33 +    ASSERT_EQ(m_queue.size(), 0);
    4.34 +    ASSERT_EQ(status, PEP_VERIFY_SIGNER_KEY_REVOKED);
    4.35 +}
    4.36 +
    4.37  
    4.38  TEST_F(KeyResetMessageTest, check_reset_mistrust_next_msg_have_not_mailed) {
    4.39      pEp_identity* carol = NULL;
     5.1 --- a/test/src/VerifyTest.cc	Tue Dec 03 12:18:52 2019 +0100
     5.2 +++ b/test/src/VerifyTest.cc	Wed Dec 04 15:28:34 2019 +0100
     5.3 @@ -126,7 +126,7 @@
     5.4                                  &keylist, NULL);
     5.5  
     5.6      // Now it should fail.
     5.7 -    ASSERT_EQ(status , PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH);
     5.8 +    ASSERT_EQ(status , PEP_VERIFY_SIGNER_KEY_REVOKED);
     5.9      ASSERT_NE(keylist, nullptr);
    5.10      // No signer.
    5.11      ASSERT_STREQ(keylist->value, "");
    5.12 @@ -150,7 +150,7 @@
    5.13                           &keylist);
    5.14  
    5.15      // Now it should fail.
    5.16 -    ASSERT_EQ(status , PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH);
    5.17 +    ASSERT_EQ(status , PEP_VERIFY_SIGNER_KEY_REVOKED);
    5.18      ASSERT_NE(keylist, nullptr);
    5.19      // No signer.
    5.20      ASSERT_STREQ(keylist->value, "");
    5.21 @@ -175,7 +175,7 @@
    5.22                                             &keylist, NULL);
    5.23  
    5.24      // It should fail.
    5.25 -    ASSERT_EQ(status , PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH);
    5.26 +    ASSERT_EQ(status , PEP_VERIFY_SIGNER_KEY_REVOKED);
    5.27      ASSERT_NE(keylist, nullptr);
    5.28      // No signer.
    5.29      ASSERT_STREQ(keylist->value, "");
    5.30 @@ -199,7 +199,7 @@
    5.31                           &keylist);
    5.32  
    5.33      // Now it should fail.
    5.34 -    ASSERT_EQ(status , PEP_DECRYPT_SIGNATURE_DOES_NOT_MATCH);
    5.35 +    ASSERT_EQ(status , PEP_VERIFY_SIGNER_KEY_REVOKED);
    5.36      ASSERT_NE(keylist, nullptr);
    5.37      // No signer.
    5.38      ASSERT_STREQ(keylist->value, "");