Merge branch 'fix/CHT-452-update-keyxid-globally' into 'develop'

CHT-452. Update the keyxid globally, rather than per chat

Closes CHT-452

See merge request megachat/MEGAchat!1290

src/chatd.cpp

@@ -3570,8 +3570,8 @@ bool Chat::msgEncryptAndSend(OutputQueue::iterator it)
  MsgCommand *msgCmd = pms.value().first;
  KeyCommand *keyCmd = pms.value().second;
  assert(!keyCmd // no newkey required...
- || (keyCmd && keyCmd->localKeyid() == msg->keyid // ... or localkeyid is assigned to message
- && msgCmd->keyId() == CHATD_KEYID_UNCONFIRMED)); // and msgCmd's keyid is unconfirmed
+ || (keyCmd->localKeyid() == msg->keyid  // ... or localkeyid is assigned to message
+ && isValidKeyxId(msgCmd->keyId()))); // and msgCmd's keyid is within the range of valid keyxid's
 
  it->msgCmd = pms.value().first;
  it->keyCmd = pms.value().second;
@@ -3600,9 +3600,9 @@ bool Chat::msgEncryptAndSend(OutputQueue::iterator it)
  }
  else
  {
- assert(keyCmd);
- assert(keyCmd->localKeyid() == msg->keyid);
- assert(msgCmd->keyId() == CHATD_KEYID_UNCONFIRMED);
+ assert(keyCmd // key command required
+  && keyCmd->localKeyid() == msg->keyid // and localkeyid is assigned to message
+  && isValidKeyxId(msgCmd->keyId())); // and msgCmd's keyid is within the range of valid keyxid's
  }
 
  SendingItem &item = mSending.front();
@@ -4320,9 +4320,9 @@ Idx Chat::msgConfirm(Id msgxid, Id msgid, uint32_t timestamp)
 
 void Chat::keyConfirm(KeyId keyxid, KeyId keyid)
 {
- if (keyxid != CHATD_KEYID_UNCONFIRMED)
+ if (!isValidKeyxId(keyxid))
  {
- CHATID_LOG_ERROR("keyConfirm: Key transaction id != 0xfffffffe");
+ CHATID_LOG_ERROR("keyConfirm: Key transaction id [%s] is out of range [0xffff0000, 0xfffffffe)", ID_CSTR(keyxid));
  return;
  }
 

src/chatdMsg.h

@@ -10,13 +10,18 @@
 enum
 {
  CHATD_KEYID_INVALID = 0, // used when no keyid is set
- CHATD_KEYID_UNCONFIRMED = 0xfffffffe, // used when a new keyid has been requested. Should be kept as constant as possible and in the range of 0xffff0001 to 0xffffffff
  CHATD_KEYID_MAX = 0xffffffff, // higher keyid allowed for unconfirmed new keys
  CHATD_KEYID_MIN = 0xffff0000 // lower keyid allowed for unconfirmed new keys
 };
 
 namespace chatd
 {
+typedef uint64_t BackRefId;
+typedef uint32_t KeyId;
+inline bool isValidKeyxId(KeyId keyxid)
+{
+ return keyxid <= CHATD_KEYID_MAX && keyxid >= CHATD_KEYID_MIN;
+}
 
 enum CallDataReason
 {
@@ -33,8 +38,7 @@ enum
  kTsMissingCallUnread = 1592222400, // This ts enable missing call as unread message from 15th June of 2020
 };
 
-typedef uint32_t KeyId;
-typedef uint64_t BackRefId;
+
 
 static bool isLocalKeyId(KeyId localKeyid)
 {
@@ -1131,8 +1135,7 @@ class Command: public Buffer
  * userid.8 + keylen.2 + key.keylen
  *
  * Additionally, the KeyCommand stores the given local keyid, which is used
- * internally. The keyid encoded in the command is always hardwire to the
- * constant value CHATD_KEYID_UNCONFIRMED.
+ * internally.
  */
 class KeyCommand: public Command
 {
@@ -1143,10 +1146,8 @@ class KeyCommand: public Command
  explicit KeyCommand(karere::Id chatid, KeyId aLocalkeyid, size_t reserve=128)
  : Command(OP_NEWKEY, reserve), mLocalKeyid(aLocalkeyid)
  {
- assert(isLocalKeyId(mLocalKeyid));
-
- KeyId keyid = CHATD_KEYID_UNCONFIRMED;
- append(chatid.val).append<KeyId>(keyid).append<uint32_t>(0); //last is length of keys payload, initially empty
+ assert(isValidKeyxId(mLocalKeyid) || mLocalKeyid == CHATD_KEYID_INVALID);
+ append(chatid.val).append<KeyId>(mLocalKeyid).append<uint32_t>(0); //last is length of keys payload, initially empty
  }
 
  KeyId localKeyid() const { return mLocalKeyid; }

src/strongvelope/strongvelope.cpp

@@ -44,6 +44,8 @@ struct Context
  EcKey edKey;
 };
 
+chatd::KeyId ProtocolHandler::mCurrentLocalKeyId = CHATD_KEYID_MAX;
+
 const std::string SVCRYPTO_PAIRWISE_KEY = "strongvelope pairwise key\x01";
 const std::string SVCRYPTO_SIG = "strongvelopesig";
 void deriveNonceSecret(const StaticBuffer& masterNonce, const StaticBuffer &result,
@@ -752,7 +754,7 @@ void ProtocolHandler::loadUnconfirmedKeysFromDb()
  stmt.blobCol(1, keyBlobs);
 
  // read keyid
- KeyId keyid = (KeyId)stmt.intCol(2);
+ KeyId keyid = static_cast<KeyId>(stmt.intCol(2));
  assert(isLocalKeyId(keyid));
 
  //pick the version that is encrypted for us
@@ -795,12 +797,12 @@ void ProtocolHandler::loadUnconfirmedKeysFromDb()
  mCurrentKeyId = keyid;
  mCurrentKeyParticipants = recipients;
  mCurrentLocalKeyId = keyid;
-
  NewKeyEntry entry(key, recipients, keyid);
  mUnconfirmedKeys.push_back(entry);
  });
  }
-
+ // we have retrieved min LocalKeyxId (transactional keyid) from sending table, we need to obtain next valid value (decrement)
+ getNextValidLocalKeyId();
  STRONGVELOPE_LOG_DEBUG("(%" PRId64 "): Loaded %zu unconfirmed keys from database", chatid.val, mUnconfirmedKeys.size());
 }
 
@@ -988,7 +990,7 @@ ProtocolHandler::msgEncrypt(Message* msg, const SetOfIds &recipients, MsgCommand
  if (mCurrentKey && mCurrentKeyParticipants == recipients)
  {
  msg->keyid = mCurrentKeyId;
- msgCmd->setKeyId(isLocalKeyId(mCurrentKeyId) ? CHATD_KEYID_UNCONFIRMED : mCurrentKeyId);
+ msgCmd->setKeyId(mCurrentKeyId);
  msgEncryptWithKey(*msg, *msgCmd, *mCurrentKey);
  return std::make_pair(msgCmd, (KeyCommand*)nullptr);
  }
@@ -999,7 +1001,7 @@ ProtocolHandler::msgEncrypt(Message* msg, const SetOfIds &recipients, MsgCommand
  {
  wptr.throwIfDeleted();
  msg->keyid = result.first->localKeyid();
- msgCmd->setKeyId(CHATD_KEYID_UNCONFIRMED);
+ msgCmd->setKeyId(mCurrentKeyId);
  msgEncryptWithKey(*msg, *msgCmd, *result.second);
  return std::make_pair(msgCmd, result.first);
  });
@@ -1018,7 +1020,8 @@ ProtocolHandler::msgEncrypt(Message* msg, const SetOfIds &recipients, MsgCommand
  NewKeyEntry entry = it;
  if (entry.recipients == recipients && entry.localKeyid == msg->keyid)
  {
- msgCmd->setKeyId(CHATD_KEYID_UNCONFIRMED);
+ assert(isValidKeyxId(msg->keyid));
+ msgCmd->setKeyId(msg->keyid);
  msgEncryptWithKey(*msg, *msgCmd, *entry.key);
  return std::make_pair(msgCmd, (KeyCommand*)nullptr);
  }
@@ -1436,10 +1439,7 @@ ProtocolHandler::createNewKey(const SetOfIds &recipients)
 
 KeyId ProtocolHandler::createLocalKeyId()
 {
- if (--mCurrentLocalKeyId < CHATD_KEYID_MIN)
- mCurrentLocalKeyId = CHATD_KEYID_MAX;
-
- return mCurrentLocalKeyId;
+ return getNextValidLocalKeyId();
 }
 
 promise::Promise<std::pair<KeyCommand*, std::shared_ptr<SendKey>>>
@@ -1502,7 +1502,8 @@ ProtocolHandler::encryptChatTitle(const std::string& data, uint64_t extraUser, b
  wptr.throwIfDeleted();
  assert(!key->empty());
 
- return encryptKeyToAllParticipants(key, participants)
+ // we provide invalid KeyId as it's not used upon chat title encryption
+ return encryptKeyToAllParticipants(key, participants, CHATD_KEYID_INVALID)
  .then([this, wptr, data, createNewKey](const std::pair<chatd::KeyCommand*, std::shared_ptr<SendKey>>& result)
  {
  wptr.throwIfDeleted();
@@ -1558,7 +1559,7 @@ ProtocolHandler::encryptUnifiedKeyForAllParticipants(uint64_t extraUser)
  wptr.throwIfDeleted();
  assert(!key->empty());
 
- return encryptKeyToAllParticipants(key, participants)
+ return encryptKeyToAllParticipants(key, participants, CHATD_KEYID_INVALID)
  .then([this, wptr](const std::pair<KeyCommand*, std::shared_ptr<SendKey>> result)
  {
  wptr.throwIfDeleted();
@@ -1739,6 +1740,16 @@ ProtocolHandler::NewKeyEntry::NewKeyEntry(const std::shared_ptr<SendKey> &aKey,
 
 }
 
+KeyId ProtocolHandler::getNextValidLocalKeyId()
+{
+ chatd::KeyId ret = mCurrentLocalKeyId;
+ if (!isValidKeyxId(--mCurrentLocalKeyId))
+ {
+ mCurrentLocalKeyId = CHATD_KEYID_MAX;
+ }
+ return ret;
+}
+
 } //end strongvelope namespace
 
 namespace chatd

src/strongvelope/strongvelope.h

@@ -285,7 +285,8 @@ class ProtocolHandler: public chatd::ICrypto, public karere::DeleteTrackable
  std::shared_ptr<SendKey> mCurrentKey;
  chatd::KeyId mCurrentKeyId = CHATD_KEYID_INVALID;
  karere::SetOfIds mCurrentKeyParticipants;
- chatd::KeyId mCurrentLocalKeyId = CHATD_KEYID_MAX;
+ // Shared transactional keyxid among all the chats in the same chatd-shard
+ static chatd::KeyId mCurrentLocalKeyId;
 
  /**
  * @brief The NewKeyEntry struct represents a Key in the list of unconfirmed keys (mUnconfirmedKeys)
@@ -382,7 +383,7 @@ class ProtocolHandler: public chatd::ICrypto, public karere::DeleteTrackable
  encryptKeyTo(const std::shared_ptr<SendKey>& sendKey, karere::Id toUser);
 
  promise::Promise<std::pair<chatd::KeyCommand*, std::shared_ptr<SendKey>>>
- encryptKeyToAllParticipants(const std::shared_ptr<SendKey>& key, const karere::SetOfIds &participants, chatd::KeyId localkeyid = CHATD_KEYID_UNCONFIRMED);
+ encryptKeyToAllParticipants(const std::shared_ptr<SendKey>& key, const karere::SetOfIds &participants, chatd::KeyId localkeyid);
 
  promise::Promise<std::string> encryptUnifiedKeyToUser(karere::Id user) override;
 
@@ -391,6 +392,13 @@ class ProtocolHandler: public chatd::ICrypto, public karere::DeleteTrackable
  promise::Promise<chatd::Message*> handleManagementMessage(
  const std::shared_ptr<ParsedMessage>& parsedMsg, chatd::Message* msg);
 
+ /**
+ * @brief Getter method to local static variable for temporal.
+ *
+ * Id will be decreasing as used and start from CHATD_KEYID_MAX when it reaches CHATD_KEYID_MIN.
+ */
+ chatd::KeyId getNextValidLocalKeyId();
+
 public:
 //chatd::ICrypto interface
  promise::Promise<std::pair<chatd::MsgCommand*, chatd::KeyCommand*>>