Skip to content
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

Fix mimebody DKIM body-hash computation #923

Merged
merged 2 commits into from Jan 28, 2024

Conversation

birktj
Copy link
Contributor

@birktj birktj commented Dec 5, 2023

This PR fixes the DKIM body-hash computation of singlepart and multipart bodies.

I opted for a fn body_raw(&self) -> Vec<u8> method for these bodies, but if it would be preferable with a fn format_body(&self, out: &mut Vec<u8>) instead I could do that change.

This fixes #922

@paolobarbolini
Copy link
Member

Thanks for the PR, and sorry for the delay in reviewing. I don't mind the extra allocations, although it shouldn't be too much hard to remove them. Can you share the document that you used to determine that this is the right way of signing the bodies?

@birktj
Copy link
Contributor Author

birktj commented Jan 25, 2024

Thanks for having a look.

There shouldn't be have been any extra allocations the way the code was, but there could be in the future if body_raw ends up being used for something else. A format_body function would also reduce code duplication a bit as it could be reused in MultiPart::format and SinglePart::format. Thinking more about it I think it is a nicer solution so I have implemented this change now.

I am pretty sure that I used RFC 6376 as a reference, but note that this code does the exact same as the existing code, just making sure to not include the multipart headers when computing the body hash.

Copy link
Member

@paolobarbolini paolobarbolini left a comment

Choose a reason for hiding this comment

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

LGTM

@paolobarbolini paolobarbolini merged commit 12580d8 into lettre:master Jan 28, 2024
6 checks passed
@paolobarbolini paolobarbolini mentioned this pull request Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Bug in DKIM body-hash computation with mimebodies
2 participants