Skip to content

Commit

Permalink
DKIM tweaks (#1969)
Browse files Browse the repository at this point in the history 
* DKIM tweaks, see #1962, #1964, #1965

* Don't use the `l` tag in DKIM signature, fixes #1964

* CS

* CS

* Fix order of operations, see #1962

* Remove trailing line break from output of `DKIM_Add()`, see #1962
  • Loading branch information
@Synchro
Synchro committed Feb 17, 2020
1 parent 6ae5181 commit b294b44
Showing 1 changed file with 46 additions and 21 deletions.
67 changes: 46 additions & 21 deletions src/PHPMailer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -769,11 +769,22 @@ class PHPMailer
const STOP_CRITICAL = 2;

/**
* SMTP RFC standard line ending.
* The SMTP standard CRLF line break.
* If you want to change line break format, change static::$LE, not this.
*/
const CRLF = "\r\n";

/**
* "Folding White Space" a white space string used for line folding.
*/
const FWS = ' ';

/**
* SMTP RFC standard line ending; Carriage Return, Line Feed.
*
* @var string
*/
protected static $LE = "\r\n";
protected static $LE = self::CRLF;

/**
* The maximum line length supported by mail().
Expand Down Expand Up @@ -1446,7 +1457,7 @@ public function preSend()
) {
//SMTP mandates RFC-compliant line endings
//and it's also used with mail() on Windows
static::setLE("\r\n");
static::setLE(self::CRLF);
} else {
//Maintain backward compatibility with legacy Linux command line mailers
static::setLE(PHP_EOL);
Expand Down Expand Up @@ -1553,7 +1564,7 @@ public function preSend()
$this->encodeHeader($this->secureHeader($this->Subject)),
$this->MIMEBody
);
$this->MIMEHeader = rtrim($this->MIMEHeader, "\r\n ") . static::$LE .
$this->MIMEHeader = static::stripTrailingWSP($this->MIMEHeader) . static::$LE .
static::normalizeBreaks($header_dkim) . static::$LE;
}

Expand Down Expand Up @@ -1620,7 +1631,7 @@ public function postSend()
*/
protected function sendmailSend($header, $body)
{
$header = rtrim($header, "\r\n ") . static::$LE . static::$LE;
$header = static::stripTrailingWSP($header) . static::$LE . static::$LE;

// CVE-2016-10033, CVE-2016-10045: Don't pass -f if characters will be escaped.
if (!empty($this->Sender) && self::isShellSafe($this->Sender)) {
Expand Down Expand Up @@ -1750,7 +1761,7 @@ protected static function isPermittedPath($path)
*/
protected function mailSend($header, $body)
{
$header = rtrim($header, "\r\n ") . static::$LE . static::$LE;
$header = static::stripTrailingWSP($header) . static::$LE . static::$LE;

$toArr = [];
foreach ($this->to as $toaddr) {
Expand Down Expand Up @@ -1839,7 +1850,7 @@ public function setSMTPInstance(SMTP $smtp)
*/
protected function smtpSend($header, $body)
{
$header = rtrim($header, "\r\n ") . static::$LE . static::$LE;
$header = static::stripTrailingWSP($header) . static::$LE . static::$LE;
$bad_rcpt = [];
if (!$this->smtpConnect($this->SMTPOptions)) {
throw new Exception($this->lang('smtp_connect_failed'), self::STOP_CRITICAL);
Expand Down Expand Up @@ -2511,7 +2522,8 @@ public function getMailMIME()
*/
public function getSentMIMEMessage()
{
return rtrim($this->MIMEHeader . $this->mailHeader, "\n\r") . static::$LE . static::$LE . $this->MIMEBody;
return static::stripTrailingWSP($this->MIMEHeader . $this->mailHeader) .
static::$LE . static::$LE . $this->MIMEBody;
}

/**
Expand Down Expand Up @@ -4356,7 +4368,7 @@ public static function normalizeBreaks($text, $breaktype = null)
$breaktype = static::$LE;
}
// Normalise to \n
$text = str_replace(["\r\n", "\r"], "\n", $text);
$text = str_replace([self::CRLF, "\r"], "\n", $text);
// Now convert LE as needed
if ("\n" !== $breaktype) {
$text = str_replace("\n", $breaktype, $text);
Expand All @@ -4365,6 +4377,18 @@ public static function normalizeBreaks($text, $breaktype = null)
return $text;
}

/**
* Remove trailing breaks from a string.
*
* @param string $text
*
* @return string The text to remove breaks from
*/
public static function stripTrailingWSP($text)
{
return rtrim($text, " \r\n\t");
}

/**
* Return the current line break format string.
*
Expand Down Expand Up @@ -4473,13 +4497,15 @@ public function DKIM_Sign($signHeader)
*/
public function DKIM_HeaderC($signHeader)
{
//Normalize breaks to CRLF (regardless of the mailer)
$signHeader = static::normalizeBreaks($signHeader, self::CRLF);
//Unfold header lines
//Note PCRE \s is too broad a definition of whitespace; RFC5322 defines it as `[ \t]`
//@see https://tools.ietf.org/html/rfc5322#section-2.2
//That means this may break if you do something daft like put vertical tabs in your headers.
//Unfold header lines
$signHeader = preg_replace('/\r\n[ \t]+/', ' ', $signHeader);
//Break headers out into an array
$lines = explode("\r\n", $signHeader);
$lines = explode(self::CRLF, $signHeader);
foreach ($lines as $key => $line) {
//If the header is missing a :, skip it as it's invalid
//This is likely to happen because the explode() above will also split
Expand All @@ -4499,7 +4525,7 @@ public function DKIM_HeaderC($signHeader)
$lines[$key] = trim($heading, " \t") . ':' . trim($value, " \t");
}

return implode("\r\n", $lines);
return implode(self::CRLF, $lines);
}

/**
Expand All @@ -4516,13 +4542,13 @@ public function DKIM_HeaderC($signHeader)
public function DKIM_BodyC($body)
{
if (empty($body)) {
return "\r\n";
return self::CRLF;
}
// Normalize line endings to CRLF
$body = static::normalizeBreaks($body, "\r\n");
$body = static::normalizeBreaks($body, self::CRLF);

//Reduce multiple trailing line breaks to a single one
return rtrim($body, "\r\n") . "\r\n";
return static::stripTrailingWSP($body) . self::CRLF;
}

/**
Expand All @@ -4543,6 +4569,7 @@ public function DKIM_Add($headers_line, $subject, $body)
$DKIMquery = 'dns/txt'; // Query method
$DKIMtime = time();
//Always sign these headers without being asked
//Recommended list from https://tools.ietf.org/html/rfc6376#section-5.4.1
$autoSignHeaders = [
'From',
'To',
Expand Down Expand Up @@ -4626,9 +4653,9 @@ public function DKIM_Add($headers_line, $subject, $body)
//Fold long values
if (strlen($copiedHeader) > self::STD_LINE_LENGTH - 3) {
$copiedHeaderFields .= substr(
chunk_split($copiedHeader, self::STD_LINE_LENGTH - 3, static::$LE . ' '),
chunk_split($copiedHeader, self::STD_LINE_LENGTH - 3, static::$LE . self::FWS),
0,
-strlen(static::$LE . ' ')
-strlen(static::$LE . self::FWS)
);
} else {
$copiedHeaderFields .= $copiedHeader;
Expand All @@ -4640,7 +4667,6 @@ public function DKIM_Add($headers_line, $subject, $body)
$headerKeys = ' h=' . implode(':', $headersToSignKeys) . ';' . static::$LE;
$headerValues = implode(static::$LE, $headersToSign);
$body = $this->DKIM_BodyC($body);
$DKIMlen = strlen($body); // Length of body
$DKIMb64 = base64_encode(pack('H*', hash('sha256', $body))); // Base64 of packed binary SHA-256 hash of body
$ident = '';
if ('' !== $this->DKIM_identity) {
Expand All @@ -4654,7 +4680,6 @@ public function DKIM_Add($headers_line, $subject, $body)
' s=' . $this->DKIM_selector . ';' . static::$LE .
' a=' . $DKIMsignatureType . ';' .
' q=' . $DKIMquery . ';' .
' l=' . $DKIMlen . ';' .
' t=' . $DKIMtime . ';' .
' c=' . $DKIMcanonicalization . ';' . static::$LE .
$headerKeys .
Expand All @@ -4667,9 +4692,9 @@ public function DKIM_Add($headers_line, $subject, $body)
$headerValues . static::$LE . $dkimSignatureHeader
);
$signature = $this->DKIM_Sign($canonicalizedHeaders);
$signature = trim(chunk_split($signature, self::STD_LINE_LENGTH - 3, static::$LE . ' '));
$signature = trim(chunk_split($signature, self::STD_LINE_LENGTH - 3, static::$LE . self::FWS));

return static::normalizeBreaks($dkimSignatureHeader . $signature) . static::$LE;
return static::normalizeBreaks($dkimSignatureHeader . $signature);
}

/**
Expand Down

0 comments on commit b294b44

Please sign in to comment.