Skip to content

feat: Remove detached signature validation#8196

Closed
Hocuri wants to merge 1 commit intomainfrom
hoc/remove-parse-detached
Closed

feat: Remove detached signature validation#8196
Hocuri wants to merge 1 commit intomainfrom
hoc/remove-parse-detached

Conversation

@Hocuri
Copy link
Copy Markdown
Collaborator

@Hocuri Hocuri commented Apr 30, 2026

I tried to remove detached signature validation, but it makes 11 tests fail. OTOH, on a quick glance, all of them test receiving some weird emails, so maybe we can just remove or adapt the tests and then merge it.

cc @link2xt @iequidoo

@iequidoo
Copy link
Copy Markdown
Collaborator

iequidoo commented Apr 30, 2026

Maybe we need to just unwrap "multipart/signed", but w/o signature validation? Btw, some email providers are known for breaking signed-only emails by reformatting the text, so it's expected that sometimes validation fails :)

@Hocuri
Copy link
Copy Markdown
Collaborator Author

Hocuri commented Apr 30, 2026

This would look like this then:

diff --git a/src/mimeparser.rs b/src/mimeparser.rs
index 88569d5db..346aefed5 100644
--- a/src/mimeparser.rs
+++ b/src/mimeparser.rs
@@ -488,14 +488,13 @@ pub(crate) async fn from_bytes(context: &Context, body: &[u8]) -> Result<Self> {
         };
 
         let mail = mail.as_ref().map(|mail| {
-            let (content, signatures_detached) = validate_detached_signature(mail, &public_keyring)
-                .unwrap_or((mail, Default::default()));
-            let signatures_detached = signatures_detached
-                .into_iter()
-                .map(|fp| (fp, Vec::new()))
-                .collect::<HashMap<_, _>>();
-            signatures.extend(signatures_detached);
+            if mail.ctype.mimetype == "multipart/signed"
+                && let [content, _detached_signature] = &mail.subparts[..]
+            {
                 content
+            } else {
+                mail
+            }
         });
 
         if let Some(expected_sender_fingerprint) = expected_sender_fingerprint {

but I still have 7 tests failing then. Not sure if it's worth it or not to pursue removing this code, and to look into the test failures closely.

@link2xt
Copy link
Copy Markdown
Collaborator

link2xt commented Apr 30, 2026

I think we better just keep it as is then.

"Detached signature" is not about signed-only messages, it is a format that Thunderbird used at the time the code was added, or even still uses, where the message is signed with a detached signature, then encrypted and is sent as multipart/encrypted, rather than encrypted+signed in a single step.

If we want to remove this, need to test that Protonmail and Thunderbird does not use it, otherwise we are breaking compatibility with encrypted+signed messages unnecessarily and we want to have some interoperability there. Don't think it is worth the effort to dive deep into this and test if these clients have switched away from detached signature and if they did at what version.

@Hocuri Hocuri closed this Apr 30, 2026
@link2xt
Copy link
Copy Markdown
Collaborator

link2xt commented Apr 30, 2026

For reference, here is the issue from Thunderbird, actually it is closed now: https://bugzilla.mozilla.org/show_bug.cgi?id=1688863

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants