diff --git a/.gitignore b/.gitignore index 5f49d65d5..18da1fe05 100644 --- a/.gitignore +++ b/.gitignore @@ -20,7 +20,6 @@ /issues .idea/ .env -/test /public_html /vendors/knockout/spec /vendors/openpgp-5 diff --git a/snappymail/v/0.0.0/app/libraries/snappymail/smime/openssl.php b/snappymail/v/0.0.0/app/libraries/snappymail/smime/openssl.php index 3af3b7f25..c7bc0c059 100644 --- a/snappymail/v/0.0.0/app/libraries/snappymail/smime/openssl.php +++ b/snappymail/v/0.0.0/app/libraries/snappymail/smime/openssl.php @@ -324,10 +324,11 @@ class OpenSSL $opaque |= \str_contains($input, 'application/pkcs7-mime') || \str_contains($input, 'application/x-pkcs7-mime'); $input = $tmp; } + // First pass extracts the content and nothing else: PKCS7_NOSIGS skips the + // signature check, so a message with a broken signature can still be read. $output = $opaque ? new Temporary('smimeout-') : null; if (true !== \openssl_pkcs7_verify( $input->filename(), -// $flags = 0, // \PKCS7_NOVERIFY | \PKCS7_NOCHAIN | \PKCS7_NOSIGS \PKCS7_NOVERIFY | \PKCS7_NOCHAIN | \PKCS7_NOSIGS, $signers_certificates_filename ?: null, $ca_info = [], @@ -337,9 +338,23 @@ class OpenSSL )) { throw new \RuntimeException('OpenSSL verify: ' . \openssl_error_string()); } + // Second pass is the verdict. It used to be the first pass returning true, + // which with PKCS7_NOSIGS it does for any well-formed message, so altered + // or forged content was reported as validly signed. This checks the + // signature against the signer certificate in the message. PKCS7_NOVERIFY + // stays: the signer's chain is not checked against any CA, so 'success' + // means the content is what the certificate holder signed, not that the + // certificate itself is trusted. + $verified = \openssl_pkcs7_verify( + $input->filename(), + \PKCS7_NOVERIFY, + null, + [], + $this->untrusted_certificates_filename + ); return [ 'body' => $output ? $output->getContents() : null, - 'success' => true + 'success' => true === $verified ]; } } diff --git a/test/smime-verify.php b/test/smime-verify.php new file mode 100644 index 000000000..8529b14d2 --- /dev/null +++ b/test/smime-verify.php @@ -0,0 +1,69 @@ + 2048, 'private_key_type' => OPENSSL_KEYTYPE_RSA]); +$cert = openssl_csr_sign(openssl_csr_new(['commonName' => 'signer@example.com'], $key), null, $key, 1); +openssl_x509_export_to_file($cert, "{$dir}/cert.pem"); +openssl_pkey_export_to_file($key, "{$dir}/key.pem"); + +$content = "Content-Type: text/plain\r\n\r\nPay 100 EUR to account A.\r\n"; +file_put_contents("{$dir}/in.txt", $content); + +$smime = new \Tachyon\Util\SMime\OpenSSL($dir); + +// Detached (multipart/signed) and opaque (application/pkcs7-mime) signatures. +foreach (['detached' => PKCS7_DETACHED, 'opaque' => 0] as $kind => $flags) { + check(openssl_pkcs7_sign("{$dir}/in.txt", "{$dir}/signed.eml", "file://{$dir}/cert.pem", "file://{$dir}/key.pem", [], $flags), + "{$kind}: signing failed: ".openssl_error_string()); + $signed = file_get_contents("{$dir}/signed.eml"); + $opaque = 'opaque' === $kind; + + $result = $smime->verify($signed, null, $opaque); + check(true === $result['success'], "{$kind}: intact message not reported as validly signed"); + if ($opaque) { + check(str_contains($result['body'], 'Pay 100 EUR to account A.'), "{$kind}: content not extracted"); + } + + // Alter the signed content. Detached: the clear text part. Opaque: a byte of + // the base64 payload, chosen inside the encapsulated content. + if ($opaque) { + [$head, $b64] = explode("\n\n", str_replace("\r\n", "\n", $signed), 2); + $der = base64_decode($b64); + $pos = strpos($der, 'account A'); + check(false !== $pos, "{$kind}: test could not find the content to alter"); + $der[$pos + 8] = 'B'; + $tampered = $head."\n\n".chunk_split(base64_encode($der), 64, "\n"); + } else { + $tampered = str_replace('account A', 'account B', $signed); + } + check($tampered !== $signed, "{$kind}: test failed to alter the message"); + + $result = $smime->verify($tampered, null, $opaque); + check(false === $result['success'], "{$kind}: ALTERED message reported as validly signed"); + if ($opaque) { + check(str_contains($result['body'], 'account B'), "{$kind}: altered content no longer readable"); + } +} + +array_map('unlink', glob("{$dir}/*")); +rmdir($dir); +echo "smime-verify: ok\n";