[GH-ISSUE #67] sub-domain not used in header

kerem commented

2026-02-26 10:35:25 +03:00

Owner

Originally created by @gogglespisano on GitHub (Oct 10, 2014).
Original GitHub issue: https://github.com/Pro/dkim-exchange/issues/67

I've configured example.com and sub,example.com. Both have all the required dns keys (working on a previous version).

When I send mail from user@sub.example.com the header has d=example.com, not d=sub.example.com. The earlier version sent d=sub.example.com. I've tried change between simple and relaxed with no change.

is this a bug, or a new configuration setting I've missed?

Thanks,
-Stuart

Originally created by @gogglespisano on GitHub (Oct 10, 2014). Original GitHub issue: https://github.com/Pro/dkim-exchange/issues/67 I've configured example.com and sub,example.com. Both have all the required dns keys (working on a previous version). When I send mail from user@sub.example.com the header has d=example.com, not d=sub.example.com. The earlier version sent d=sub.example.com. I've tried change between simple and relaxed with no change. is this a bug, or a new configuration setting I've missed? Thanks, -Stuart

kerem

2026-02-26 10:35:25 +03:00

closed this issue
added the
bug

agent
labels

kerem commented

2026-02-26 10:35:25 +03:00

Author

Owner

@gogglespisano commented on GitHub (Oct 21, 2014):

The problem seems to be the following code in Exchange.DkimSigner\DkimSigningRoutingAgent.cs

Beyond the sub-domain matching, it appears it would also match abc.net.example.com to abc.net which would be very wrong.

As far as I can tell, the domain list isn't sorted, so as a feature it won't necessarily pick the domain closest to the root domain of a sub-domain. It will pick the last domain in the list with a partial match.

What is the reason for using Contains()? Can == not be used?

    private void SignMailItem(MailItem mailItem)
    {

...
if (mailItem.FromAddress.DomainPart
.ToUpperInvariant()
.Contains(e.Domain.ToUpperInvariant()))
domain = e;

Also, this code doesn't change with each loop interation, so it can be moved above the forech statement.

                if (mailItem.FromAddress == null || mailItem.FromAddress.DomainPart == null)
                {
                    Logger.LogWarning("Invalid from address: '" + mailItem.FromAddress + "'. Not signing email.");
                    continue;
                }

@gogglespisano commented on GitHub (Oct 21, 2014): The problem seems to be the following code in Exchange.DkimSigner\DkimSigningRoutingAgent.cs Beyond the sub-domain matching, it appears it would also match abc.net.example.com to abc.net which would be very wrong. As far as I can tell, the domain list isn't sorted, so as a feature it won't necessarily pick the domain closest to the root domain of a sub-domain. It will pick the last domain in the list with a partial match. What is the reason for using Contains()? Can == not be used? ``` private void SignMailItem(MailItem mailItem) { ``` ... if (mailItem.FromAddress.DomainPart .ToUpperInvariant() .Contains(e.Domain.ToUpperInvariant())) domain = e; Also, this code doesn't change with each loop interation, so it can be moved above the forech statement. ``` if (mailItem.FromAddress == null || mailItem.FromAddress.DomainPart == null) { Logger.LogWarning("Invalid from address: '" + mailItem.FromAddress + "'. Not signing email."); continue; } ```

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@Pro commented on GitHub (Oct 21, 2014):

Thanks for your issue report.
That's definetly a bug and not a misconfiguration.
As you already wrote the lines

if (mailItem.FromAddress.DomainPart
.ToUpperInvariant()
.Contains(e.Domain.ToUpperInvariant()))
domain = e;

should be changed to

if (mailItem.FromAddress.DomainPart
.ToUpperInvariant()
.Equals(e.Domain.ToUpperInvariant())) {
    domain = e;
    break;
}

and the FromAddress check shoud be out of the loop.

Currently I don't have access to my workstation. I'll fix this when I'm back in one to two weeks.

@Pro commented on GitHub (Oct 21, 2014): Thanks for your issue report. That's definetly a bug and not a misconfiguration. As you already wrote the lines ``` if (mailItem.FromAddress.DomainPart .ToUpperInvariant() .Contains(e.Domain.ToUpperInvariant())) domain = e; ``` should be changed to ``` if (mailItem.FromAddress.DomainPart .ToUpperInvariant() .Equals(e.Domain.ToUpperInvariant())) { domain = e; break; } ``` and the FromAddress check shoud be out of the loop. Currently I don't have access to my workstation. I'll fix this when I'm back in one to two weeks.

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@gogglespisano commented on GitHub (Oct 21, 2014):

Thanks for taking a look and thanks in advance for the fix.

@gogglespisano commented on GitHub (Oct 21, 2014): Thanks for taking a look and thanks in advance for the fix.

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@AlexLaroche commented on GitHub (Nov 13, 2014):

@Pro: We should provide a new release before close a issue.

@AlexLaroche commented on GitHub (Nov 13, 2014): @Pro: We should provide a new release before close a issue.

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@gogglespisano commented on GitHub (Nov 13, 2014):

Do you have an ETA for a new release?

@gogglespisano commented on GitHub (Nov 13, 2014): Do you have an ETA for a new release?

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@Pro commented on GitHub (Nov 13, 2014):

It's good coding practice to close an issue as soon it is fixed within the code rather then closing it after the modified code is released.
Otherwise it's difficult to keep track which issues are already fixed (and not yet released) and which still need to be fixed.

The Issue Milestone feature can be used to keep track if the issue is already integrated into a release or not.

@gogglespisano I'll do some test and then release a version including this bugfix today or in the next few days

@Pro commented on GitHub (Nov 13, 2014): It's good coding practice to close an issue as soon it is fixed within the code rather then closing it after the modified code is released. Otherwise it's difficult to keep track which issues are already fixed (and not yet released) and which still need to be fixed. The Issue Milestone feature can be used to keep track if the issue is already integrated into a release or not. @gogglespisano I'll do some test and then release a version including this bugfix today or in the next few days

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@Pro commented on GitHub (Nov 27, 2014):

@gogglespisano today I released version 2.1.0 which fixes this issue.

@Pro commented on GitHub (Nov 27, 2014): @gogglespisano today I released version 2.1.0 which fixes this issue.

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@gogglespisano commented on GitHub (Nov 30, 2014):

I had problems installing 2.1.2. I was upgrading from 2.0.3.

It failed during the zip extraction because of file in use. This happening using the GUI update or downloading and installing from the downloaded files. I had to disable the signer, then upgrade, then enable the signer.

@gogglespisano commented on GitHub (Nov 30, 2014): I had problems installing 2.1.2. I was upgrading from 2.0.3. It failed during the zip extraction because of file in use. This happening using the GUI update or downloading and installing from the downloaded files. I had to disable the signer, then upgrade, then enable the signer.

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@Pro commented on GitHub (Nov 30, 2014):

Hm, this looks like the MSExchangeTransport wasn't stopped properly.
First the files are unzipped into temp, then the service is stopped and then the files are copied.
Are you sure that zip extraction step failed, or may it have been the copy step? I tested on my machine and it worked fine. I think version 2.1.2 should fix this.

(For the next time please open a new issue instead of replying to an old one, this makes it easier to keep track of different issues, thanks :) )

@Pro commented on GitHub (Nov 30, 2014): Hm, this looks like the MSExchangeTransport wasn't stopped properly. First the files are unzipped into temp, then the service is stopped and then the files are copied. Are you sure that zip extraction step failed, or may it have been the copy step? I tested on my machine and it worked fine. I think version 2.1.2 should fix this. (For the next time please open a new issue instead of replying to an old one, this makes it easier to keep track of different issues, thanks :) )

kerem commented

2026-02-26 10:35:26 +03:00

Author

Owner

@gogglespisano commented on GitHub (Nov 30, 2014):

I entered a new issue for it.

The good news is my email is now verifying with the correct domain name ☺

Thanks,
-Stuart

From: Stefan Profanter [mailto:notifications@github.com]
Sent: Sunday, November 30, 2014 1:06 PM
To: Pro/dkim-exchange
Cc: Stuart Wyatt
Subject: Re: [dkim-exchange] sub-domain not used in header (#67)

Hm, this looks like the MSExchangeTransport wasn't stopped properly.
First the files are unzipped into temp, then the service is stopped and then the files are copied.
Are you sure that zip extraction step failed, or may it have been the copy step? I tested on my machine and it worked fine. I think version 2.1.2 should fix this.

(For the next time please open a new issue instead of replying to an old one, this makes it easier to keep track of different issues, thanks :) )

—
Reply to this email directly or view it on GitHubhttps://github.com/Pro/dkim-exchange/issues/67#issuecomment-65000291.

@gogglespisano commented on GitHub (Nov 30, 2014): I entered a new issue for it. The good news is my email is now verifying with the correct domain name ☺ Thanks, -Stuart From: Stefan Profanter [mailto:notifications@github.com] Sent: Sunday, November 30, 2014 1:06 PM To: Pro/dkim-exchange Cc: Stuart Wyatt Subject: Re: [dkim-exchange] sub-domain not used in header (#67) Hm, this looks like the MSExchangeTransport wasn't stopped properly. First the files are unzipped into temp, then the service is stopped and then the files are copied. Are you sure that zip extraction step failed, or may it have been the copy step? I tested on my machine and it worked fine. I think version 2.1.2 should fix this. (For the next time please open a new issue instead of replying to an old one, this makes it easier to keep track of different issues, thanks :) ) — Reply to this email directly or view it on GitHubhttps://github.com/Pro/dkim-exchange/issues/67#issuecomment-65000291.

Rows
Columns

[GH-ISSUE #67] sub-domain not used in header #50