From 6080e1f338aa3bf3a872df4be6d526f02683c14b Mon Sep 17 00:00:00 2001 From: Alan Evans Date: Tue, 5 Jan 2021 17:42:27 -0400 Subject: [PATCH] Ensure ProfileKeyCredentials match ProfileKey. Fixes #10344 --- .../securesms/crypto/ProfileKeyUtil.java | 4 -- .../securesms/database/RecipientDatabase.java | 54 ++++++++++++------- .../database/helpers/SQLCipherOpenHelper.java | 12 ++++- .../securesms/groups/GroupManagerV2.java | 6 ++- .../groups/v2/GroupCandidateHelper.java | 13 +++-- .../jobs/GroupV2UpdateSelfProfileKeyJob.java | 2 +- .../securesms/recipients/Recipient.java | 5 +- .../recipients/RecipientDetails.java | 3 +- app/src/main/proto/Database.proto | 5 ++ 9 files changed, 73 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/crypto/ProfileKeyUtil.java b/app/src/main/java/org/thoughtcrime/securesms/crypto/ProfileKeyUtil.java index 886aac4fff..688d1a481f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/crypto/ProfileKeyUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/crypto/ProfileKeyUtil.java @@ -82,10 +82,6 @@ public final class ProfileKeyUtil { return Optional.of(profileKeyOrThrow(profileKey)); } - public static @NonNull Optional profileKeyCredentialOptional(@Nullable byte[] profileKey) { - return Optional.fromNullable(profileKeyCredentialOrNull(profileKey)); - } - public static @NonNull ProfileKey createNew() { try { return new ProfileKey(Util.getSecretBytes(32)); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java index d8bed10ed4..07878be888 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java @@ -10,11 +10,14 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.annimon.stream.Stream; +import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; import net.sqlcipher.database.SQLiteConstraintException; import org.signal.core.util.logging.Log; import org.signal.storageservice.protos.groups.local.DecryptedGroup; +import org.signal.zkgroup.InvalidInputException; import org.signal.zkgroup.groups.GroupMasterKey; import org.signal.zkgroup.profiles.ProfileKey; import org.signal.zkgroup.profiles.ProfileKeyCredential; @@ -25,6 +28,7 @@ import org.thoughtcrime.securesms.database.IdentityDatabase.IdentityRecord; import org.thoughtcrime.securesms.database.IdentityDatabase.VerifiedStatus; import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper; import org.thoughtcrime.securesms.database.model.ThreadRecord; +import org.thoughtcrime.securesms.database.model.databaseprotos.ProfileKeyCredentialColumnData; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.groups.GroupId; import org.thoughtcrime.securesms.groups.v2.ProfileKeySet; @@ -1257,9 +1261,9 @@ public class RecipientDatabase extends Database { String storageKeyRaw = CursorUtil.requireString(cursor, STORAGE_SERVICE_ID); int mentionSettingId = CursorUtil.requireInt(cursor, MENTION_SETTING); - MaterialColor color; - byte[] profileKey = null; - byte[] profileKeyCredential = null; + MaterialColor color; + byte[] profileKey = null; + ProfileKeyCredential profileKeyCredential = null; try { color = serializedColor == null ? null : MaterialColor.fromSerialized(serializedColor); @@ -1278,10 +1282,17 @@ public class RecipientDatabase extends Database { if (profileKeyCredentialString != null) { try { - profileKeyCredential = Base64.decode(profileKeyCredentialString); - } catch (IOException e) { - Log.w(TAG, e); - profileKeyCredential = null; + byte[] columnDataBytes = Base64.decode(profileKeyCredentialString); + + ProfileKeyCredentialColumnData columnData = ProfileKeyCredentialColumnData.parseFrom(columnDataBytes); + + if (Arrays.equals(columnData.getProfileKey().toByteArray(), profileKey)) { + profileKeyCredential = new ProfileKeyCredential(columnData.getProfileKeyCredential().toByteArray()); + } else { + Log.i(TAG, "Out of date profile key credential data ignored on read"); + } + } catch (InvalidInputException | IOException e) { + Log.w(TAG, "Profile key credential column data could not be read", e); } } } @@ -1581,23 +1592,30 @@ public class RecipientDatabase extends Database { /** * Updates the profile key credential as long as the profile key matches. */ - public void setProfileKeyCredential(@NonNull RecipientId id, - @NonNull ProfileKey profileKey, - @NonNull ProfileKeyCredential profileKeyCredential) + public boolean setProfileKeyCredential(@NonNull RecipientId id, + @NonNull ProfileKey profileKey, + @NonNull ProfileKeyCredential profileKeyCredential) { String selection = ID + " = ? AND " + PROFILE_KEY + " = ?"; String[] args = new String[]{id.serialize(), Base64.encodeBytes(profileKey.serialize())}; ContentValues values = new ContentValues(1); - values.put(PROFILE_KEY_CREDENTIAL, Base64.encodeBytes(profileKeyCredential.serialize())); + ProfileKeyCredentialColumnData columnData = ProfileKeyCredentialColumnData.newBuilder() + .setProfileKey(ByteString.copyFrom(profileKey.serialize())) + .setProfileKeyCredential(ByteString.copyFrom(profileKeyCredential.serialize())) + .build(); + + values.put(PROFILE_KEY_CREDENTIAL, Base64.encodeBytes(columnData.toByteArray())); SqlUtil.Query updateQuery = SqlUtil.buildTrueUpdateQuery(selection, args, values); - if (update(updateQuery, values)) { - // TODO [greyson] If we sync this in future, mark dirty - //markDirty(id, DirtyState.UPDATE); + boolean updated = update(updateQuery, values); + + if (updated) { Recipient.live(id).refresh(); } + + return updated; } private void clearProfileKeyCredential(@NonNull RecipientId id) { @@ -2596,7 +2614,7 @@ public class RecipientDatabase extends Database { private static void updateProfileValuesForMerge(@NonNull ContentValues values, @NonNull RecipientSettings settings) { values.put(PROFILE_KEY, settings.getProfileKey() != null ? Base64.encodeBytes(settings.getProfileKey()) : null); - values.put(PROFILE_KEY_CREDENTIAL, settings.getProfileKeyCredential() != null ? Base64.encodeBytes(settings.getProfileKeyCredential()) : null); + values.putNull(PROFILE_KEY_CREDENTIAL); values.put(SIGNAL_PROFILE_AVATAR, settings.getProfileAvatar()); values.put(PROFILE_GIVEN_NAME, settings.getProfileName().getGivenName()); values.put(PROFILE_FAMILY_NAME, settings.getProfileName().getFamilyName()); @@ -2718,7 +2736,7 @@ public class RecipientDatabase extends Database { private final int expireMessages; private final RegisteredState registered; private final byte[] profileKey; - private final byte[] profileKeyCredential; + private final ProfileKeyCredential profileKeyCredential; private final String systemDisplayName; private final String systemContactPhoto; private final String systemPhoneLabel; @@ -2757,7 +2775,7 @@ public class RecipientDatabase extends Database { int expireMessages, @NonNull RegisteredState registered, @Nullable byte[] profileKey, - @Nullable byte[] profileKeyCredential, + @Nullable ProfileKeyCredential profileKeyCredential, @Nullable String systemDisplayName, @Nullable String systemContactPhoto, @Nullable String systemPhoneLabel, @@ -2892,7 +2910,7 @@ public class RecipientDatabase extends Database { return profileKey; } - public @Nullable byte[] getProfileKeyCredential() { + public @Nullable ProfileKeyCredential getProfileKeyCredential() { return profileKeyCredential; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java index b231b2a44a..7ad1b0cf9c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java @@ -167,8 +167,9 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper implements SignalDatab private static final int VIEWED_RECEIPTS = 83; private static final int CLEAN_UP_GV1_IDS = 84; private static final int GV1_MIGRATION_REFACTOR = 85; + private static final int CLEAR_PROFILE_KEY_CREDENTIALS = 86; - private static final int DATABASE_VERSION = 85; + private static final int DATABASE_VERSION = 86; private static final String DATABASE_NAME = "signal.db"; private final Context context; @@ -1232,6 +1233,15 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper implements SignalDatab Log.i(TAG, "Cleared former_v1_members for " + count + " rows"); } + if (oldVersion < CLEAR_PROFILE_KEY_CREDENTIALS) { + ContentValues values = new ContentValues(1); + values.putNull("profile_key_credential"); + + int count = db.update("recipient", values, "profile_key_credential NOT NULL", null); + + Log.i(TAG, "Cleared profile key credentials for " + count + " rows"); + } + db.setTransactionSuccessful(); } finally { db.endTransaction(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java index 24e9067bd2..0e174ab12c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java @@ -39,6 +39,7 @@ import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.groups.v2.GroupCandidateHelper; import org.thoughtcrime.securesms.groups.v2.GroupLinkPassword; import org.thoughtcrime.securesms.groups.v2.processing.GroupsV2StateProcessor; +import org.thoughtcrime.securesms.jobs.ProfileUploadJob; import org.thoughtcrime.securesms.jobs.PushGroupSilentUpdateSendJob; import org.thoughtcrime.securesms.jobs.RequestGroupV2InfoJob; import org.thoughtcrime.securesms.keyvalue.SignalStore; @@ -461,12 +462,15 @@ final class GroupManagerV2 { if (Arrays.equals(profileKey.serialize(), selfInGroup.get().getProfileKey().toByteArray())) { Log.i(TAG, "Own Profile Key is already up to date in group " + groupId); return null; + } else { + Log.i(TAG, "Profile Key does not match that in group " + groupId); } GroupCandidate groupCandidate = groupCandidateHelper.recipientIdToCandidate(Recipient.self().getId()); if (!groupCandidate.hasProfileKeyCredential()) { - Log.w(TAG, "No credential available"); + Log.w(TAG, "No credential available, repairing"); + ApplicationDependencies.getJobManager().add(new ProfileUploadJob()); return null; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/GroupCandidateHelper.java b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/GroupCandidateHelper.java index 812a2ac7f5..f4f5d65542 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/GroupCandidateHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/GroupCandidateHelper.java @@ -51,19 +51,26 @@ public final class GroupCandidateHelper { throw new AssertionError("Non UUID members should have need detected by now"); } - Optional profileKeyCredential = ProfileKeyUtil.profileKeyCredentialOptional(recipient.getProfileKeyCredential()); + Optional profileKeyCredential = Optional.fromNullable(recipient.getProfileKeyCredential()); GroupCandidate candidate = new GroupCandidate(uuid, profileKeyCredential); if (!candidate.hasProfileKeyCredential()) { ProfileKey profileKey = ProfileKeyUtil.profileKeyOrNull(recipient.getProfileKey()); if (profileKey != null) { + Log.i(TAG, String.format("No profile key credential on recipient %s, fetching", recipient.getId())); + Optional profileKeyCredentialOptional = signalServiceAccountManager.resolveProfileKeyCredential(uuid, profileKey); if (profileKeyCredentialOptional.isPresent()) { - candidate = candidate.withProfileKeyCredential(profileKeyCredentialOptional.get()); + boolean updatedProfileKey = recipientDatabase.setProfileKeyCredential(recipient.getId(), profileKey, profileKeyCredentialOptional.get()); - recipientDatabase.setProfileKeyCredential(recipient.getId(), profileKey, profileKeyCredentialOptional.get()); + if (!updatedProfileKey) { + Log.w(TAG, String.format("Failed to update the profile key credential on recipient %s", recipient.getId())); + } else { + Log.i(TAG, String.format("Got new profile key credential for recipient %s", recipient.getId())); + candidate = candidate.withProfileKeyCredential(profileKeyCredentialOptional.get()); + } } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV2UpdateSelfProfileKeyJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV2UpdateSelfProfileKeyJob.java index 648e09ce54..ce82539516 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV2UpdateSelfProfileKeyJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV2UpdateSelfProfileKeyJob.java @@ -66,7 +66,7 @@ public final class GroupV2UpdateSelfProfileKeyJob extends BaseJob { public void onRun() throws IOException, GroupNotAMemberException, GroupChangeFailedException, GroupInsufficientRightsException, GroupChangeBusyException { - Log.i(TAG, "Updating profile key on group " + groupId); + Log.i(TAG, "Ensuring profile key up to date on group " + groupId); GroupManager.updateSelfProfileKeyInGroup(context, groupId); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java index 4ad934c918..2606296d44 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java @@ -14,6 +14,7 @@ import com.annimon.stream.Stream; import org.signal.core.util.concurrent.SignalExecutors; import org.signal.core.util.logging.Log; +import org.signal.zkgroup.profiles.ProfileKeyCredential; import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.color.MaterialColor; import org.thoughtcrime.securesms.contacts.avatars.ContactColors; @@ -84,7 +85,7 @@ public class Recipient { private final int expireMessages; private final RegisteredState registered; private final byte[] profileKey; - private final byte[] profileKeyCredential; + private final ProfileKeyCredential profileKeyCredential; private final String name; private final Uri systemContactPhoto; private final String customLabel; @@ -813,7 +814,7 @@ public class Recipient { return profileKey; } - public @Nullable byte[] getProfileKeyCredential() { + public @Nullable ProfileKeyCredential getProfileKeyCredential() { return profileKeyCredential; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientDetails.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientDetails.java index faf4c6a82c..ae26408e61 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientDetails.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientDetails.java @@ -7,6 +7,7 @@ import android.text.TextUtils; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import org.signal.zkgroup.profiles.ProfileKeyCredential; import org.thoughtcrime.securesms.color.MaterialColor; import org.thoughtcrime.securesms.database.RecipientDatabase.InsightsBannerTier; import org.thoughtcrime.securesms.database.RecipientDatabase.MentionSetting; @@ -49,7 +50,7 @@ public class RecipientDetails { final Optional defaultSubscriptionId; final RegisteredState registered; final byte[] profileKey; - final byte[] profileKeyCredential; + final ProfileKeyCredential profileKeyCredential; final String profileAvatar; final boolean hasProfileImage; final boolean profileSharing; diff --git a/app/src/main/proto/Database.proto b/app/src/main/proto/Database.proto index 8c191d5325..569c6b5ab7 100644 --- a/app/src/main/proto/Database.proto +++ b/app/src/main/proto/Database.proto @@ -77,3 +77,8 @@ message GroupCallUpdateDetails { repeated string inCallUuids = 4; bool isCallFull = 5; } + +message ProfileKeyCredentialColumnData { + bytes profileKey = 1; + bytes profileKeyCredential = 2; +}