From e8b4474cb9c9342bd533528f644582300a80bb74 Mon Sep 17 00:00:00 2001
From: Jack Lloyd <jack@signal.org>
Date: Thu, 7 Jan 2021 14:08:50 -0500
Subject: [PATCH] Fix handling when attempting to decrypt with a session that
 isn't found

There were two discrepancies between the logic here and the original
logic of libsignal-protocol-java.

First, if the session record had an uninitialized active session, in
Java this would still attempt decryption with the old session states,
but Rust would stop immediately without trying the old states. [I am
not sure if this ever happens but it could possibly occur due to use
of archiveCurrentState]

Secondly, we returned the wrong error condition. We treated lack of a
sender chain as an invalid state (effectively an internal error) but
Java treats it as an invalid message, which makes sense in so far as
it is a message which we are unable to process with the information we
have available. This wrong error type led to an unexpected exception
being thrown in Android.
---
 rust/protocol/src/session_cipher.rs | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/rust/protocol/src/session_cipher.rs b/rust/protocol/src/session_cipher.rs
index 1893de303..9ccdfce76 100644
--- a/rust/protocol/src/session_cipher.rs
+++ b/rust/protocol/src/session_cipher.rs
@@ -243,21 +243,23 @@ fn decrypt_message_with_record<R: Rng + CryptoRng>(
     ciphertext: &SignalMessage,
     csprng: &mut R,
 ) -> Result<Vec<u8>> {
-    let mut current_state = record.session_state()?.clone();
+    if let Ok(current_state) = record.session_state() {
+        let mut current_state = current_state.clone();
+        let result = decrypt_message_with_state(&mut current_state, ciphertext, csprng);
 
-    let result = decrypt_message_with_state(&mut current_state, ciphertext, csprng);
-
-    match result {
-        Ok(ptext) => {
-            record.set_session_state(current_state)?; // update the state
-            return Ok(ptext);
+        match result {
+            Ok(ptext) => {
+                record.set_session_state(current_state)?; // update the state
+                return Ok(ptext);
+            }
+            Err(SignalProtocolError::DuplicatedMessage(_, _)) => {
+                return result;
+            }
+            Err(_) => {}
         }
-        Err(SignalProtocolError::DuplicatedMessage(_, _)) => {
-            return result;
-        }
-        Err(_) => {}
     }
 
+    // Try some old sessions:
     let mut updated_session = None;
 
     for (idx, previous) in record.previous_session_states()?.enumerate() {
@@ -293,7 +295,9 @@ fn decrypt_message_with_state<R: Rng + CryptoRng>(
     csprng: &mut R,
 ) -> Result<Vec<u8>> {
     if !state.has_sender_chain()? {
-        return Err(SignalProtocolError::InvalidSessionStructure);
+        return Err(SignalProtocolError::InvalidMessage(
+            "No session available to decrypt",
+        ));
     }
 
     let ciphertext_version = ciphertext.message_version() as u32;