From 42f6d2459f2e11dbd86e309a73d992a31713da8c Mon Sep 17 00:00:00 2001
From: Andrew Rowson <github@growse.com>
Date: Sun, 8 Sep 2024 17:25:06 +0100
Subject: [PATCH] fix(encryption): better exception handling for failed
 decryption of incoming encrypted messages

Fixes #1831
---
 CHANGELOG.md                                  |  2 ++
 .../android/model/EncryptionProvider.java     | 27 ++++++++++++-------
 .../org/owntracks/android/model/Parser.kt     |  7 +++--
 .../net/mqtt/MQTTMessageProcessorEndpoint.kt  |  2 +-
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8a07f23be1..db7a00ef5b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,8 @@
 - Fix bug where geofencing client wasn't initialized properly, leading to very unreliable region transition detection
 - Fix bug where some settings (`pubQos`, `mqttProtocolLevel` etc.) couldn't be set via the config editor (#1801)
 - Fix crash when trying to decode an invalid face image on an info card
+- Fix MQTT disconnect when receiving an encrypted message that can't be decrypted (#1831)
+- Fix HTTP client certs not working properly with Nginx (#1793)
 
 ## Version 2.5.1
 
diff --git a/project/app/src/main/java/org/owntracks/android/model/EncryptionProvider.java b/project/app/src/main/java/org/owntracks/android/model/EncryptionProvider.java
index 1cdc878315..602f5db2e3 100644
--- a/project/app/src/main/java/org/owntracks/android/model/EncryptionProvider.java
+++ b/project/app/src/main/java/org/owntracks/android/model/EncryptionProvider.java
@@ -68,16 +68,25 @@ public EncryptionProvider(Preferences preferences) {
     }
 
     String decrypt(String cyphertextb64) throws Parser.EncryptionException {
-        byte[] onTheWire = Base64.decode(cyphertextb64.getBytes(), Base64.DEFAULT);
-        byte[] nonce = new byte[crypto_secretbox_NONCEBYTES];
-        if (onTheWire.length <= crypto_secretbox_NONCEBYTES) {
-            throw new Parser.EncryptionException("Message length shorter than nonce");
-        }
-        byte[] cyphertext = new byte[onTheWire.length - crypto_secretbox_NONCEBYTES];
+        try {
+            byte[] onTheWire = Base64.decode(cyphertextb64.getBytes(), Base64.DEFAULT);
 
-        System.arraycopy(onTheWire, 0, nonce, 0, crypto_secretbox_NONCEBYTES);
-        System.arraycopy(onTheWire, crypto_secretbox_NONCEBYTES, cyphertext, 0, onTheWire.length - crypto_secretbox_NONCEBYTES);
-        return new String(b.decrypt(nonce, cyphertext));
+            byte[] nonce = new byte[crypto_secretbox_NONCEBYTES];
+            if (onTheWire.length <= crypto_secretbox_NONCEBYTES) {
+                throw new Parser.EncryptionException("Message length shorter than nonce");
+            }
+            byte[] cyphertext = new byte[onTheWire.length - crypto_secretbox_NONCEBYTES];
+
+            System.arraycopy(onTheWire, 0, nonce, 0, crypto_secretbox_NONCEBYTES);
+            System.arraycopy(onTheWire, crypto_secretbox_NONCEBYTES, cyphertext, 0, onTheWire.length - crypto_secretbox_NONCEBYTES);
+            try {
+                return new String(b.decrypt(nonce, cyphertext));
+            } catch (Exception e) {
+                throw new Parser.EncryptionException("Decryption failed", e);
+            }
+        } catch (IllegalArgumentException e) {
+            throw new Parser.EncryptionException("Base64 decoding failed", e);
+        }
     }
 
     String encrypt(@NonNull String plaintext) {
diff --git a/project/app/src/main/java/org/owntracks/android/model/Parser.kt b/project/app/src/main/java/org/owntracks/android/model/Parser.kt
index 2a706d017b..5a388ac877 100644
--- a/project/app/src/main/java/org/owntracks/android/model/Parser.kt
+++ b/project/app/src/main/java/org/owntracks/android/model/Parser.kt
@@ -97,7 +97,7 @@ class Parser @Inject constructor(private val encryptionProvider: EncryptionProvi
     return if (a.size == 1 && a[0] is MessageEncrypted) {
       if (encryptionProvider == null || !encryptionProvider.isPayloadEncryptionEnabled) {
         throw EncryptionException(
-            "received encrypted message but payload encryption is not enabled")
+            "received encrypted message but payload encryption is not enabled", null)
       }
       defaultMapper.readValue(
           encryptionProvider.decrypt((a[0] as MessageEncrypted).data),
@@ -139,7 +139,10 @@ class Parser @Inject constructor(private val encryptionProvider: EncryptionProvi
     return input
   }
 
-  class EncryptionException internal constructor(s: String?) : Exception(s)
+  class EncryptionException internal constructor(s: String, cause: Throwable?) :
+      Exception(s, cause) {
+    constructor(s: String) : this(s, null)
+  }
 }
 
 val thisModule =
diff --git a/project/app/src/main/java/org/owntracks/android/net/mqtt/MQTTMessageProcessorEndpoint.kt b/project/app/src/main/java/org/owntracks/android/net/mqtt/MQTTMessageProcessorEndpoint.kt
index bd43eee631..a78ff6dbf7 100644
--- a/project/app/src/main/java/org/owntracks/android/net/mqtt/MQTTMessageProcessorEndpoint.kt
+++ b/project/app/src/main/java/org/owntracks/android/net/mqtt/MQTTMessageProcessorEndpoint.kt
@@ -326,7 +326,7 @@ class MQTTMessageProcessorEndpoint(
                       }
                       .also { Timber.d("Parsed message: $it") })
             } catch (e: Parser.EncryptionException) {
-              Timber.w("Enable to decrypt received message ${message.id} on $topic")
+              Timber.w("Unable to decrypt received message ${message.id} on $topic")
             } catch (e: InvalidFormatException) {
               Timber.w("Malformed JSON message received ${message.id} on $topic")
             }