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

Error when validating signature (message-digest attribute value does not match calculated value) #569

Closed
mikkelbm opened this issue May 18, 2020 · 18 comments
Labels
bug Something isn't working compatibility Compatibility with existing software

Comments

@mikkelbm
Copy link

mikkelbm commented May 18, 2020

We are using your great MimeKit to handle AS2 messages in our software.
And it's running smoothly for almost all partners (and we have a lot). But we have an issue with one of them (OpenText/GXS) - we can't validate their signature on the incoming mime-message.

I have tried a lot of different debugging, but I can't figure out why we are getting the following error:
"message-digest attribute value does not match calculated value"

My first thoughts was, that they (the external partner OpenText) was doing something wrong, but I then downloaded some Java AS2 software (OpenAS2 https://github.com/phax/as2-lib) and another .NET library (Chilkat) - and both of them can validate the signature of the message.

So I'm stuck here... Hopefully you can tell me, if I'm doing something wrong - or if there is something wrong with the mime-message.

Platform (please complete the following information):

  • OS: Windows
  • .NET Framework: 4.5+
  • MimeKit Version: 2.6

To Reproduce
Steps to reproduce the behavior:
I have create a small unit-test solution in VS2019, where I show the issue with MimeKit and a test-method showing that it works with Chilkat:
UnitTestProject_Mimekit.zip

  1. Download the zip-file and unpack it
  2. Restore nuget packages in the solution
  3. Run the two test-methods in the test-class

Expected behavior
I would expect the message to validate, since other frameworks validates the message. And OpenText are quite big, so I think they have a lot of partners where this is not an issue.

Screenshots
image

Additional context
I can see that the mime-message I'm working with from OpenText have a mix of Carriage-Return and Line-Feed. The mime-message contains both the carraige-return and line-feed (as it should), but the actual payload (the body) is using only line-feed (unix style). See the screenshot from notepad++ above.
And I can see, that when I use the WriteTo method you provide, it will change the actual content of the body depending on the FormatOptions. I don't know if this can be causing this issue?

@mikkelbm
Copy link
Author

I have just played around with the SignedCms (from the System.Security.Cryptography.Pkcs namespace), and I can actually made that validate the signature as well with the following test-method:

    [TestMethod]
    public void TestMethod_VerifySignatureWithDotNetSignedCms()
    {
        var stream = LoadResource("UnitTestProject_Mimekit.Files.AS2In_RawDecryptedData.txt");
        var message = MimeMessage.Load(stream);

        var multipart = message.Body as MultipartSigned;
        var contentMemStream = new MemoryStream();
        ((MimePart)multipart[0]).Headers.WriteTo(contentMemStream);
        ((MimePart)multipart[0]).Content.Stream.CopyTo(contentMemStream); // Don't use the WriteTo method because it will change the actual payload

        var signatureMemStream = new MemoryStream();
        ((MimePart)multipart[1]).Content.Stream.CopyTo(signatureMemStream);

        var contentInfo = new ContentInfo(contentMemStream.ToArray());
        var signedCms = new SignedCms(contentInfo, true);

        signedCms.Decode(signatureMemStream.ToArray());
        signedCms.CheckHash();
        signedCms.CheckSignature(true); // Throws an exception if validation fails...
    }

I don't know if that can help?

@jstedfast
Copy link
Owner

Hi @mikkelbm,

It looks like you've dug into this quite a bit and my guess is that the mix of LF vs CRLF probably is the culprit based on your latest comment just as you suspect.

I can definitely verify the error you are seeing (thanks for the test case!), but I haven't dig into it yet (will try to do it tonight after work).

I also noticed that you are bypassing chain validation. Perhaps I can add an argument to IDigitalSignature.Verify() to make this easier as well?

@mikkelbm
Copy link
Author

mikkelbm commented May 18, 2020

Hi @jstedfast,

Thanks for your quick reply...

An overload to the verify method, that can bypass chain-validation, will be quite cool for us.
Almost all partners are using self-signed certificates, and don't provide a root-certificate - that is why we need to by-pass the validation of the chain.

jstedfast added a commit that referenced this issue May 18, 2020
@jstedfast
Copy link
Owner

No problem ;-)

Feature added. Will dig into the main issue tonight. I'll need to figure out the best way to handle the mixed line ending scenario. One option might be to add a NewLineFormat.AsIs option to the FormatOptions, but maybe I'll be able to think of a better way.

@jstedfast
Copy link
Owner

Okay, I actually have a fix for this (altho it's a bit of a hack).

I'm using the fact that the content in this example being verified does not have a Content-Transfer-Encoding header and treating parts like that as if they held binary content (aka Content-Transfer-Encoding: binary) thereby preventing canonicalization of the line endings.

I might need to implement a better fix long-term, but this may be good enough.

Do all of the cases that fail have a missing Content-Transfer-Encoding?

Also: is it safe for me to include the test file you added to this bug report in MimeKits unit tests?

jstedfast added a commit that referenced this issue May 18, 2020
@jstedfast
Copy link
Owner

BTW, thanks a ton for your generous donation. That was quite a surprise and very appreciated ;-)

@mikkelbm
Copy link
Author

Good catch with the content-transfer-encoding. I haven't thought of that...
And we can absolutely live with that "hack" :-)

I've just tested it - and it works very well! Both the actual signature validation and skipping the chain validation.

For us it's actually not a hack, since we treat all AS2 payload as binary until it reaches our EDI engine (although the payload is normally EDIFACT/X12 or XML, but we handle that later on in the process).
Hopefully this won't break anything for other users of MimeKit.

And yes, it looks like the CTE is missing in all the failed transmissions from OpenText.

You are welcome to use the test file in your unit tests. There are no sensitive data in that file.

Do you have any upcoming release of MimeKit scheduled?

@jstedfast
Copy link
Owner

Awesome! I'm glad that works for you :-)

The only change I might make (just thought of this a few minutes ago) is that MimeKit should probably still canonicalize the content if the Content-Type is text/plain or text/html, etc (text/[something]). That was going to be my next question to verify with you. Sounds like that change would also be safe for you.

And the more I thought about my fix, the less I thought of it as a hack as well since a missing Content-Transfer-Encoding probably should generally be treated as binary, with the exception (perhaps) of text/* parts.

Let me know if canonicalizing Content-Type: text/* parts would break you, and I'll go with my current fix instead.

I'll try to push a new release today.

@mikkelbm
Copy link
Author

I have just been through a wide selection of our incoming documents, and all of them have application/*something* as Content-Type. And all of them (except for OpenText) have "binary" as Content-Transfer-Encoding.
So for us it will be safe to canonicalize the content, if the Content-Type is text/*.

@jstedfast
Copy link
Owner

Thanks for the verification! I'm writing up the release notes right now.

@jstedfast
Copy link
Owner

Just published MimeKit 2.7.0 to nuget.org

@mikkelbm
Copy link
Author

We have updated the references, and everything is working again :-)

Thanks a lot for the quick response and action!

/Mikkel

@jstedfast
Copy link
Owner

You're welcome!

@jstedfast jstedfast added bug Something isn't working compatibility Compatibility with existing software labels May 23, 2020
@jstedfast
Copy link
Owner

I may have an idea for a better fix for this...

I'm thinking that if I can make the parser track the original new-line format of the content of a MimePart, that when a MimePart is written back out (as part of the process of cryptographically verifying a multipart/signed part), then if the original new-line format is the same as the new-line format needed for verifying (aka DOS format), then there's no need to force-canonicalize the content allowing whatever is there to just pass-through w/o modification.

jstedfast added a commit that referenced this issue May 23, 2020
…d line endings

This is a better fix for issue #569
jstedfast added a commit that referenced this issue May 23, 2020
…-endings

Modified parser to track NewLineFormat of content and set it on the MimeContent
object so that MimePart.WriteTo() can use that information (along with a new
FormatOptions.VerifyingSignature property) to determine if it should avoid
canonicalizing the content line endings.

Fixes issue #569 in a better way.
@mikkelbm
Copy link
Author

That could actually be quite cool, if you can detect that without compromising performance.

@jstedfast
Copy link
Owner

MimeKit 2.8.0 has just been released with the line-ending tracking changes - let me know if this re-breaks things for you. I don't expect it will break (the test you gave me before still passes), so everything should be ok, but in the event that it does, let me know, and I'll fix ASAP and push a 2.8.1 release or something.

@mikkelbm
Copy link
Author

mikkelbm commented Jun 2, 2020

My tests complets successful with 2.8 as well, so everythings seems to be ok :)

@jstedfast
Copy link
Owner

Awesome :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compatibility Compatibility with existing software
Projects
None yet
Development

No branches or pull requests

2 participants