rhymeswithmogul / MailPolicyExplainer

A PowerShell module to test and explain all facets of a domain's email records.
https://www.powershellgallery.com/packages/MailPolicyExplainer/
GNU Affero General Public License v3.0
9 stars 1 forks source link

Enhancement: Better handling of missing DKIM records #4

Open skyblaster opened 1 month ago

skyblaster commented 1 month ago

I'm not going to submit a PR for this one, as you may have other ideas on how to handle it.

The issue I'm having is that Microsoft (as one example) will issue a CNAME record for you to point your domain to your 365 tenant sub-domain (eg: example.onmicrosoft.com).

Once you create the CNAME record, they give you a nice green checkmark in the M365 admin center portal even when your DKIM is disabled in Microsoft Defender resulting in an empty TXT record.

Sample code:

+   If ($DnsLookup.PSObject.Properties.Name -Contains 'Answer' -and $DnsLookup.Status -eq 3)
+   {
+       Write-BadNews "DKIM selector${Name}: CNAME record exists, but resultant TXT record is empty."
+       Return
+   }

    If ($DnsLookup.PSObject.Properties.Name -NotContains 'Answer' -or $DnsLookup.Status -eq 3)
    {
        Write-BadNews "DKIM selector${Name}: This selector was not found."
        Return
    }

Example results: image

skyblaster commented 1 month ago

I feel that may be the intention of the following code, but don't think it's given the chance to run due to the earlier Return https://github.com/rhymeswithmogul/MailPolicyExplainer/blob/9faa1f2a37c0d439c6ec1f73bfcc6a610ab4b740/src/MailPolicyExplainer.psm1#L550-L555

rhymeswithmogul commented 1 month ago

I forget what my intention was with that second check. I’ll have to dig into this. I think that second test is for CNAMEs that don't point anywhere, as you describe, but I could be wrong. (I usually comment something weird like that.)

I’ll play around with this and see what I can come up with. If it's a bug in my code, I’ll correct it or handle it better.

I am familiar with Exchange Online DKIM. This module should resolve CNAMEs and work on the TXT record. There's no way to test if signing is actually enabled at the DNS level; that might be better checked or enforced with a tool like CIPP.

skyblaster commented 1 month ago

*EDIT: There is a bug below.* It allows A CNAME records to be processed as TXT records, which is obviously not ideal.**

***EDIT2: Actually it was just a specific domain returning TXT records that appear as A CNAME records. These records then trip up the token check code.

***EDIT3: Edit to edits above. I was too tired when I wrote it. I meant CNAME, not A records.

I'm not sure the following will meet your standards, but at least the logic in my testing seems sound.

    #endregion

    $DkimKeyRecord = ($DnsLookup.Answer | Where-Object type -eq 16).Data
    If ($DnsLookup.PSObject.Properties.Name -NotContains 'Answer')
    {
        Write-BadNews "DKIM selector${Name}: This selector was not found in DNS."
        Return
    }
    Else {
        Write-Verbose "DKIM selector${Name}: `"$DkimKeyRecord`""
    }

    $DkimCNAMERecord = ($DnsLookup.Answer | Where-Object type -eq 5).Data
    If ($DnsLookup.PSObject.Properties.Name -Contains 'Answer' -and $DnsLookup.Status -eq 3)
    {
        Write-BadNews "DKIM selector${Name}: Only the following CNAME record was found."
        Write-BadPractice "DKIM selector${Name}: $DkimCNAMERecord" 
        Return
    }

    #region Check for default values.
    # If there is no "k=" token, it's assumed to be "k=rsa" (per the RFC).

image