Merge remote-tracking branch 'drebs/bug/3243-avoid-possible-timing-attack-in-hash-comparison' into develop

2 files changed, 10 insertions, 1 deletions

diff --git a/soledad/changes/bug_3243-avoid-possible-timing-attack-in-hash-comparison b/soledad/changes/bug_3243-avoid-possible-timing-attack-in-hash-comparison
new file mode 100644
index 00000000..0794b1ab
--- /dev/null
+++ b/soledad/changes/bug_3243-avoid-possible-timing-attack-in-hash-comparison
@@ -0,0 +1,2 @@
+  o Avoid possible timing attack in document's mac comparison by comparing
+    hashes instead of plain macs. Closes #3243.
diff --git a/soledad/src/leap/soledad/target.py b/soledad/src/leap/soledad/target.py
index 8b7aa8c7..9fac9f54 100644
--- a/soledad/src/leap/soledad/target.py
+++ b/soledad/src/leap/soledad/target.py
@@ -231,7 +231,14 @@ def decrypt_doc(crypto, doc):
         crypto, doc.doc_id, doc.rev,
         ciphertext,
         doc.content[MAC_METHOD_KEY])
-    if binascii.a2b_hex(doc.content[MAC_KEY]) != mac:  # mac is stored as hex.
+    # we compare mac's hashes to avoid possible timing attacks that might
+    # exploit python's builtin comparison operator behaviour, which fails
+    # immediatelly when non-matching bytes are found.
+    doc_mac_hash = hashlib.sha256(
+        binascii.a2b_hex(  # the mac is stored as hex
+            doc.content[MAC_KEY])).digest()
+    calculated_mac_hash = hashlib.sha256(mac).digest()
+    if doc_mac_hash != calculated_mac_hash:
         raise WrongMac('Could not authenticate document\'s contents.')
     # decrypt doc's content
     enc_scheme = doc.content[ENC_SCHEME_KEY]