ENGINE-781: hey, at least it doesn't break stuff. Existing tests pass. ENGINE-781
authorKrista 'DarthMama' Bennett <krista@pep.foundation>
Fri, 31 Jul 2020 15:04:07 +0200
branchENGINE-781
changeset 4919b16e57f30d89
parent 4918 9cb1db279495
child 4920 0b584b77b1f6
ENGINE-781: hey, at least it doesn't break stuff. Existing tests pass.
src/key_reset.c
src/keymanagement.c
src/pEpEngine.c
src/pEpEngine.h
src/pEp_internal.h
test/src/Engine.cc
test/src/SyncTest.cc
     1.1 --- a/src/key_reset.c	Fri Jul 31 11:22:55 2020 +0200
     1.2 +++ b/src/key_reset.c	Fri Jul 31 15:04:07 2020 +0200
     1.3 @@ -973,31 +973,50 @@
     1.4  }
     1.5  
     1.6  static PEP_STATUS _check_own_reset_passphrase_readiness(PEP_SESSION session,
     1.7 -                                                        const char* key) {
     1.8 -    // Before we do anything else, make sure the key to sign the 
     1.9 -    // revocation has the right passphrase set
    1.10 -    PEP_STATUS status = probe_encrypt(session, key);
    1.11 -    
    1.12 -    // We only care if this has signing-related issues; key could 
    1.13 -    // already be revoked and it will be fine below.
    1.14 -    if (PASS_ERROR(status))
    1.15 -        return status;
    1.16 -    
    1.17 -    // Because of the above, we CAN support a signing passphrase 
    1.18 +                                                        const char* key) { 
    1.19 +
    1.20 +    // Check generation setup
    1.21 +    // Because of the above, we can support a signing passphrase 
    1.22      // that differs from the generation passphrase. We'll 
    1.23      // just check to make sure everything is in order for 
    1.24      // later use, however
    1.25 -
    1.26      if (session->new_key_pass_enable) {
    1.27          if (EMPTYSTR(session->generation_passphrase))
    1.28              return PEP_PASSPHRASE_FOR_NEW_KEYS_REQUIRED;
    1.29 +    }
    1.30 +                                
    1.31 +    stringlist_t* test_key = NULL;
    1.32 +                              
    1.33 +    // Be sure we HAVE this key
    1.34 +    PEP_STATUS status = find_keys(session, key, &test_key);
    1.35 +    bool exists_key = test_key != NULL;
    1.36 +    free_stringlist(test_key);    
    1.37  
    1.38 -        if (EMPTYSTR(session->curr_passphrase)) {
    1.39 -            // We'll need it as the current passphrase to sign 
    1.40 -            // messages with the generated keys
    1.41 -            session->curr_passphrase = strdup(session->generation_passphrase);
    1.42 -        }        
    1.43 -    } 
    1.44 +    if (!exists_key || status == PEP_KEY_NOT_FOUND) {
    1.45 +        remove_fpr_as_default(session, key);
    1.46 +        return PEP_KEY_NOT_FOUND;
    1.47 +    }        
    1.48 +    if (status != PEP_STATUS_OK)
    1.49 +        return status;
    1.50 +            
    1.51 +    ensure_passphrase_t ensure_key_cb = session->ensure_passphrase;
    1.52 +    
    1.53 +    // Check to see that this key has its passphrase set as the configured 
    1.54 +    // passphrase, IF it has one. If not, bail early.
    1.55 +    status = probe_encrypt(session, key);
    1.56 +    if (PASS_ERROR(status)) {
    1.57 +        if (ensure_key_cb)
    1.58 +            status = ensure_key_cb(session, key);
    1.59 +    }
    1.60 +    if (status != PEP_STATUS_OK)
    1.61 +        return status;
    1.62 +                            
    1.63 +    if (EMPTYSTR(session->curr_passphrase) && !EMPTYSTR(session->generation_passphrase)) {
    1.64 +        // We'll need it as the current passphrase to sign 
    1.65 +        // messages with the generated keys
    1.66 +        session->curr_passphrase = strdup(session->generation_passphrase);
    1.67 +    }        
    1.68 +                                                          
    1.69      return PEP_STATUS_OK;                                                       
    1.70  }
    1.71  
    1.72 @@ -1024,27 +1043,21 @@
    1.73      
    1.74      if (!session || !key_idents || EMPTYSTR(old_key))
    1.75          return PEP_ILLEGAL_VALUE;
    1.76 -            
    1.77 -    message* enc_msg = NULL;
    1.78 -    message* outmsg = NULL;
    1.79 -            
    1.80 +
    1.81      messageToSend_t send_cb = session->messageToSend;
    1.82 +    
    1.83      if (!send_cb)
    1.84          return PEP_SYNC_NO_MESSAGE_SEND_CALLBACK;
    1.85  
    1.86      PEP_STATUS status = PEP_STATUS_OK;
    1.87 +            
    1.88 +    message* enc_msg = NULL;
    1.89 +    message* outmsg = NULL;
    1.90 +    stringlist_t* test_key = NULL;
    1.91 +            
    1.92  
    1.93 -    // N.B. The mechanism here is that we make the caller, if 
    1.94 -    //      necessary, keep trying until they give us the 
    1.95 -    //      right password for the key we're going to reset/revoke. 
    1.96 -    //      The revocation does not happen until later, but basically,
    1.97 -    //      if the key is unrevoked and still requires a correct 
    1.98 -    //      password, we need to stop this before we get there.
    1.99 -    //      
   1.100 -    //      This allows us to switch the correct passphrase in 
   1.101 -    //      and out if there are different generation and old 
   1.102 -    //      key signing passwords. (Not a concern if already revoked)
   1.103 -
   1.104 +    // Make sure the signing password is set correctly and that 
   1.105 +    // we are also ready for keygen
   1.106      status = _check_own_reset_passphrase_readiness(session, old_key);
   1.107      if (status != PEP_STATUS_OK)
   1.108          return status;
   1.109 @@ -1127,12 +1140,12 @@
   1.110      status = revoke_key(session, old_key, NULL);
   1.111  
   1.112      // again, we should not have key-related issues here,
   1.113 -    // as we tested for the correct password earlier
   1.114 +    // as we ensured the correct password earlier
   1.115      if (status != PEP_STATUS_OK)
   1.116          return status;
   1.117          
   1.118      // Ok, NOW - the current password needs to be swapped out 
   1.119 -    // because we're going to sign with keys using.
   1.120 +    // because we're going to sign with keys using it.
   1.121      //
   1.122      // All new keys have the same passphrase, if any
   1.123      //
   1.124 @@ -1207,9 +1220,9 @@
   1.125      return status;
   1.126      
   1.127  pEp_error:
   1.128 -
   1.129      // Just in case
   1.130      session->curr_passphrase = cached_passphrase;
   1.131 +    free_stringlist(test_key);
   1.132      free_message(outmsg);
   1.133      free_message(enc_msg);
   1.134      return status;
   1.135 @@ -1228,8 +1241,6 @@
   1.136      return PEP_STATUS_OK;               
   1.137  }
   1.138  
   1.139 -// This is simply NOT SAFE for multiple passwords on the extant 
   1.140 -// keys. Cannot be called with multiple passwords for that purpose.
   1.141  DYNAMIC_API PEP_STATUS key_reset_own_grouped_keys(PEP_SESSION session) {
   1.142      assert(session);
   1.143      
   1.144 @@ -1247,12 +1258,7 @@
   1.145      status = get_all_keys_for_user(session, user_id, &keys);
   1.146  
   1.147      // TODO: free
   1.148 -    if (status == PEP_STATUS_OK) {
   1.149 -        status = probe_signing_for_keylist(session, keys);
   1.150 -        
   1.151 -        if (PASS_ERROR(status))
   1.152 -            goto pEp_free;
   1.153 -            
   1.154 +    if (status == PEP_STATUS_OK) {            
   1.155          stringlist_t* curr_key;
   1.156          
   1.157          for (curr_key = keys; curr_key && curr_key->value; curr_key = curr_key->next) {
   1.158 @@ -1269,13 +1275,16 @@
   1.159              else 
   1.160                  goto pEp_free;
   1.161              
   1.162 -            // Because of the key check above, this should not happen unless we were 
   1.163 -            // UNLESS we required a passphrase for keygen and it's not set.
   1.164 -            if (PASS_ERROR(status))
   1.165 -                goto pEp_free;
   1.166 -                
   1.167 -            // FIXME: what about other statuses, though???
   1.168 -                
   1.169 +            // This is in a switch because our return statuses COULD get more 
   1.170 +            // complicated
   1.171 +            switch (status) {
   1.172 +                case PEP_STATUS_OK:
   1.173 +                case PEP_KEY_NOT_FOUND: // call removed it as a default
   1.174 +                    break;
   1.175 +                default:
   1.176 +                    goto pEp_free;
   1.177 +            }
   1.178 +                            
   1.179              free_identity_list(key_idents);    
   1.180          }
   1.181      }
   1.182 @@ -1413,15 +1422,7 @@
   1.183          // case of own identities with private keys.
   1.184          
   1.185          if (is_own_private) {
   1.186 -            
   1.187 -            // Make sure we can even progress - if there are passphrase issues,
   1.188 -            // bounce back to the caller now.
   1.189 -            status = _check_own_reset_passphrase_readiness(session, fpr_copy);
   1.190 -
   1.191 -            // These will always be passphrase errors.
   1.192 -            if (status != PEP_STATUS_OK)
   1.193 -                return status;
   1.194 -            
   1.195 +                        
   1.196              // This is now the "is_own" base case - we don't do this 
   1.197              // per-identity, because all identities using this key will 
   1.198              // need new ones. That said, this is really only a problem 
   1.199 @@ -1443,6 +1444,14 @@
   1.200                  if (is_grouped) 
   1.201                      status = _key_reset_device_group_for_shared_key(session, key_idents, fpr_copy, false);
   1.202                  else if (status == PEP_STATUS_OK) {
   1.203 +                    // KB: FIXME_NOW - revoked?
   1.204 +                    // Make sure we can even progress - if there are passphrase issues,
   1.205 +                    // bounce back to the caller now, because our attempts to make it work failed,
   1.206 +                    // even possibly with callback.
   1.207 +                    status = _check_own_reset_passphrase_readiness(session, fpr_copy);
   1.208 +                    if (status != PEP_STATUS_OK)
   1.209 +                        return status;
   1.210 +                    
   1.211                      // now have ident list, or should
   1.212                      identity_list* curr_ident;
   1.213  
   1.214 @@ -1453,7 +1462,7 @@
   1.215                          // Do the full reset on this identity        
   1.216                          // Base case for is_own_private starts here
   1.217                          // Note that we reset this key for ANY own ident that has it. And if 
   1.218 -                        // tmp_ident did NOT have this key, it won't matter. We will reset the 
   1.219 +                        // tmp_ident did NOT have this key, it won't matter. We will reset this 
   1.220                          // key for all idents for this user.
   1.221                          status = revoke_key(session, fpr_copy, NULL);
   1.222                          
     2.1 --- a/src/keymanagement.c	Fri Jul 31 11:22:55 2020 +0200
     2.2 +++ b/src/keymanagement.c	Fri Jul 31 15:04:07 2020 +0200
     2.3 @@ -1358,7 +1358,8 @@
     2.4  {
     2.5      PEP_SESSION session;
     2.6      pEp_identity *identity;
     2.7 -    PEP_STATUS status = init(&session, NULL, NULL);
     2.8 +    // FIXME_NOW: ensure_decrypt callback???
     2.9 +    PEP_STATUS status = init(&session, NULL, NULL, NULL);
    2.10      assert(!status);
    2.11      if (status)
    2.12          return status;
     3.1 --- a/src/pEpEngine.c	Fri Jul 31 11:22:55 2020 +0200
     3.2 +++ b/src/pEpEngine.c	Fri Jul 31 15:04:07 2020 +0200
     3.3 @@ -947,7 +947,8 @@
     3.4  DYNAMIC_API PEP_STATUS init(
     3.5          PEP_SESSION *session,
     3.6          messageToSend_t messageToSend,
     3.7 -        inject_sync_event_t inject_sync_event
     3.8 +        inject_sync_event_t inject_sync_event,
     3.9 +        ensure_passphrase_t ensure_passphrase
    3.10      )
    3.11  {
    3.12      PEP_STATUS status = PEP_STATUS_OK;
    3.13 @@ -997,6 +998,7 @@
    3.14      _session->version = PEP_ENGINE_VERSION;
    3.15      _session->messageToSend = messageToSend;
    3.16      _session->inject_sync_event = inject_sync_event;
    3.17 +    _session->ensure_passphrase = ensure_passphrase;
    3.18  
    3.19  #ifdef DEBUG_ERRORSTACK
    3.20      _session->errorstack = new_stringlist("init()");
     4.1 --- a/src/pEpEngine.h	Fri Jul 31 11:22:55 2020 +0200
     4.2 +++ b/src/pEpEngine.h	Fri Jul 31 15:04:07 2020 +0200
     4.3 @@ -205,7 +205,7 @@
     4.4  
     4.5  typedef int (*inject_sync_event_t)(SYNC_EVENT ev, void *management);
     4.6  
     4.7 -// ensure_decrypt_key() - callee ensures correct password for (signing) key is configured in the session on
     4.8 +// ensure_passphrase() - callee ensures correct password for (signing) key is configured in the session on
     4.9  //                        return, or returns error when it is not found
    4.10  //  parameters:
    4.11  //.     session (in)      session for which the guarantee is made
    4.12 @@ -221,7 +221,7 @@
    4.13  //      The callee is responsible for iterating through passwords
    4.14  //      to ensure signing/encryption can occur successfully. 
    4.15  //
    4.16 -typedef PEP_STATUS (*ensure_decrypt_key_t)(PEP_SESSION session, const char* fpr);
    4.17 +typedef PEP_STATUS (*ensure_passphrase_t)(PEP_SESSION session, const char* fpr);
    4.18  
    4.19  // INIT_STATUS init() - initialize pEpEngine for a thread
    4.20  //
    4.21 @@ -231,7 +231,7 @@
    4.22  //      messageToSend (in)                  callback for sending message by the
    4.23  //                                          application
    4.24  //      inject_sync_event (in)              callback for injecting a sync event
    4.25 -//      ensure_decrypt_key (in)             callback for ensuring correct password for key is set
    4.26 +//      ensure_passphrase (in)             callback for ensuring correct password for key is set
    4.27  //
    4.28  //  return value:
    4.29  //      PEP_STATUS_OK = 0                   if init() succeeds
    4.30 @@ -266,7 +266,7 @@
    4.31          PEP_SESSION *session,
    4.32          messageToSend_t messageToSend,
    4.33          inject_sync_event_t inject_sync_event,
    4.34 -        ensure_decrypt_key_t ensure_decrypt_key
    4.35 +        ensure_passphrase_t ensure_passphrase
    4.36      );
    4.37  
    4.38  
     5.1 --- a/src/pEp_internal.h	Fri Jul 31 11:22:55 2020 +0200
     5.2 +++ b/src/pEp_internal.h	Fri Jul 31 15:04:07 2020 +0200
     5.3 @@ -250,7 +250,7 @@
     5.4      notifyHandshake_t notifyHandshake;
     5.5      inject_sync_event_t inject_sync_event;
     5.6      retrieve_next_sync_event_t retrieve_next_sync_event;
     5.7 -    ensure_decrypt_key_t ensure_decrypt_key;
     5.8 +    ensure_passphrase_t ensure_passphrase;
     5.9  
    5.10      // pEp Sync
    5.11      void *sync_management;
     6.1 --- a/test/src/Engine.cc	Fri Jul 31 11:22:55 2020 +0200
     6.2 +++ b/test/src/Engine.cc	Fri Jul 31 15:04:07 2020 +0200
     6.3 @@ -90,7 +90,7 @@
     6.4              
     6.5      unix_local_db(true);
     6.6              
     6.7 -    PEP_STATUS status = init(&session, cached_messageToSend, cached_inject_sync_event);
     6.8 +    PEP_STATUS status = init(&session, cached_messageToSend, cached_inject_sync_event, NULL);
     6.9      assert(status == PEP_STATUS_OK);
    6.10      assert(session);
    6.11  
     7.1 --- a/test/src/SyncTest.cc	Fri Jul 31 11:22:55 2020 +0200
     7.2 +++ b/test/src/SyncTest.cc	Fri Jul 31 15:04:07 2020 +0200
     7.3 @@ -246,7 +246,7 @@
     7.4                  
     7.5                  ST_fake_this = (void*)(&adapter);
     7.6  
     7.7 -                status = init(&sync, ST_message_send_callback, ST_inject_sync_event_callback);
     7.8 +                status = init(&sync, ST_message_send_callback, ST_inject_sync_event_callback, NULL);
     7.9                  if (status != PEP_STATUS_OK)
    7.10                      throw std::runtime_error((string("init returned ") + tl_status_string(status)).c_str());
    7.11