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

I think this is a bug or other wise a question from a noob person (me) #1000

Closed
Sicos1977 opened this issue Feb 2, 2024 · 32 comments
Closed
Labels
question A question about how to do something

Comments

@Sicos1977
Copy link

I'm using the latest MimeKit version.

I'm trying to verify if a DKIM signature is valid.

The message that I have recieved has got this signature (I removed some sensitive parts)

DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xxxxxx.nl;
	s=dkim_public1; t=1706864343;
	bh=MEzZj4qwq/jdFy7B4VdgJpWXPiZ11g4gak3QfQ4HX0g=;
	h=From:To:Date:Subject;
	b=xxxxxxxxxxxxxxxxxxxxx.....

As far as I can see the request is to verify the From, To, Date and Subject.

    public bool? DKimSignature
    {
        get
        {
            if (Headers.DKimSignature == null)
                return null;

            var dkimVerifier = new DkimVerifier(new DkimPublicKeyLocator());
            var header = new MimeKit.Header(HeaderId.DkimSignature, Headers.DKimSignature);

            try
            {
                var test = dkimVerifier.Verify(MimeMessage, header);
            }
            catch (Exception exception)
            {
                //ErrorLogs.Insert(this, exception);
                return false;
            }

            return true;
        }
    }

But when I try to verify the e-mail like this, it always fails because of this line of code.

		/// <summary>
		/// Verify the hash of the message body.
		/// </summary>
		/// <remarks>
		/// Verifies the hash of the message body.
		/// </remarks>
		/// <param name="options">The formatting options.</param>
		/// <param name="message">The signed MIME message.</param>
		/// <param name="signatureAlgorithm">The algorithm used to sign the message.</param>
		/// <param name="canonicalizationAlgorithm">The algorithm used to canonicalize the message body.</param>
		/// <param name="maxLength">The max length of the message body to hash or <c>-1</c> to hash the entire message body.</param>
		/// <param name="bodyHash">The expected message body hash encoded in base64.</param>
		/// <returns><c>true</c> if the calculated body hash matches <paramref name="bodyHash"/>; otherwise, <c>false</c>.</returns>
		protected static bool VerifyBodyHash (FormatOptions options, MimeMessage message, DkimSignatureAlgorithm signatureAlgorithm, DkimCanonicalizationAlgorithm canonicalizationAlgorithm, int maxLength, string bodyHash)
		{
			var hash = Convert.ToBase64String (message.HashBody (options, signatureAlgorithm, canonicalizationAlgorithm, maxLength));

			return hash == bodyHash;
		}

The code returns false because the body hash is not the same. But as far as I understand the request it to verify the From, To, Date and Subject ... or am I missing something.

I'm rather new with verifying dkim signatures.

@jstedfast
Copy link
Owner

There's 2 parts of a DKIM signature verification:

  1. Verify that the hash of the body matches the bh= value
  2. Verify that the signed hash of the headers matches the b= value

(I might have the b and bh values backwards, it's been a while since I read over the spec - but that's the basic idea at least)

The signature in the b= value also includes the bh= value in that the algorithm first needs to calculate the bh= value and then combines that with the hash of the other content, and then that result is signed.

Hopefully that answers your question about why the VerifyBodyHash() method is being called.

As to why the BodyHash doesn't match, that's the million dollar question. It might be that the body was modified or it might be that MimeKit isn't correctly canonicalizing the message body.

@Sicos1977
Copy link
Author

Sicos1977 commented Feb 3, 2024

Hi,

I was playing around with the Dkim verification and did see that there is also an async overload method that accepts a FormatOptions class.

When I play around with settings in this class then I get different hash values for the body... so the question is ... how do I know what the correct settings are?

var formatOptions = new FormatOptions
{
    AllowMixedHeaderCharsets = true,
    AlwaysQuoteParameterValues = true,
    International = true,
    MaxLineLength = 100,
    ParameterEncodingMethod = ParameterEncodingMethod.Rfc2047
};

_dKimSignatureIsValid = dkimVerifier.VerifyAsync(formatOptions, MimeMessage, header).GetAwaiter().GetResult();

Reason for asking is because I cant figure out why my Dkim verification always returns false.

@Sicos1977
Copy link
Author

I'm getting the e-mails from Exchange like this (with EWS).

It is just getting the byte array out of the soap message and returning that as a stream. This way there should be no modifications on the data.

            case MailBoxType.EwsBasicAuthentication:
            case MailBoxType.EwsApplicationAuthentication:
            {
                try
                {
                    WriteProgress($"Connecting to Exchange Web Services and getting e-mail with id '{uniqueId}'");
                    var item = Item.Bind(ExchangeService, new ItemId(uniqueId));
                    item.Load(new PropertySet(ItemSchema.Subject, ItemSchema.MimeContent));
                    result.FileName = FileManager.RemoveInvalidFileNameChars(item.Subject) + ".eml";
                    var messageStream = StreamHelpers.Manager.GetStream("MailBoxes.GetMessage");
                    var content = item.MimeContent.Content;
                    messageStream.Write(content, 0, content.Length);
                    messageStream.Position = 0;
                    result.Stream = messageStream;
                    WriteProgress(Resources.Progress_Done);
                    break;
                }
                catch (Exception exception)
                {
                    throw new MailBoxException(exception.Message + Environment.NewLine + _lastEwsMessage);
                }
                finally
                {
                    _lastEwsMessage = null;
                }
            }

@jstedfast
Copy link
Owner

Hmmm, I recently made it so that users can override the FormatOptions.Default values and FormatOptions.Default was what was used for this by default.

Ok, so... you are going to want the following options:

var formatOptions = new FormatOptions
{
    NewLineFormat = NewLineFormat.Dos,
    International = false,
};

I need to do some digging to see what else gets used. The MaxLineLength might get used (but if it does, that needs to be fixed). By default, it is 72.

@Sicos1977
Copy link
Author

Sicos1977 commented Feb 3, 2024

I tried these but it still fails. I looked inside the message and did see the Authentication Result header and it had these values in it.

authentication-results: spf=pass (sender IP is 40.107.1.126)
 smtp.mailfrom=xxxxx.nl; xxxxx.nl; dkim=pass (signature was
 verified) header.d=xxxxx.nl;xxxx.nl; dmarc=pass action=none
 header.from=xxxx.nl;compauth=pass reason=100
dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xxxxx.nl;
 s=selector1;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=HB23qgOfUkdPvdLPZk4q5Y/2X/nK1tdSSdPGiTaJoR0=;
 b=Exo2oHKW+lmGGNgy9K6QApgF4Z464MiePeD1G84hmYn86+pV3VDgtSgMYx0q57OzrE95G2rp0E5ld3/wnE2DEmcXYlFRpJiTW/RvoVdiLGyHSLqhytKJUWpOLVFQzFw/P0CWDcHdYjQ1lYVKz6i4R5eZx4stCh/XG/+SE0w/7XI=

So I think office 365 is somehow modifying the body of the email so that the DKim verification fails... but because there is a dkim=pass in the authenticatio-result header it seems that office 365 already validated if the signature was correct. So I'm now using that value to decide if the message is valid or not.

I really have no idea what office 365 is doing with the e-mail so that the body is failing.

So I'm now doing it like this. I'm using your AuthenticationResults class to parse that header and then copy the values from it to my own class that I need to extend with other things.

    /// <summary>
    ///    Returns the authentication results as a <see cref="AuthenticationResults"/> object or
    ///    <c>null</c> when the string could not be decoded
    /// </summary>
    public AuthenticationResults AuthenticationResults 
    {
        get
        {
            if (_authenticationResults != null)
                return _authenticationResults;

            if(MimeKit.Cryptography.AuthenticationResults.TryParse(Encoding.ASCII.GetBytes(AuthenticationResultsRaw), out var results))
               _authenticationResults = new AuthenticationResults(results);

            return _authenticationResults;
        }
    }
    /// <summary>
    ///     Retourneert <c>true</c> als de <see cref="Headers"/> een <see cref="Headers.DKimSignatureRaw"/> bevat" en
    ///     deze geldig is, bevat deze een signature maar is deze niet geldig dan wordt <c>false</c> geretourneerd.
    /// </summary>
    /// <remarks>
    ///     <c>null</c> wordt geretourneerd als er geen <see cref="Headers.DKimSignatureRaw"/> is gevonden
    /// </remarks>
    public bool? DKimSignatureIsValid
    {
        get
        {
            if (Headers.DKimSignatureRaw == null)
                return null;

            if (_dKimSignatureIsValid.HasValue)
                return _dKimSignatureIsValid.Value;

            // Check if the DKIM signature is already validated
            var result = Headers.AuthenticationResults.Results.Find(m => m.Method.Equals("dkim", StringComparison.InvariantCultureIgnoreCase));
            if (result is { Result: "pass" })
            {
                _dKimSignatureIsValid = true;
                return _dKimSignatureIsValid.Value;
            }
            
            var dkimVerifier = new DkimVerifier(new DkimPublicKeyLocator());
            var header = new Header(HeaderId.DkimSignature, Headers.DKimSignatureRaw);

            try
            {
                var formatOptions = new FormatOptions
                { 
                    NewLineFormat = NewLineFormat.Dos,
                    MaxLineLength = 72,
                    International = false
                };

                _dKimSignatureIsValid = dkimVerifier.VerifyAsync(formatOptions, MimeMessage, header).GetAwaiter().GetResult();
            }
            catch (Exception exception)
            {
                ErrorLogs.Insert(this, exception);
                _dKimSignatureIsValid = false;
            }

            return _dKimSignatureIsValid.Value;
        }
    }

jstedfast added a commit that referenced this issue Feb 3, 2024
Now that users can modify FormatOptions.Default, we need to be smarter
about whether or not we need to reformat header values in
Header.GetRawValue().

The old logic used to assume that cached raw values would always use
FormatOptions.International=False unless they were explicitly set, but
now that FormatOptions.Default.International can be True, we need to cache
that value and determine if it has changed since the rawValue was last
cached.

Realistically, we should be caching other FormatOptions values as well,
but this was the most important one (and the only one that was used to
determine whether the rawValue needed to be reformatted or not).

Also modified the ARC and DKIM verifier logic to share code that clones
the user-requested FormatOptions to override certain values like
NewLineFormat to also set VerifyingSignature = True.

This *may* solve issue #1000
@jstedfast
Copy link
Owner

Give the latest MimeKit code a try and see if that fixes it. If not, I'm not sure we can assume that Exchange necessarily broke something unless other DKIM messages verify ok.

@jstedfast
Copy link
Owner

Since you are playing with DKIM, you might be interested in this video that my wife just sent me: https://www.youtube.com/watch?v=NwnT15q_PS8

Pretty crazy.

@Sicos1977
Copy link
Author

I somebody in the future reads this and is wondering how to get a public DKIM key from DNS... this is how I did do it.

LookupClient --> https://dnsclient.michaco.net/docs/DnsClient.LookupClient.html

/// <summary>
///     <see cref="IDkimPublicKeyLocator" /> implementation which uses DNS to locate the public key
/// </summary>
internal class DkimPublicKeyLocator : IDkimPublicKeyLocator
{
    #region LocatePublicKey
    public AsymmetricKeyParameter LocatePublicKey(string methods, string domain, string selector, CancellationToken cancellationToken = new())
    {
        return GetDkimAsymmetricKeyParameter(domain, selector);
    }

    public Task<AsymmetricKeyParameter> LocatePublicKeyAsync(string methods, string domain, string selector, CancellationToken cancellationToken = new())
    {
        return Task.FromResult(GetDkimAsymmetricKeyParameter(domain, selector));
    }
    #endregion

    #region GetDkimAsymmetricKeyParameter
    private static AsymmetricKeyParameter GetDkimAsymmetricKeyParameter(string domain, string selector)
    {
        var client = EmailHelpers.DnsLookupClient;
        var result = client.Query($"{selector}._domainkey.{domain}", QueryType.TXT);
        var record = result.Answers.TxtRecords().FirstOrDefault();
        
        if (record is null)
            throw new EmailException("No DKIM public key found");
        
        var text = string.Join(string.Empty, record.Text);
        return new DkimPublicRecord(text).PublicKey;
    }
    #endregion
}
/// <summary>
///     Represents a public DKIM record
/// </summary>
internal class DkimPublicRecord
{
    #region Properties
    /// <summary>
    ///     Returns the version of the DKIM record
    /// </summary>
    public string Version { get; }

    /// <summary>
    ///     Returns the public key of the DKIM record as a base64 string
    /// </summary>
    public string PublicKeyRaw { get; }

    /// <summary>
    ///     Returns the public key of the DKIM record as an <see cref="AsymmetricKeyParameter" /> or
    ///     <c>null</c> when the public key is invalid
    /// </summary>
    public AsymmetricKeyParameter PublicKey
    {
        get
        {
            var keyBytes = Convert.FromBase64String(PublicKeyRaw);
            return PublicKeyFactory.CreateKey(keyBytes);
        }
    }
    #endregion

    #region Constructor
    /// <summary>
    ///     Makes this object and sets it's needed properties
    /// </summary>
    /// <param name="record"></param>
    internal DkimPublicRecord(string record)
    {
        var parts = record.Split(';');

        foreach (var part in parts)
        {
            var subParts = part.Split('=');
            if (subParts.Length != 2)
                continue;

            var key = subParts[0].Trim().ToLower();
            var value = subParts[1].Trim();

            switch (key)
            {
                case "v":
                    Version = value;
                    break;
                
                case "p":
                    PublicKeyRaw = value;
                    break;
            }
        }
    }
    #endregion
}

@Sicos1977
Copy link
Author

Since you are playing with DKIM, you might be interested in this video that my wife just sent me: https://www.youtube.com/watch?v=NwnT15q_PS8

Pretty crazy.

Thanks... going to watch that this evening when I have some youtube time.

Already doing things with e-mail for the past 20 years and still learning new things. To bad it is sometimes so damn hard to find good documentation. There are enough RFC docs but these are sometimes so damn hard to read :-) ... but I guess that is the fate of a programmer

@jstedfast
Copy link
Owner

FWIW, there is also an implementation of a DkimPublicKeyLocator in the samples: https://github.com/jstedfast/MimeKit/blob/master/samples/DkimVerifier

@Sicos1977
Copy link
Author

Seemed to have missed that example.. I searched through your testcases but missed this one. But I was already using the LookUpClient to validate e-mail adresses ... that is why I did it that way.

@Sicos1977
Copy link
Author

I tested your latest commit by even then the body hash keeps failing ... so I guess office 365 is doing something with the body. Probably Microsoft that is thinking that it is good to modify it... whats new :-)

I'll close this issue because it works for me by checking the Authentication-Result header..

Thanks for your help and have a nice weekend.

@jstedfast
Copy link
Owner

If the particular message that DkimVerifier is failing on isn't private/personal or if you trust me with using the message to debug this issue, let me know and I can try debugging things to see if I can figure out what the issue is.

This type of thing is pretty fragile, unfortunately, because it requires that MimeKit be able to perfectly serialize every part of the message exactly the way it was before parsing. This is a pretty frustratingly difficult thing to try and do, so it's possible that this message hits some sort of corner case that MimeKit doesn't handle correctly.

@jstedfast
Copy link
Owner

Ugh, looks like my changes actually broke DKIM and other unit tests :(

@Sicos1977
Copy link
Author

If the particular message that DkimVerifier is failing on isn't private/personal or if you trust me with using the message to debug this issue, let me know and I can try debugging things to see if I can figure out what the issue is.

This type of thing is pretty fragile, unfortunately, because it requires that MimeKit be able to perfectly serialize every part of the message exactly the way it was before parsing. This is a pretty frustratingly difficult thing to try and do, so it's possible that this message hits some sort of corner case that MimeKit doesn't handle correctly.

I trust you but can't share the e-mail because it people personal information in it. I'll try to find a test e-mail in our test and development system that has the same problem. Can share those with you without any problems.

Thank your wife for me... I watched the youtube link... I also never heard of mailchannels... it was funny to watch.

@Sicos1977
Copy link
Author

I found a mail in our test system that I can share with you. Don't want to post it here because there is routing information in the headers like ip numbers and server names. Do you have a e-mail address that I can use to sent you the file?

@Sicos1977
Copy link
Author

The mail that I can sent also fails on the body hash so personally I think O365 is doing something with the mail after that it has verified the DKIM signature itself.

image

@jstedfast
Copy link
Owner

You can email me at jestedfa@microsoft.com

I would recommend zipping up the email to minimize risk of the attachment getting modified in transit.

@Sicos1977
Copy link
Author

You can email me at jestedfa@microsoft.com

I would recommend zipping up the email to minimize risk of the attachment getting modified in transit.

You have mail... I always zip e-mails... I also always ask it from other people when they file an issue when something is not working on my MSGReader project. Microsoft has the bad habbit in modifying e-mails when sending them :-)

@jstedfast
Copy link
Owner

jstedfast commented Feb 4, 2024

Microsoft has the bad habbit in modifying e-mails when sending them :-)

Exactly :-)

I'll work on trying to figure this out, but it may take me a few days as I poke around for an hour here and an hour there as I have moments of down-time that I can use for hacking on MimeKit & MailKit.

BTW, I received the email just so you know.

@Sicos1977
Copy link
Author

I reopened the issue so that I can keep track on this conversation... close it whenever you want.

@Sicos1977 Sicos1977 reopened this Feb 4, 2024
@jstedfast
Copy link
Owner

Okay, got myself a little test case and I can confirm that the calculated hash != the expected hash from the DKIM-Signature header.

Now it's just a matter of figuring out why.

@Sicos1977
Copy link
Author

I did sent you a second e-mail ... it has the exact same problem. I personally don't see anything weird inside the e-mail.

@jstedfast
Copy link
Owner

I saw, thanks.

I've been digging around and so far I have not been able to spot anything obviously wrong with the "relaxed" body canonicalization output, it looks correct.

I also tried forcing it to use the "simple" algorithm but that also gave a different hash result.

The "relaxed" output only appeared to remove 1 space character (which is all it is supposed to do).

Taking the dog out for a walk and then I gotta head out with the wife on some errands, but I think my next step will be to verify line endings in a hex editor.

If that is correct, then it may really be that the message was modified somehow.

On the plus side, I did see a minor optimization that I could do in the RelaxedBodyFilter.

@Sicos1977
Copy link
Author

Have a nice walk.. I'm going to hit the shower because it is already evening over here so preparing for bed time

@jstedfast
Copy link
Owner

jstedfast commented Feb 7, 2024

hmmm, I can't get it to pass DKIM verification nor can I figure out what is off about the body canonicalization.

How did you save this message? Did you save the original System.IO.Stream content of the message? Or did you parse it as a MimeMessage and then save the MimeMessage?

If you parsed it with MimeKit to get a MimeMessage and then saved that, I wonder if maybe that is the source of modification hence why the DKIM signature isn't verifying?

I just want to make sure we're touching all of the bases here.

It's possible that there's a bug in MimeMessage, Multipart, or MimePart that is somehow altering the message data when it gets saved if you are using message.WriteTo() to save the message that we are trying to verify.

@Sicos1977
Copy link
Author

I'll try to look into this tomorrow... was a little bit busy with other things this week.

@jstedfast jstedfast added the need-info More information is needed to diagnose the issue. label Feb 9, 2024
@Sicos1977
Copy link
Author

Sicos1977 commented Feb 10, 2024

Sorry still didn't find any time... probably next week. The problem of working on to many projects at the same time.

@Sicos1977
Copy link
Author

Sicos1977 commented Feb 13, 2024

I get the message from the mailbox like this;

  • Bind to the exchange mailbox
  • Get the message with its unique id ... this will return the e-mail as a base64 encoded byte array in the soap message
  • Write the byte array to a memorystream and return that stream to the client
var folder = WebHelpers.GetRequestKey(Request, "folder", true);
var message = await mailBox.GetMessageAsync(folder, uniqueId);
        
await Response.StreamToClientAsync(message.FileName, message.Stream, false);

....

case MailBoxType.EwsBasicAuthentication:
case MailBoxType.EwsApplicationAuthentication:
{
    try
    {
        WriteProgress($"Connecting to Exchange Web Services and getting e-mail with id '{uniqueId}'");
        var item = Item.Bind(ExchangeService, new ItemId(uniqueId));
        item.Load(new PropertySet(ItemSchema.Subject, ItemSchema.MimeContent));
        result.FileName = FileManager.RemoveInvalidFileNameChars(item.Subject) + ".eml";
        var messageStream = StreamHelpers.Manager.GetStream("MailBoxes.GetMessage");
        var content = item.MimeContent.Content;
        messageStream.Write(content, 0, content.Length);
        messageStream.Position = 0;
        result.Stream = messageStream;
        WriteProgress(Resources.Progress_Done);
        break;
    }
    catch (Exception exception)
    {
        throw new MailBoxException(exception.Message + Environment.NewLine + _lastEwsMessage);
    }
    finally
    {
        _lastEwsMessage = null;
    }
}

@jstedfast
Copy link
Owner

Right, you said this in an earlier comment.

What I'm asking is: the raw message that you saved that we are using to diagnose this issue - how did you save that? Is that just the stream that you returned from the method above? Or did it get parsed and then saved? Or did you download it using Outlook?

I just want to make sure that MimeKit couldn't have "corrupted" the message text file that we are using.

@Sicos1977
Copy link
Author

Sicos1977 commented Feb 13, 2024

The Stream was saved directly to file... so not tool that added or removed something like line ending.

The StreamToClient methods sends the stream directly to the browser so that the user (in this case me) gets a prompt to save the file to disk.

@jstedfast
Copy link
Owner

Awesome, thanks, that's what I needed to know :-)

Alright, so... given that - I think we can safely say that this doesn't appear to be a MimeKit DKIM verifier bug. I was also unable to verify the DKIM-Signature using other tools on this message file.

@jstedfast jstedfast added question A question about how to do something and removed need-info More information is needed to diagnose the issue. labels Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question A question about how to do something
Projects
None yet
Development

No branches or pull requests

2 participants