Aggressive LMTP header cleanup#816
Conversation
feld
had a problem deploying
to
staging-ipv4.testrun.org
GitHub Actions
Failure
feld
had a problem deploying
to
staging2.testrun.org
GitHub Actions
Failure
|
We should deploy this on ci-chatmail.testrun.org before merging, to run core CI against it. @link2xt do we need to do anything for this other than |
|
This will break message previews in the new core because of unencrypted |
|
I don't want to merge this until several core members are very confident this will not break anything. We can just continue iterating and thinking about whether the test coverage we have is sufficient and what the downsides may be as we continue the journey to remove all possible plaintext metadata from relay storage. Does anyone know why we need the "From" header? |
feld
had a problem deploying
to
staging-ipv4.testrun.org
GitHub Actions
Error
feld
had a problem deploying
to
staging2.testrun.org
GitHub Actions
Error
|
could you add content-type and rebase, so we can see what fails in the cmlxc runs? |
done, let's see how it goes |
tests are going to fail due to this, so I'll add Subject back in to see if it gets further. Maybe a "refute Subject in..." will be appropriate if everything else works |
feld
force-pushed
the
lmtp_header_checks
branch
2 times, most recently
from
7354938 to
aeb33fa
Compare
hpk42
left a comment
hpk42
left a comment
There was a problem hiding this comment.
i think it's worthwhile to go for this change of having an explicit allow-list of headers, instead of blocking all unwanted ones. But i'd be more in a "if in doubt, allow it" mode for now. In particular, i think that Subject and Date better remain allowed for now, to not disrupt cleartext receival as a side effect of this allowlist approach.
This will remove all headers possible during LMTP delivery. From: required or core does not process the message correctly. Also required for cleartext compatibility. Message-Id: required for clients to know which messages have been downloaded Chat-Is-Post-Message: is required for our attachment previews Content-Type: required Subject and Date: retained for cleartext compatibility for now
|
I've just pushed an update with Date also retained
|
Co-authored-by: holger krekel <holger@merlinux.eu>
For cleartext: To, CC, In-Reply-To, References For Chatmail future expansion, allow Chat-*
|
Should we do the same for Secure Join and allow |
| /^Subject:/ DUNNO | ||
| /^Date:/ DUNNO | ||
| /^To:/ DUNNO | ||
| /^CC:/ DUNNO |
There was a problem hiding this comment.
Are these checks case-sensitive? Is Cc: accepted?
There was a problem hiding this comment.
yes, header comparisons are generally case-insensitive, and postfix defaults to case-insensitive. you have to do something special like "/^xal:/i DUNNO" if you want to have case sensitive IIRC.
| /^Chat-/ DUNNO | ||
| /^Content-Type:/ DUNNO | ||
|
|
||
| # For receiving clear-text messages (still supported in May 2026) |
There was a problem hiding this comment.
This is not only for cleartext, but any incoming encrypted messages that don't have header protection for these headers.
There was a problem hiding this comment.
yes, maybe best if you suggest a wording?
sidenote: ultimately, end-to-end encryption should require header protection, and anything else is roughly considered cleartext, as are messages today that are only encrypted and not signed. we don't need to cargo-carry forward some Autocrypt L1 scope from 8 years ago.
Yes, I think it makes sense to do the same as with |
|
There is also a comment in master.cf.j2 that needs an update: relay/cmdeploy/src/cmdeploy/postfix/master.cf.j2 Lines 94 to 96 in 39d1eca With this change we will cleanup Received headers for incoming mails, and it is not a problem because DKIM is checked now in filtermail-incoming. |
4 participants
This will remove all headers possible during LMTP delivery.
From header: required or clients do not process the message correctly
Message-Id header: required for clients to know which messages have been downloaded