Fix issue with group creation on linked devices.

This commit is contained in:
Greyson Parrelli 2021-06-25 16:35:42 -04:00 committed by GitHub
parent 3d2a634aac
commit a71fe0fd75
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 96 additions and 28 deletions

View file

@ -22,6 +22,9 @@ import org.signal.zkgroup.InvalidInputException;
import org.signal.zkgroup.groups.GroupMasterKey; import org.signal.zkgroup.groups.GroupMasterKey;
import org.thoughtcrime.securesms.crypto.SenderKeyUtil; import org.thoughtcrime.securesms.crypto.SenderKeyUtil;
import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper; import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper;
import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
import org.thoughtcrime.securesms.groups.v2.processing.GroupsV2StateProcessor;
import org.thoughtcrime.securesms.jobs.RequestGroupV2InfoJob;
import org.whispersystems.signalservice.api.push.DistributionId; import org.whispersystems.signalservice.api.push.DistributionId;
import org.thoughtcrime.securesms.groups.GroupAccessControl; import org.thoughtcrime.securesms.groups.GroupAccessControl;
import org.thoughtcrime.securesms.groups.GroupId; import org.thoughtcrime.securesms.groups.GroupId;
@ -441,6 +444,47 @@ private static final String[] GROUP_PROJECTION = {
return groupId; return groupId;
} }
/**
* There was a point in time where we weren't properly responding to group creates on linked devices. This would result in us having a Recipient entry for the
* group, but we'd either be missing the group entry, or that entry would be missing a master key. This method fixes this scenario.
*/
public void fixMissingMasterKey(@NonNull GroupMasterKey groupMasterKey) {
GroupId.V2 groupId = GroupId.v2(groupMasterKey);
if (getGroupV1ByExpectedV2(groupId).isPresent()) {
throw new MissedGroupMigrationInsertException(groupId);
}
SQLiteDatabase db = databaseHelper.getWritableDatabase();
db.beginTransaction();
try {
String query = GROUP_ID + " = ?";
String[] args = SqlUtil.buildArgs(groupId);
ContentValues values = new ContentValues();
values.put(V2_MASTER_KEY, groupMasterKey.serialize());
int updated = db.update(TABLE_NAME, values, query, args);
if (updated < 1) {
Log.w(TAG, "No group entry. Creating restore placeholder for " + groupId);
create(groupMasterKey, DecryptedGroup.newBuilder()
.setRevision(GroupsV2StateProcessor.RESTORE_PLACEHOLDER_REVISION)
.build());
} else {
Log.w(TAG, "Had a group entry, but it was missing a master key. Updated.");
}
db.setTransactionSuccessful();
} finally {
db.endTransaction();
}
Log.w(TAG, "Scheduling request for latest group info for " + groupId);
ApplicationDependencies.getJobManager().add(new RequestGroupV2InfoJob(groupId));
}
/** /**
* @param groupMasterKey null for V1, must be non-null for V2 (presence dictates group version). * @param groupMasterKey null for V1, must be non-null for V2 (presence dictates group version).
*/ */

View file

@ -274,6 +274,8 @@ public class StorageSyncJob extends BaseJob {
try { try {
self = freshSelf(); self = freshSelf();
Log.i(TAG, "[Remote Sync] Remote-Only :: Contacts: " + remoteContacts.size() + ", GV1: " + remoteGv1.size() + ", GV2: " + remoteGv2.size() + ", Account: " + remoteAccount.size());
new ContactRecordProcessor(context, self).process(remoteContacts, StorageSyncHelper.KEY_GENERATOR); new ContactRecordProcessor(context, self).process(remoteContacts, StorageSyncHelper.KEY_GENERATOR);
new GroupV1RecordProcessor(context).process(remoteGv1, StorageSyncHelper.KEY_GENERATOR); new GroupV1RecordProcessor(context).process(remoteGv1, StorageSyncHelper.KEY_GENERATOR);
new GroupV2RecordProcessor(context).process(remoteGv2, StorageSyncHelper.KEY_GENERATOR); new GroupV2RecordProcessor(context).process(remoteGv2, StorageSyncHelper.KEY_GENERATOR);

View file

@ -155,7 +155,6 @@ import org.whispersystems.signalservice.api.messages.shared.SharedContact;
import org.whispersystems.signalservice.api.payments.Money; import org.whispersystems.signalservice.api.payments.Money;
import org.whispersystems.signalservice.api.push.DistributionId; import org.whispersystems.signalservice.api.push.DistributionId;
import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.api.push.SignalServiceAddress;
import org.whispersystems.signalservice.internal.push.SignalServiceProtos;
import java.io.IOException; import java.io.IOException;
import java.security.SecureRandom; import java.security.SecureRandom;
@ -243,21 +242,7 @@ public final class MessageContentProcessor {
boolean isGv2Message = groupId.isPresent() && groupId.get().isV2(); boolean isGv2Message = groupId.isPresent() && groupId.get().isV2();
if (isGv2Message) { if (isGv2Message) {
GroupId.V2 groupIdV2 = groupId.get().requireV2(); if (handleGv2PreProcessing(groupId.orNull().requireV2(), content, content.getDataMessage().get().getGroupContext().get().getGroupV2().get())) {
Optional<GroupRecord> possibleGv1 = groupDatabase.getGroupV1ByExpectedV2(groupIdV2);
if (possibleGv1.isPresent()) {
GroupsV1MigrationUtil.performLocalMigration(context, possibleGv1.get().getId().requireV1());
}
if (!updateGv2GroupFromServerOrP2PChange(content, message.getGroupContext().get().getGroupV2().get())) {
log(String.valueOf(content.getTimestamp()), "Ignoring GV2 message for group we are not currently in " + groupIdV2);
return;
}
Recipient sender = Recipient.externalPush(context, content.getSender());
if (!groupDatabase.isCurrentMember(groupIdV2, sender.getId())) {
log(String.valueOf(content.getTimestamp()), "Ignoring GV2 message from member not in group " + groupIdV2);
return; return;
} }
} }
@ -395,8 +380,35 @@ public final class MessageContentProcessor {
.enqueue(); .enqueue();
} }
private static @Nullable /**
SignalServiceGroupContext getGroupContextIfPresent(@NonNull SignalServiceContent content) { * @return True if the content should be ignored, otherwise false.
*/
private boolean handleGv2PreProcessing(GroupId.V2 groupId, SignalServiceContent content, SignalServiceGroupV2 groupV2)
throws IOException, GroupChangeBusyException
{
GroupDatabase groupDatabase = DatabaseFactory.getGroupDatabase(context);
Optional<GroupRecord> possibleGv1 = groupDatabase.getGroupV1ByExpectedV2(groupId);
if (possibleGv1.isPresent()) {
GroupsV1MigrationUtil.performLocalMigration(context, possibleGv1.get().getId().requireV1());
}
if (!updateGv2GroupFromServerOrP2PChange(content, groupV2)) {
log(String.valueOf(content.getTimestamp()), "Ignoring GV2 message for group we are not currently in " + groupId);
return true;
}
Recipient sender = Recipient.externalPush(context, content.getSender());
if (!groupDatabase.isCurrentMember(groupId, sender.getId())) {
log(String.valueOf(content.getTimestamp()), "Ignoring GV2 message from member not in group " + groupId);
return true;
}
return false;
}
private static @Nullable SignalServiceGroupContext getGroupContextIfPresent(@NonNull SignalServiceContent content) {
if (content.getDataMessage().isPresent() && content.getDataMessage().get().getGroupContext().isPresent()) { if (content.getDataMessage().isPresent() && content.getDataMessage().get().getGroupContext().isPresent()) {
return content.getDataMessage().get().getGroupContext().get(); return content.getDataMessage().get().getGroupContext().get();
} else if (content.getSyncMessage().isPresent() && } else if (content.getSyncMessage().isPresent() &&
@ -678,10 +690,13 @@ public final class MessageContentProcessor {
if (groupV1.getType() != SignalServiceGroup.Type.REQUEST_INFO) { if (groupV1.getType() != SignalServiceGroup.Type.REQUEST_INFO) {
ApplicationDependencies.getJobManager().add(new RequestGroupInfoJob(Recipient.externalHighTrustPush(context, content.getSender()).getId(), GroupId.v1(groupV1.getGroupId()))); ApplicationDependencies.getJobManager().add(new RequestGroupInfoJob(Recipient.externalHighTrustPush(context, content.getSender()).getId(), GroupId.v1(groupV1.getGroupId())));
} else { } else {
warn(String.valueOf(content.getTimestamp()), "Received a REQUEST_INFO message for a group we don't know about. Ignoring."); warn(content.getTimestamp(), "Received a REQUEST_INFO message for a group we don't know about. Ignoring.");
} }
} else if (group.getGroupV2().isPresent()) {
warn(content.getTimestamp(), "Received a GV2 message for a group we have no knowledge of -- attempting to fix this state.");
DatabaseFactory.getGroupDatabase(context).fixMissingMasterKey(group.getGroupV2().get().getMasterKey());
} else { } else {
warn(String.valueOf(content.getTimestamp()), "Received a message for a group we don't know about without a GV1 context. Ignoring."); warn(content.getTimestamp(), "Received a message for a group we don't know about without a group context. Ignoring.");
} }
} }
@ -870,7 +885,7 @@ public final class MessageContentProcessor {
StorageSyncHelper.scheduleSyncForDataChange(); StorageSyncHelper.scheduleSyncForDataChange();
break; break;
default: default:
Log.w(TAG, "Received a fetch message for an unknown type."); warn(TAG, "Received a fetch message for an unknown type.");
} }
} }
@ -929,7 +944,7 @@ public final class MessageContentProcessor {
Optional<MobileCoinPublicAddress> address = outgoingPaymentMessage.getAddress().transform(MobileCoinPublicAddress::fromBytes); Optional<MobileCoinPublicAddress> address = outgoingPaymentMessage.getAddress().transform(MobileCoinPublicAddress::fromBytes);
if (!address.isPresent() && recipientId == null) { if (!address.isPresent() && recipientId == null) {
Log.i(TAG, "Inserting defrag"); log("Inserting defrag");
address = Optional.of(ApplicationDependencies.getPayments().getWallet().getMobileCoinPublicAddress()); address = Optional.of(ApplicationDependencies.getPayments().getWallet().getMobileCoinPublicAddress());
recipientId = Recipient.self().getId(); recipientId = Recipient.self().getId();
} }
@ -960,9 +975,9 @@ public final class MessageContentProcessor {
GroupDatabase groupDatabase = DatabaseFactory.getGroupDatabase(context); GroupDatabase groupDatabase = DatabaseFactory.getGroupDatabase(context);
if (message.getMessage().isGroupV2Message()) { if (message.getMessage().isGroupV2Message()) {
Optional<GroupRecord> possibleGv1 = groupDatabase.getGroupV1ByExpectedV2(GroupId.v2(message.getMessage().getGroupContext().get().getGroupV2().get().getMasterKey())); GroupId.V2 groupId = GroupId.v2(message.getMessage().getGroupContext().get().getGroupV2().get().getMasterKey());
if (possibleGv1.isPresent()) { if (handleGv2PreProcessing(groupId, content, message.getMessage().getGroupContext().get().getGroupV2().get())) {
GroupsV1MigrationUtil.performLocalMigration(context, possibleGv1.get().getId().requireV1()); return;
} }
} }
@ -981,7 +996,7 @@ public final class MessageContentProcessor {
} else if (Build.VERSION.SDK_INT > 19 && message.getMessage().getGroupCallUpdate().isPresent()) { } else if (Build.VERSION.SDK_INT > 19 && message.getMessage().getGroupCallUpdate().isPresent()) {
handleGroupCallUpdateMessage(content, message.getMessage(), GroupUtil.idFromGroupContext(message.getMessage().getGroupContext())); handleGroupCallUpdateMessage(content, message.getMessage(), GroupUtil.idFromGroupContext(message.getMessage().getGroupContext()));
} else if (message.getMessage().isEmptyGroupV2Message()) { } else if (message.getMessage().isEmptyGroupV2Message()) {
// Do nothing warn(content.getTimestamp(), "Empty GV2 message! Doing nothing.");
} else if (message.getMessage().isExpirationUpdate()) { } else if (message.getMessage().isExpirationUpdate()) {
threadId = handleSynchronizeSentExpirationUpdate(message); threadId = handleSynchronizeSentExpirationUpdate(message);
} else if (message.getMessage().getReaction().isPresent()) { } else if (message.getMessage().getReaction().isPresent()) {
@ -1013,7 +1028,7 @@ public final class MessageContentProcessor {
} }
if (SignalStore.rateLimit().needsRecaptcha()) { if (SignalStore.rateLimit().needsRecaptcha()) {
Log.i(TAG, "Got a sent transcript while in reCAPTCHA mode. Assuming we're good to message again."); log(content.getTimestamp(), "Got a sent transcript while in reCAPTCHA mode. Assuming we're good to message again.");
RateLimitUtil.retryAllRateLimitedMessages(context); RateLimitUtil.retryAllRateLimitedMessages(context);
} }
@ -2150,6 +2165,10 @@ public final class MessageContentProcessor {
Log.i(TAG, message); Log.i(TAG, message);
} }
protected void log(long timestamp, @NonNull String message) {
log(String.valueOf(timestamp), message);
}
protected void log(@NonNull String extra, @NonNull String message) { protected void log(@NonNull String extra, @NonNull String message) {
String extraLog = Util.isEmpty(extra) ? "" : "[" + extra + "] "; String extraLog = Util.isEmpty(extra) ? "" : "[" + extra + "] ";
Log.i(TAG, extraLog + message); Log.i(TAG, extraLog + message);

View file

@ -26,6 +26,7 @@ public final class GroupV2RecordProcessor extends DefaultStorageRecordProcessor<
private final Context context; private final Context context;
private final RecipientDatabase recipientDatabase; private final RecipientDatabase recipientDatabase;
private final GroupDatabase groupDatabase;
private final Map<GroupId.V2, GroupId.V1> gv1GroupsByExpectedGv2Id; private final Map<GroupId.V2, GroupId.V1> gv1GroupsByExpectedGv2Id;
public GroupV2RecordProcessor(@NonNull Context context) { public GroupV2RecordProcessor(@NonNull Context context) {
@ -35,6 +36,7 @@ public final class GroupV2RecordProcessor extends DefaultStorageRecordProcessor<
GroupV2RecordProcessor(@NonNull Context context, @NonNull RecipientDatabase recipientDatabase, @NonNull GroupDatabase groupDatabase) { GroupV2RecordProcessor(@NonNull Context context, @NonNull RecipientDatabase recipientDatabase, @NonNull GroupDatabase groupDatabase) {
this.context = context; this.context = context;
this.recipientDatabase = recipientDatabase; this.recipientDatabase = recipientDatabase;
this.groupDatabase = groupDatabase;
this.gv1GroupsByExpectedGv2Id = groupDatabase.getAllExpectedV2Ids(); this.gv1GroupsByExpectedGv2Id = groupDatabase.getAllExpectedV2Ids();
} }
@ -54,7 +56,8 @@ public final class GroupV2RecordProcessor extends DefaultStorageRecordProcessor<
if (settings.getSyncExtras().getGroupMasterKey() != null) { if (settings.getSyncExtras().getGroupMasterKey() != null) {
return StorageSyncModels.localToRemoteRecord(settings); return StorageSyncModels.localToRemoteRecord(settings);
} else { } else {
Log.w(TAG, "No local master key. Assuming it matches remote since the groupIds match."); Log.w(TAG, "No local master key. Assuming it matches remote since the groupIds match. Enqueuing a fetch to fix the bad state.");
groupDatabase.fixMissingMasterKey(record.getMasterKeyOrThrow());
return StorageSyncModels.localToRemoteRecord(settings, record.getMasterKeyOrThrow()); return StorageSyncModels.localToRemoteRecord(settings, record.getMasterKeyOrThrow());
} }
}) })