Skip to content

Commit 2dcd1e0

Browse files
andrew-signaljrose-signal
andrew-signal
and
jrose-signal
authored
backups: Validate username uniqueness
Co-authored-by: Jordan Rose <[email protected]>
1 parent cc7a670 commit 2dcd1e0

File tree

7 files changed

+66
-8
lines changed

7 files changed

+66
-8
lines changed

‎RELEASE_NOTES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
v0.86.2
22

33
- backups: Include more account fields into the exported JSON
4+
- backups: Throw validator errors if multiple contacts have the same username (a case-insensitive check)

‎rust/message-backup/src/backup.rs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ pub enum CompletionError {
190190
MissingSelfRecipient,
191191
/// {0:?} and {1:?} have the same phone number
192192
DuplicateContactE164(RecipientId, RecipientId),
193+
/// {0:?} and {1:?} have the same username
194+
DuplicateContactUsername(RecipientId, RecipientId),
193195
/// {0:?} and {1:?} have the same ACI
194196
DuplicateContactAci(RecipientId, RecipientId),
195197
/// {0:?} and {1:?} have the same PNI
@@ -321,18 +323,23 @@ impl<M: Method + ReferencedTypes> CompletedBackup<M> {
321323
* std::mem::size_of::<(Aci, RecipientId)>()
322324
+ HIGH_BUT_REASONABLE_NUMBER_OF_CONTACTS
323325
* std::mem::size_of::<(Pni, RecipientId)>()
326+
+ HIGH_BUT_REASONABLE_NUMBER_OF_CONTACTS
327+
* std::mem::size_of::<(&str, RecipientId)>()
324328
+ HIGH_BUT_REASONABLE_NUMBER_OF_GROUPS
325329
* std::mem::size_of::<(zkgroup::GroupMasterKeyBytes, RecipientId)>()
326330
+ HIGH_BUT_REASONABLE_NUMBER_OF_DISTRIBUTION_LISTS
327331
* std::mem::size_of::<(uuid::Uuid, RecipientId)>()
328332
+ HIGH_BUT_REASONABLE_NUMBER_OF_CALL_LINKS
329333
* std::mem::size_of::<(call::CallLinkRootKey, RecipientId)>()
330-
< 250_000,
334+
< 350_000,
331335
);
332336

333337
let mut e164s = IntMap::<u64, RecipientId>::with_capacity(
334338
recipients.len().min(HIGH_BUT_REASONABLE_NUMBER_OF_CONTACTS),
335339
);
340+
let mut usernames = HashMap::<&str, RecipientId>::with_capacity(
341+
recipients.len().min(HIGH_BUT_REASONABLE_NUMBER_OF_CONTACTS),
342+
);
336343
let mut acis = AssumedRandomInputHasher::map_with_capacity::<Aci, RecipientId>(
337344
recipients.len().min(HIGH_BUT_REASONABLE_NUMBER_OF_CONTACTS),
338345
);
@@ -365,7 +372,19 @@ impl<M: Method + ReferencedTypes> CompletedBackup<M> {
365372

366373
for (id, recipient) in recipients.iter() {
367374
match recipient.as_ref() {
368-
MinimalRecipientData::Contact { e164, aci, pni } => {
375+
MinimalRecipientData::Contact {
376+
e164,
377+
aci,
378+
pni,
379+
username,
380+
} => {
381+
insert_or_error(
382+
&mut usernames,
383+
username.as_deref(),
384+
id,
385+
CompletionError::DuplicateContactUsername,
386+
)?;
387+
369388
// We can't use insert_or_throw_error for `e164s` because it's an IntMap.
370389
// Here's an inlined copy:
371390
if let Some(e164) = *e164 {
@@ -1279,6 +1298,9 @@ mod test {
12791298
(CompletionError::DuplicateContactE164, |x| {
12801299
x.e164 = Some(proto::Contact::TEST_E164.into());
12811300
}),
1301+
(CompletionError::DuplicateContactUsername, |x| {
1302+
x.username = Some("duplicate.1234".to_owned());
1303+
}),
12821304
]
12831305
)]
12841306
fn duplicate_contact_id<M: Method + ReferencedTypes>(

‎rust/message-backup/src/backup/call.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ impl<C: LookupPair<RecipientId, MinimalRecipientData, R> + ReportUnusualTimestam
247247
aci: None,
248248
e164: _,
249249
pni: _,
250+
username: _,
250251
} => Err(CallError::CallStarterHasNoAci(id)),
251252
MinimalRecipientData::Contact { .. } | MinimalRecipientData::Self_ => {
252253
Ok(starter.clone())
@@ -272,6 +273,7 @@ impl<C: LookupPair<RecipientId, MinimalRecipientData, R> + ReportUnusualTimestam
272273
aci: None,
273274
e164: _,
274275
pni: _,
276+
username: _,
275277
} => Err(CallError::RingerHasNoAci(id)),
276278
MinimalRecipientData::Contact { .. } | MinimalRecipientData::Self_ => {
277279
Ok(ringer.clone())
@@ -518,13 +520,15 @@ pub(crate) mod test {
518520
proto::Contact::TEST_ACI,
519521
)),
520522
pni: None,
523+
username: None,
521524
};
522525
static PNI_RECIPIENT: MinimalRecipientData = MinimalRecipientData::Contact {
523526
e164: Some(proto::Contact::TEST_E164),
524527
aci: None,
525528
pni: Some(libsignal_core::Pni::from_uuid_bytes(
526529
proto::Contact::TEST_PNI,
527530
)),
531+
username: None,
528532
};
529533
match key {
530534
RecipientId(proto::Recipient::TEST_ID) => Some((&CONTACT_RECIPIENT, key)),

‎rust/message-backup/src/backup/chat.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,7 @@ impl<
605605
e164: None,
606606
aci: None,
607607
pni: _,
608+
username: _,
608609
},
609610
Direction::Incoming { .. },
610611
) => Err(ChatItemError::IncomingMessageFromContactWithoutAciOrE164(

‎rust/message-backup/src/backup/chat/quote.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,20 @@ impl<R: Clone, C: LookupPair<RecipientId, MinimalRecipientData, R> + ReportUnusu
8888
return Err(QuoteError::AuthorNotFound(author_id));
8989
};
9090
let author = match author_data {
91-
MinimalRecipientData::Contact { e164: None, aci: None, pni: _ } => {
91+
MinimalRecipientData::Contact {
92+
e164: None,
93+
aci: None,
94+
pni: _,
95+
username: _,
96+
} => {
9297
Err(QuoteError::AuthorHasNoAciOrE164(author_id))
9398
}
94-
MinimalRecipientData::Contact { e164: _, aci: _, pni: _ } => {
99+
MinimalRecipientData::Contact {
100+
e164: _,
101+
aci: _,
102+
pni: _,
103+
username: _,
104+
} => {
95105
Ok(author.clone())
96106
}
97107
MinimalRecipientData::Self_

‎rust/message-backup/src/backup/chat/reactions.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,13 @@ impl<R: Clone, C: LookupPair<RecipientId, MinimalRecipientData, R> + ReportUnusu
8181
e164: None,
8282
aci: None,
8383
pni: _,
84+
username: _,
8485
} => Err(ReactionError::AuthorHasNoAciOrE164(author_id)),
8586
MinimalRecipientData::Contact {
8687
e164: _,
8788
aci: _,
8889
pni: _,
90+
username: _,
8991
}
9092
| MinimalRecipientData::Self_ => Ok(author.clone()),
9193
MinimalRecipientData::Group { .. }

‎rust/message-backup/src/backup/recipient.rs‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ pub enum MinimalRecipientData {
103103
e164: Option<E164>,
104104
aci: Option<Aci>,
105105
pni: Option<Pni>,
106+
username: Option<String>,
106107
},
107108
Group {
108109
master_key: zkgroup::GroupMasterKeyBytes,
@@ -388,9 +389,18 @@ impl std::ops::Deref for FullRecipientData {
388389
impl<R> From<Destination<R>> for MinimalRecipientData {
389390
fn from(value: Destination<R>) -> Self {
390391
match value {
391-
Destination::Contact(ContactData { aci, pni, e164, .. }) => {
392-
Self::Contact { e164, aci, pni }
393-
}
392+
Destination::Contact(ContactData {
393+
aci,
394+
pni,
395+
username,
396+
e164,
397+
..
398+
}) => Self::Contact {
399+
e164,
400+
aci,
401+
pni,
402+
username,
403+
},
394404
Destination::Group(GroupData { master_key, .. }) => Self::Group { master_key },
395405
Destination::DistributionList(
396406
DistributionListItem::Deleted {
@@ -541,7 +551,14 @@ impl<C: ReportUnusualTimestamp> TryIntoWith<ContactData, C> for proto::Contact {
541551
.map(|username| {
542552
usernames::Username::new(&username)
543553
.map_err(|_| RecipientError::InvalidContactUsername)
544-
.map(|_| username)
554+
.map(|_| {
555+
// NB: There's a little bit of spooky at a distance here.
556+
// By storing the username in a cannonical lowercase format as soon as
557+
// we pull it in from the proto, we ensure that if this backup has multiple
558+
// usernames that are the same despite the capitalization, they will be
559+
// caught by the de-duplication check in the CompletedBackup::try_from.
560+
username.to_ascii_lowercase()
561+
})
545562
})
546563
.transpose()?;
547564

@@ -713,6 +730,7 @@ impl<R: Clone, C: LookupPair<RecipientId, MinimalRecipientData, R> + ReportUnusu
713730
aci: None,
714731
pni: None,
715732
e164: _,
733+
username: _,
716734
} => Err(RecipientError::DistributionListMemberHasNoServiceIds(id)),
717735
MinimalRecipientData::Contact { .. } => {
718736
Ok(recipient_reference.clone())

0 commit comments

Comments
 (0)