Revert "Add registration IDs to the Sealed Sender v2 upload (encrypt)…

… format" (#303)
signalapp · May 17, 2021 · 1fd8da6 · 1fd8da6
1 parent 45f717a
commit 1fd8da6
Show file tree

Hide file tree

Showing 19 changed files with 8 additions and 112 deletions.
diff --git a/java/java/src/main/java/org/signal/internal/Native.java b/java/java/src/main/java/org/signal/internal/Native.java
@@ -178,7 +178,7 @@ private Native() {}
 
   public static native long SealedSessionCipher_DecryptToUsmc(byte[] ctext, IdentityKeyStore identityStore, Object ctx);
   public static native byte[] SealedSessionCipher_Encrypt(long destination, long content, IdentityKeyStore identityKeyStore, Object ctx);
-  public static native byte[] SealedSessionCipher_MultiRecipientEncrypt(long[] recipients, int[] recipientRegistrationIds, long content, IdentityKeyStore identityKeyStore, Object ctx);
+  public static native byte[] SealedSessionCipher_MultiRecipientEncrypt(long[] recipients, long content, IdentityKeyStore identityKeyStore, Object ctx);
   public static native byte[] SealedSessionCipher_MultiRecipientMessageForSingleRecipient(byte[] encodedMultiRecipientMessage);
 
   public static native long SenderCertificate_Deserialize(byte[] data);

diff --git a/java/java/src/main/java/org/signal/libsignal/metadata/SealedSessionCipher.java b/java/java/src/main/java/org/signal/libsignal/metadata/SealedSessionCipher.java
@@ -76,7 +76,7 @@ public byte[] encrypt(SignalProtocolAddress destinationAddress, UnidentifiedSend
       null);
   }
 
-  public byte[] multiRecipientEncrypt(List<SignalProtocolAddress> recipients, int[] recipientRegistrationIds, UnidentifiedSenderMessageContent content)
+  public byte[] multiRecipientEncrypt(List<SignalProtocolAddress> recipients, UnidentifiedSenderMessageContent content)
       throws InvalidKeyException, UntrustedIdentityException
   {
     long[] recipientHandles = new long[recipients.size()];
@@ -87,7 +87,6 @@ public byte[] multiRecipientEncrypt(List<SignalProtocolAddress> recipients, int[
     }
     return Native.SealedSessionCipher_MultiRecipientEncrypt(
       recipientHandles,
-      recipientRegistrationIds,
       content.nativeHandle(),
       this.signalProtocolStore,
       null);

diff --git a/java/tests/src/test/java/org/signal/libsignal/metadata/SealedSessionCipherTest.java b/java/tests/src/test/java/org/signal/libsignal/metadata/SealedSessionCipherTest.java
@@ -172,7 +172,7 @@ public void testEncryptDecryptGroup() throws UntrustedIdentityException, Invalid
 
     UnidentifiedSenderMessageContent usmcFromAlice = new UnidentifiedSenderMessageContent(ciphertextFromAlice, senderCertificate, UnidentifiedSenderMessageContent.CONTENT_HINT_SUPPLEMENTARY, Optional.of(new byte[]{42}));
 
-    byte[] aliceMessage = aliceCipher.multiRecipientEncrypt(Arrays.asList(bobAddress), new int[]{0x3FFD}, usmcFromAlice);
+    byte[] aliceMessage = aliceCipher.multiRecipientEncrypt(Arrays.asList(bobAddress), usmcFromAlice);
     byte[] bobMessage = SealedSessionCipher.multiRecipientMessageForSingleRecipient(aliceMessage);
 
     DecryptionResult plaintext = bobCipher.decrypt(new CertificateValidator(trustRoot.getPublicKey()), bobMessage, 31335);
@@ -211,7 +211,7 @@ public void testProtocolException() throws UntrustedIdentityException, InvalidKe
 
     UnidentifiedSenderMessageContent usmcFromAlice = new UnidentifiedSenderMessageContent(ciphertextFromAlice, senderCertificate, UnidentifiedSenderMessageContent.CONTENT_HINT_SUPPLEMENTARY, Optional.of(new byte[]{42, 1}));
 
-    byte[] aliceMessage = aliceCipher.multiRecipientEncrypt(Arrays.asList(bobAddress), new int[]{0x3FFD}, usmcFromAlice);
+    byte[] aliceMessage = aliceCipher.multiRecipientEncrypt(Arrays.asList(bobAddress), usmcFromAlice);
     byte[] bobMessage = SealedSessionCipher.multiRecipientMessageForSingleRecipient(aliceMessage);
 
     try {

diff --git a/node/Native.d.ts b/node/Native.d.ts
@@ -99,7 +99,7 @@ export function SealedSenderDecryptionResult_Message(obj: Wrapper<SealedSenderDe
 export function SealedSender_DecryptMessage(message: Buffer, trustRoot: Wrapper<PublicKey>, timestamp: number, localE164: string | null, localUuid: string, localDeviceId: number, sessionStore: SessionStore, identityStore: IdentityKeyStore, prekeyStore: PreKeyStore, signedPrekeyStore: SignedPreKeyStore): Promise<SealedSenderDecryptionResult>;
 export function SealedSender_DecryptToUsmc(ctext: Buffer, identityStore: IdentityKeyStore, ctx: null): Promise<UnidentifiedSenderMessageContent>;
 export function SealedSender_Encrypt(destination: Wrapper<ProtocolAddress>, content: Wrapper<UnidentifiedSenderMessageContent>, identityKeyStore: IdentityKeyStore, ctx: null): Promise<Buffer>;
-export function SealedSender_MultiRecipientEncrypt(recipients: Wrapper<ProtocolAddress>[], recipientRegistrationIds: number[], content: Wrapper<UnidentifiedSenderMessageContent>, identityKeyStore: IdentityKeyStore, ctx: null): Promise<Buffer>;
+export function SealedSender_MultiRecipientEncrypt(recipients: Wrapper<ProtocolAddress>[], content: Wrapper<UnidentifiedSenderMessageContent>, identityKeyStore: IdentityKeyStore, ctx: null): Promise<Buffer>;
 export function SealedSender_MultiRecipientMessageForSingleRecipient(encodedMultiRecipientMessage: Buffer): Buffer;
 export function SenderCertificate_Deserialize(buffer: Buffer): SenderCertificate;
 export function SenderCertificate_GetCertificate(obj: Wrapper<SenderCertificate>): Buffer;

diff --git a/node/index.ts b/node/index.ts
@@ -1319,12 +1319,10 @@ export function sealedSenderEncrypt(
 export function sealedSenderMultiRecipientEncrypt(
   content: UnidentifiedSenderMessageContent,
   recipients: ProtocolAddress[],
-  recipientRegistrationIds: number[],
   identityStore: IdentityKeyStore
 ): Promise<Buffer> {
   return NativeImpl.SealedSender_MultiRecipientEncrypt(
     recipients,
-    recipientRegistrationIds,
     content,
     identityStore,
     null

diff --git a/node/test/PublicAPITest.ts b/node/test/PublicAPITest.ts
@@ -1251,7 +1251,6 @@ describe('SignalClient', () => {
       const aSealedSenderMessage = await SignalClient.sealedSenderMultiRecipientEncrypt(
         aUsmc,
         [bAddress],
-        [0x3ffd],
         aKeys
       );
 

diff --git a/rust/bridge/jni/bin/gen_java_decl.py b/rust/bridge/jni/bin/gen_java_decl.py
@@ -69,7 +69,6 @@ def translate_to_java(typ):
         "JObject": "Object",
         "JClass": "Class",
         "jbyteArray": "byte[]",
-        "jintArray": "int[]",
         "jlongArray": "long[]",
         "ObjectHandle": "long",
         "jint": "int",

diff --git a/rust/bridge/node/bin/gen_ts_decl.py b/rust/bridge/node/bin/gen_ts_decl.py
@@ -25,7 +25,6 @@ def translate_to_ts(typ):
     type_map = {
         "()": "void",
         "&[u8]": "Buffer",
-        "&[u16]": "number[]",
         "i32": "number",
         "u8": "number",
         "u32": "number",

diff --git a/rust/bridge/shared/src/ffi/convert.rs b/rust/bridge/shared/src/ffi/convert.rs
@@ -167,21 +167,6 @@ impl SizedArgTypeInfo for &[u8] {
     }
 }
 
-impl SizedArgTypeInfo for &[u16] {
-    type ArgType = *const u16;
-    fn convert_from(input: Self::ArgType, input_len: usize) -> SignalFfiResult<Self> {
-        if input.is_null() {
-            if input_len != 0 {
-                return Err(SignalFfiError::NullPointer);
-            }
-            // We can't just fall through because slice::from_raw_parts still expects a non-null pointer. Reference a dummy buffer instead.
-            return Ok(&[]);
-        }
-
-        unsafe { Ok(std::slice::from_raw_parts(input, input_len)) }
-    }
-}
-
 impl SizedArgTypeInfo for &mut [u8] {
     type ArgType = *mut c_uchar;
     fn convert_from(input: Self::ArgType, input_len: usize) -> SignalFfiResult<Self> {
@@ -470,7 +455,6 @@ macro_rules! trivial {
 
 trivial!(i32);
 trivial!(u8);
-trivial!(u16);
 trivial!(u32);
 trivial!(u64);
 trivial!(usize);
@@ -488,13 +472,11 @@ trivial!(bool);
 /// (For example, `(&[u8]) => (*const libc::c_uchar);`.)
 macro_rules! ffi_arg_type {
     (u8) => (u8);
-    (u16) => (u16);
     (u32) => (u32);
     (u64) => (u64);
     (Option<u32>) => (u32);
     (usize) => (libc::size_t);
     (&[u8]) => (*const libc::c_uchar);
-    (&[u16]) => (*const u16);
     (&mut [u8]) => (*mut libc::c_uchar);
     (String) => (*const libc::c_char);
     (Option<String>) => (*const libc::c_char);

diff --git a/rust/bridge/shared/src/jni/convert.rs b/rust/bridge/shared/src/jni/convert.rs
@@ -171,15 +171,6 @@ impl<'a> SimpleArgTypeInfo<'a> for u8 {
     }
 }
 
-/// Supports all valid u16 values `0..=65535`.
-impl<'a> SimpleArgTypeInfo<'a> for u16 {
-    type ArgType = jint;
-    fn convert_from(_env: &JNIEnv, foreign: jint) -> SignalJniResult<Self> {
-        u16::try_from(foreign)
-            .map_err(|_| SignalJniError::IntegerOverflow(format!("{} to u16", foreign)))
-    }
-}
-
 impl<'a> SimpleArgTypeInfo<'a> for String {
     type ArgType = JString<'a>;
     fn convert_from(env: &JNIEnv, foreign: JString<'a>) -> SignalJniResult<Self> {
@@ -266,28 +257,6 @@ impl<'storage, 'context: 'storage> ArgTypeInfo<'storage, 'context> for &'storage
     }
 }
 
-/// Maps a Java `int[]` to a Rust `[u16]` to preserve the full range.
-impl<'storage, 'context: 'storage> ArgTypeInfo<'storage, 'context> for &'storage [u16] {
-    type ArgType = jintArray;
-    type StoredType = Vec<u16>;
-    fn borrow(env: &'context JNIEnv, foreign: Self::ArgType) -> SignalJniResult<Self::StoredType> {
-        let java_array = env.get_int_array_elements(foreign, ReleaseMode::NoCopyBack)?;
-        let slice = unsafe {
-            std::slice::from_raw_parts(
-                java_array.as_ptr() as *const jint,
-                java_array.size()? as usize,
-            )
-        };
-        slice.iter().map(|v| u16::convert_from(env, *v)).collect()
-    }
-    fn load_from(
-        _env: &JNIEnv,
-        stored: &'storage mut Self::StoredType,
-    ) -> SignalJniResult<&'storage [u16]> {
-        Ok(&stored[..])
-    }
-}
-
 macro_rules! store {
     ($name:ident) => {
         paste! {
@@ -709,9 +678,6 @@ macro_rules! jni_arg_type {
     (&mut [u8]) => {
         jni::jbyteArray
     };
-    (&[u16]) => {
-        jni::jintArray
-    };
     (Context) => {
         jni::JObject
     };

diff --git a/rust/bridge/shared/src/jni/mod.rs b/rust/bridge/shared/src/jni/mod.rs
@@ -15,7 +15,7 @@ use std::convert::{TryFrom, TryInto};
 use std::error::Error;
 
 pub(crate) use jni::objects::{AutoArray, JClass, JObject, JString, ReleaseMode};
-pub(crate) use jni::sys::{jboolean, jbyteArray, jint, jintArray, jlong, jlongArray, jstring};
+pub(crate) use jni::sys::{jboolean, jbyteArray, jint, jlong, jlongArray, jstring};
 pub(crate) use jni::JNIEnv;
 
 /// Converts a function signature to a JNI signature string.

diff --git a/rust/bridge/shared/src/node/convert.rs b/rust/bridge/shared/src/node/convert.rs
@@ -246,29 +246,6 @@ impl SimpleArgTypeInfo for uuid::Uuid {
     }
 }
 
-impl<'storage> AsyncArgTypeInfo<'storage> for &'storage [u16] {
-    type ArgType = JsArray;
-    type StoredType = Vec<u16>;
-
-    fn save_async_arg(
-        cx: &mut FunctionContext,
-        foreign: Handle<Self::ArgType>,
-    ) -> NeonResult<Self::StoredType> {
-        let len = foreign.len(cx);
-        let mut result = Vec::with_capacity(len as usize);
-        for i in 0..len {
-            let next = foreign.get(cx, i)?.downcast_or_throw::<JsNumber, _>(cx)?;
-            let converted = u16::convert_from(cx, next)?;
-            result.push(converted)
-        }
-        Ok(result)
-    }
-
-    fn load_async_arg(stored: &'storage mut Self::StoredType) -> Self {
-        &stored[..]
-    }
-}
-
 /// Converts `null` to `None`, passing through all other values.
 impl<'storage, 'context: 'storage, T> ArgTypeInfo<'storage, 'context> for Option<T>
 where
@@ -641,7 +618,6 @@ macro_rules! full_range_integer {
 }
 
 full_range_integer!(u8);
-full_range_integer!(u16);
 full_range_integer!(u32);
 full_range_integer!(i32);
 

diff --git a/rust/bridge/shared/src/protocol.rs b/rust/bridge/shared/src/protocol.rs
@@ -986,15 +986,13 @@ async fn SealedSessionCipher_Encrypt<E: Env>(
 async fn SealedSender_MultiRecipientEncrypt<E: Env>(
     env: E,
     recipients: &[&ProtocolAddress],
-    recipient_registration_ids: &[u16],
     content: &UnidentifiedSenderMessageContent,
     identity_key_store: &mut dyn IdentityKeyStore,
     ctx: Context,
 ) -> Result<E::Buffer> {
     let mut rng = rand::rngs::OsRng;
     let ctext = sealed_sender_multi_recipient_encrypt(
         recipients,
-        recipient_registration_ids,
         content,
         identity_key_store,
         ctx,

diff --git a/rust/protocol/src/sealed_sender.rs b/rust/protocol/src/sealed_sender.rs
@@ -870,18 +870,11 @@ mod sealed_sender_v2 {
 
 pub async fn sealed_sender_multi_recipient_encrypt<R: Rng + CryptoRng>(
     destinations: &[&ProtocolAddress],
-    destination_registration_ids: &[u16],
     usmc: &UnidentifiedSenderMessageContent,
     identity_store: &mut dyn IdentityKeyStore,
     ctx: Context,
     rng: &mut R,
 ) -> Result<Vec<u8>> {
-    if destinations.len() != destination_registration_ids.len() {
-        return Err(SignalProtocolError::InvalidArgument(
-            "must have the same number of destination registration IDs as addresses".to_string(),
-        ));
-    }
-
     let m: [u8; 32] = rng.gen();
     let keys = sealed_sender_v2::DerivedKeys::calculate(&m);
     let e_pub = keys.e.public_key()?;
@@ -902,15 +895,15 @@ pub async fn sealed_sender_multi_recipient_encrypt<R: Rng + CryptoRng>(
             SignalProtocolError::InternalError("failed to encrypt using AES-GCM-SIV")
         })?;
 
-    // Uses a flat representation: count || UUID_i || deviceId_i || registrationId_i || C_i || AT_i || ... || E.pub || ciphertext
+    // Uses a flat representation: count || UUID_i || deviceId_i || C_i || AT_i || ... || E.pub || ciphertext
     let version = SEALED_SENDER_V2_VERSION;
     let mut serialized: Vec<u8> = vec![(version | (version << 4))];
 
     prost::encode_length_delimiter(destinations.len(), &mut serialized)
         .expect("cannot fail encoding to Vec");
 
     let our_identity = identity_store.get_identity_key_pair(ctx).await?;
-    for (destination, registration_id) in destinations.iter().zip(destination_registration_ids) {
+    for destination in destinations {
         let their_uuid = Uuid::parse_str(destination.name()).map_err(|_| {
             SignalProtocolError::InvalidArgument(format!(
                 "multi-recipient sealed sender requires UUID recipients (not {})",
@@ -941,7 +934,6 @@ pub async fn sealed_sender_multi_recipient_encrypt<R: Rng + CryptoRng>(
         serialized.extend_from_slice(their_uuid.as_bytes());
         prost::encode_length_delimiter(destination.device_id() as usize, &mut serialized)
             .expect("cannot fail encoding to Vec");
-        serialized.extend_from_slice(&registration_id.to_be_bytes());
         serialized.extend_from_slice(&c_i);
         serialized.extend_from_slice(&at_i);
     }
@@ -986,8 +978,6 @@ pub fn sealed_sender_multi_recipient_fan_out(data: &[u8]) -> Result<Vec<Vec<u8>>
         let _ = advance(&mut remaining, 16)?;
         // Skip device ID.
         let _ = decode_varint(&mut remaining)?;
-        // Skip registration ID.
-        let _ = advance(&mut remaining, 2)?;
         // Read C_i and AT_i.
         let c_and_at = advance(&mut remaining, 32 + 16)?;
 

diff --git a/rust/protocol/tests/groups.rs b/rust/protocol/tests/groups.rs
@@ -330,7 +330,6 @@ fn group_sealed_sender() -> Result<(), SignalProtocolError> {
 
         let alice_ctext = sealed_sender_multi_recipient_encrypt(
             &[&bob_uuid_address, &carol_uuid_address],
-            &[0x3FFD, 0x07E5],
             &alice_usmc,
             &mut alice_store.identity_store,
             None,

diff --git a/rust/protocol/tests/sealed_sender.rs b/rust/protocol/tests/sealed_sender.rs
@@ -498,7 +498,6 @@ fn test_sealed_sender_multi_recipient() -> Result<(), SignalProtocolError> {
 
         let alice_ctext = sealed_sender_multi_recipient_encrypt(
             &[&bob_uuid_address],
-            &[0x3FFD],
             &alice_usmc,
             &mut alice_store.identity_store,
             None,
@@ -549,7 +548,6 @@ fn test_sealed_sender_multi_recipient() -> Result<(), SignalProtocolError> {
 
         let alice_ctext = sealed_sender_multi_recipient_encrypt(
             &[&bob_uuid_address],
-            &[0x3FFD],
             &alice_usmc,
             &mut alice_store.identity_store,
             None,
@@ -606,7 +604,6 @@ fn test_sealed_sender_multi_recipient() -> Result<(), SignalProtocolError> {
 
         let alice_ctext = sealed_sender_multi_recipient_encrypt(
             &[&bob_uuid_address],
-            &[0x3FFD],
             &alice_usmc,
             &mut alice_store.identity_store,
             None,

diff --git a/swift/Sources/SignalClient/SealedSender.swift b/swift/Sources/SignalClient/SealedSender.swift
@@ -149,7 +149,6 @@ public func sealedSenderEncrypt(_ content: UnidentifiedSenderMessageContent,
 
 public func sealedSenderMultiRecipientEncrypt(_ content: UnidentifiedSenderMessageContent,
                                               for recipients: [ProtocolAddress],
-                                              registrationIds: [UInt16],
                                               identityStore: IdentityKeyStore,
                                               context: StoreContext) throws -> [UInt8] {
     return try context.withOpaquePointer { context in
@@ -158,8 +157,6 @@ public func sealedSenderMultiRecipientEncrypt(_ content: UnidentifiedSenderMessa
                 signal_sealed_sender_multi_recipient_encrypt($0, $1,
                                                              recipients.map { $0.nativeHandle },
                                                              recipients.count,
-                                                             registrationIds,
-                                                             registrationIds.count,
                                                              content.nativeHandle,
                                                              ffiIdentityStore, context)
             }

diff --git a/swift/Sources/SignalFfi/signal_ffi.h b/swift/Sources/SignalFfi/signal_ffi.h
@@ -896,8 +896,6 @@ SignalFfiError *signal_sealed_sender_multi_recipient_encrypt(const unsigned char
                                                              size_t *out_len,
                                                              const SignalProtocolAddress *const *recipients,
                                                              size_t recipients_len,
-                                                             const uint16_t *recipient_registration_ids,
-                                                             size_t recipient_registration_ids_len,
                                                              const SignalUnidentifiedSenderMessageContent *content,
                                                              const SignalIdentityKeyStore *identity_key_store,
                                                              void *ctx);

diff --git a/swift/Tests/SignalClientTests/SessionTests.swift b/swift/Tests/SignalClientTests/SessionTests.swift
@@ -282,7 +282,6 @@ class SessionTests: TestCaseBase {
 
         let a_ctext = try! sealedSenderMultiRecipientEncrypt(a_usmc,
                                                              for: [bob_address],
-                                                             registrationIds: [0x3FFD],
                                                              identityStore: alice_store,
                                                              context: NullContext())
-Original file line number
+Diff line change
@@ Expand Up / @@ -1251,7 +1251,6 @@ describe('SignalClient', () => { @@
           const aSealedSenderMessage = await SignalClient.sealedSenderMultiRecipientEncrypt(
             aUsmc,
             [bAddress],
-            [0x3ffd],
             aKeys
           );
@@ Expand Down @@