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

Enhancement: Better handling of missing DKIM records #4

Open
skyblaster opened this issue May 25, 2024 · 3 comments
Open

Enhancement: Better handling of missing DKIM records #4

skyblaster opened this issue May 25, 2024 · 3 comments

Comments

@skyblaster
Copy link
Contributor

skyblaster commented May 25, 2024

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
Copy link
Contributor Author

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

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

@rhymeswithmogul
Copy link
Owner

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
Copy link
Contributor Author

skyblaster commented May 29, 2024

***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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants