Skip to content

Commit

Permalink
Fail when MTA-STS file has non-CRLF line endings.
Browse files Browse the repository at this point in the history
Up until now, `Test-MtaStsPolicy` would display an unhelpful error
message -- the entire file contents! -- when it read an MTA-STS policy
file with CR or LF line endings.  The specification calls for CRLF line
endings, and that cmdlet has always respected that.

This commit causes it to parse the file twice, once with Windows-style
CRLF line endings assumed, and again with UNIX-style LF line endings
assumed.  (We're going to assume no one is developing on classic macOS
with its bizarre CR line endings.)  If the results are different, then
we can assume the file has invalid line endings, and we will fail with
a helpful error message, rather than the token parser printing the
entire file via Write-Host.

DMARCian, fix your MTA-STS file!
  • Loading branch information
rhymeswithmogul committed Dec 7, 2023
1 parent 981377b commit 20fd51e
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- **FIX** `Test-DaneRecords` now correctly checks DANE records for domains without MX records.
- **FIX** The DMARC `fo` token is now parsed correctly when multiple values are present.
- **FIX** The DMARC `rf` token is now parsed correctly.
- **FIX** The MTA-STS policy file test returns a better error message when the file does not have the correct CRLF line endings.
- **FIX** The SPF `exists` and `mx` token parsers no longer generate a spurious error when *not* counting DNS lookups.
- **FIX** IntelliSense's handling of `Test-SpfRecord` has been improved by hiding some internal-use-only parameters.
- **FIX** Cleaned up `Test-DaneRecords`' output.
Expand Down
1 change: 1 addition & 0 deletions MailPolicyExplainer.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ Many bugs were fixed, too:
- The DMARC `fo` token is now parsed correctly when multiple values are present.
- The DMARC `rf` token is now parsed correctly.
- The IntelliSense handling of `Test-SpfRecord` has been improved by hiding some internal-use-only parameters.
- The MTA-STS policy file test returns a better error message when the file does not have the correct CRLF line endings.
- The SPF `exists` and `mx` token parsers no longer generate a spurious error when not counting DNS lookups.
- Cleaned up the output of `Test-DaneRecords` a little.
- Miscellaneous code cleanup.
Expand Down
18 changes: 16 additions & 2 deletions src/MailPolicyExplainer.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,21 @@ Function Test-MtaStsPolicy
Write-BadNews "MTA-STS Policy: It was found, but was returned with the wrong content type ($($policy.Headers.'Content-Type'))."
}
Else {
$policy.Content.Split("`r`n") | ForEach-Object {
#region Make sure the file has the correct line endings.
# The MTA-STS RFC says that they should end with CRLF (i.e., "`r`n").
# Split it up two different ways and see if we get the same results.
# If not, then someone probably saved the file with UNIX ("`r") endings.
# We're going to be strict and refuse to parse the file in this case.
$lines = $policy.Content.Split("`r`n")
$LFlines = $policy.Content -Split "`r?`n"

If ($lines -ne $LFLines) {
Write-BadNews "MTA-STS Policy: The policy file does not have the correct CRLF line endings!"
Return
}
#endregion

$lines | ForEach-Object {
$line = $_.Trim()
If ($line -CLike 'version: *') {
If (($line -Split ':')[1].Trim() -Eq 'STSv1') {
Expand Down Expand Up @@ -1279,7 +1293,7 @@ Function Test-SpfRecord
If ($CountDnsLookups) {
$DnsLookups.Value++
}

If ($token -Match "^\+?exists:.*") {
Write-GoodNews "${RecordType}: Accept mail if $($token -Replace '\+' -Replace 'exists:') resolves to an A record.$(Write-DnsLookups $DnsLookups -Enabled:$CountDnsLookups)"
}
Expand Down

0 comments on commit 20fd51e

Please sign in to comment.