Skip to content

fix: always prefer the last header #7000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 14, 2025
Merged

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Jul 13, 2025

Headers are normally added at the top of the message, e.g. when forwarding new Received headers are
added at the top.

When headers are protected with DKIM-Signature
and oversigning is not used,
forged headers may be added on top
so headers from the top are generally less trustworthy.

This is tested with test_take_last_header,
but so far last header was only preferred
for known headers. This change extends
preference of the last header to all headers.

@link2xt
Copy link
Collaborator Author

link2xt commented Jul 13, 2025

Unless I overlook something, probably makes sense to merge this first and rebase #6370 on top of this. This seems to be in line with https://datatracker.ietf.org/doc/draft-ietf-lamps-header-protection/ (future RFC 9788), protected headers should always be preferred over non-protected ones.

Headers are normally added at the top of the message,
e.g. when forwarding new `Received` headers are
added at the top.

When headers are protected with DKIM-Signature
and oversigning is not used,
forged headers may be added on top
so headers from the top are generally less trustworthy.

This is tested with `test_take_last_header`,
but so far last header was only preferred
for known headers. This change extends
preference of the last header to all headers.
@link2xt link2xt force-pushed the link2xt/prefer-last-header branch from f4d20bb to 1c97d11 Compare July 13, 2025 16:34
@@ -2009,28 +2005,6 @@ pub(crate) fn parse_message_id(ids: &str) -> Result<String> {
}
}

/// Returns true if the header overwrites outer header
/// when it comes from protected headers.
fn is_known(key: &str) -> bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is needed anyway because i want to use it in #6370 (because protected headers must be removed from the IMF section. Currently it's done using the list of headers under the if !signatures.is_empty() condition, but that list is incomplete and that condition doesn't handle signed-only messages).

But it's renamed and modified by that PR anyway, so for now it's fine to remove it

@link2xt link2xt merged commit e6fd52a into main Jul 14, 2025
29 checks passed
@link2xt link2xt deleted the link2xt/prefer-last-header branch July 14, 2025 14:57
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