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/dev/Common/Html.js b/dev/Common/Html.js index af068489b..43c29be7e 100644 --- a/dev/Common/Html.js +++ b/dev/Common/Html.js @@ -283,7 +283,11 @@ export const // hr 'noshade', // img - 'hspace', 'sizes', 'srcset', 'vspace', + // srcset and sizes are not allowed: only src goes through the + // remote content consent and proxy rewrite below, so a srcset + // URL loaded without asking (directly, or relative to this + // origin through ?/ProxyExternal as a read receipt). + 'hspace', 'vspace', // meter 'low', 'high', 'optimum', 'value', // ol @@ -569,6 +573,19 @@ export const oStyle.color = delAttribute('color'); } + // Only the three properties below go through the remote content and + // cid: handling. A url() in any other property (border-image-source, + // mask-image, filter, ...) would load without consent, a relative one + // through ?/ProxyExternal on this origin, so drop those outright. + let i = oStyle.length; + while (i--) { + const property = oStyle[i]; + if (!['background-image', 'list-style-image', 'content'].includes(property) + && /url\s*\(/i.test(oStyle.getPropertyValue(property))) { + oStyle.removeProperty(property); + } + } + if (!skipStyle) { /* if ('fixed' === oStyle.position) { diff --git a/plugins/two-factor-auth/index.php b/plugins/two-factor-auth/index.php index 21b449daa..b06676df5 100644 --- a/plugins/two-factor-auth/index.php +++ b/plugins/two-factor-auth/index.php @@ -88,6 +88,34 @@ class TwoFactorAuthPlugin extends \RainLoop\Plugins\AbstractPlugin } } + /** + * Turning 2-step verification off, wiping it, replacing its secret or showing + * the secret all used to need only the session. Whoever held a session (a + * stolen cookie, an unlocked machine) could quietly drop the second factor or + * copy it. While it is enabled, these now need a current code or backup code, + * as login does. A used backup code is spent. + */ + private function requireCurrentCode(MainAccount $oAccount) : void + { + $aData = $this->getTwoFactorInfo($oAccount); + if (empty($aData['IsSet']) || empty($aData['Enable']) || empty($aData['Secret'])) { + return; + } + $sCode = \trim($this->jsonParam('Code', '')); + if (\strlen($sCode)) { + if ($this->TwoFactorAuthProvider($oAccount)->VerifyCode($aData['Secret'], $sCode)) { + return; + } + $aBackupCodes = \explode(' ', \trim(\preg_replace('/[^\d]+/', ' ', $aData['BackupCodes'] ?? ''))); + if (6 < \strlen($sCode) && \in_array($sCode, $aBackupCodes, true)) { + $this->removeBackupCodeFromTwoFactorInfo($oAccount, $sCode); + return; + } + } + $this->Logger()->Write("TFA: change refused for {$oAccount->Email()}, no valid current code"); + throw new ClientException(\RainLoop\Notifications::AuthError); + } + public function DoGetTwoFactorInfo() : array { $oAccount = $this->getMainAccountFromToken(); @@ -107,11 +135,15 @@ class TwoFactorAuthPlugin extends \RainLoop\Plugins\AbstractPlugin return $this->jsonResponse(__FUNCTION__, false); } + // Overwrites the stored secret with Enable false, i.e. also switches it off + $this->requireCurrentCode($oAccount); + $sEmail = $oAccount->Email(); $sSecret = $this->TwoFactorAuthProvider($oAccount)->CreateSecret(); - $aCodes = \array_map(function(){return \rand(100000000, 900000000);}, \array_fill(0, 8, null)); + // Backup codes are a second factor; rand() is not a CSPRNG + $aCodes = \array_map(fn() => \random_int(100000000, 899999999), \array_fill(0, 8, null)); $this->StorageProvider()->Put($oAccount, \RainLoop\Providers\Storage\Enumerations\StorageType::CONFIG, @@ -148,6 +180,8 @@ class TwoFactorAuthPlugin extends \RainLoop\Plugins\AbstractPlugin return $this->jsonResponse(__FUNCTION__, false); } + $this->requireCurrentCode($oAccount); + $aResult = $this->getTwoFactorInfo($oAccount); unset($aResult['BackupCodes']); @@ -164,6 +198,10 @@ class TwoFactorAuthPlugin extends \RainLoop\Plugins\AbstractPlugin return $this->jsonResponse(__FUNCTION__, false); } + if ('1' !== \trim($this->jsonParam('Enable', '0'))) { + $this->requireCurrentCode($oAccount); + } + $oActions = $this->Manager()->Actions(); if ($oActions->HasActionParam('EnableTwoFactor')) { $sValue = $oActions->GetActionParam('EnableTwoFactor', ''); @@ -215,6 +253,8 @@ class TwoFactorAuthPlugin extends \RainLoop\Plugins\AbstractPlugin return $this->jsonResponse(__FUNCTION__, false); } + $this->requireCurrentCode($oAccount); + $this->StorageProvider()->Clear($oAccount, \RainLoop\Providers\Storage\Enumerations\StorageType::CONFIG, 'two_factor' diff --git a/plugins/two-factor-auth/js/TwoFactorAuthSettings.js b/plugins/two-factor-auth/js/TwoFactorAuthSettings.js index 79d8c6bb5..c85940d37 100644 --- a/plugins/two-factor-auth/js/TwoFactorAuthSettings.js +++ b/plugins/two-factor-auth/js/TwoFactorAuthSettings.js @@ -22,8 +22,9 @@ const * @param {?Function} fCallback * @param {boolean} bEnable */ - enableTwoFactor(fCallback, bEnable) { + enableTwoFactor(fCallback, bEnable, oParams) { rl.pluginRemoteRequest(fCallback, 'EnableTwoFactor', { + ...oParams, Enable: bEnable ? 1 : 0 }); } @@ -61,8 +62,12 @@ class TwoFactorAuthSettings rl.settings.get('RequireTwoFactor') && rl.settings.set('SetupTwoFactor', !!iError); }, value); } else { - value || this.viewEnable_(value); - Remote.enableTwoFactor(fn, false); + // Turning it off needs a current code while it is on. + // Cancelled or refused, the checkbox goes back to on. + this.withCode(params => { + value || this.viewEnable_(value); + Remote.enableTwoFactor(iError => iError && this.viewEnable_(true), false, params); + }, () => this.viewEnable_.valueHasMutated()); } } }, @@ -86,12 +91,28 @@ class TwoFactorAuthSettings }).forEach(([key, fn]) => this[key] = ko.computed(fn)); this.onResult = this.onResult.bind(this); + this.onChangeResult = this.onChangeResult.bind(this); this.onShowSecretResult = this.onShowSecretResult.bind(this); } + /** + * While 2-step verification is on, the server wants a current code (or a + * backup code) to turn it off, clear it, replace it or show its secret. + */ + withCode(fDo, fCancel) { + if (!this.viewEnable_()) { + fDo({}); + } else { + const code = (prompt(rl.i18n('PLUGIN_2FA/LABEL_TWO_FACTOR_CODE')) || '').trim(); + code ? fDo({ Code: code }) : fCancel?.(); + } + } + showSecret() { - this.secreting(true); - rl.pluginRemoteRequest(this.onShowSecretResult, 'ShowTwoFactorSecret'); + this.withCode(params => { + this.secreting(true); + rl.pluginRemoteRequest(this.onShowSecretResult, 'ShowTwoFactorSecret', params); + }); } hideSecret() { @@ -101,8 +122,10 @@ class TwoFactorAuthSettings } createTwoFactor() { - this.processing(true); - rl.pluginRemoteRequest(this.onResult, 'CreateTwoFactorSecret'); + this.withCode(params => { + this.processing(true); + rl.pluginRemoteRequest(this.onChangeResult, 'CreateTwoFactorSecret', params); + }); } testTwoFactor() { @@ -115,12 +138,26 @@ class TwoFactorAuthSettings } clearTwoFactor() { - this.hideSecret(); + this.withCode(params => { + this.hideSecret(); - this.twoFactorTested(false); + this.twoFactorTested(false); - this.clearing(true); - rl.pluginRemoteRequest(this.onResult, 'ClearTwoFactorInfo'); + this.clearing(true); + rl.pluginRemoteRequest(this.onChangeResult, 'ClearTwoFactorInfo', params); + }); + } + + // A refused change must not show the account as unconfigured, which is what + // onResult does with an error; reload the real state instead. + onChangeResult(iError, oData) { + if (iError) { + this.processing(false); + this.clearing(false); + this.onBuild(); + } else { + this.onResult(iError, oData); + } } onShow() { diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php b/snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php index f16598be8..d96e2f74c 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php +++ b/snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php @@ -300,7 +300,11 @@ abstract class Utils $sValue = static::ConvertEncoding($sValue, $sMainCharset, Enumerations\Charset::UTF_8); } - return $sValue; + // The strip above runs before decoding, so an encoded-word such as + // =?utf-8?b?...?= can still decode to a CR or LF. A decoded header value + // is one line; a line break here would be carried into the headers of a + // reply (In-Reply-To is copied from Message-ID) and inject new ones. + return \preg_replace('/[\r\n\0]+/', ' ', $sValue); } public static function RemoveHeaderFromHeaders(string $sIncHeaders, array $aHeadersToRemove = array()) : string diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php b/snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php index 523134093..0a8b29789 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php +++ b/snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php @@ -37,8 +37,13 @@ class Header implements \JsonSerializable private function initInputData(string $sName, string $sValue, string $sEncodedValueForReparse): void { - $this->sName = \trim($sName); - $this->sFullValue = \trim($sValue); + // A header is one logical line. A CR or LF in a name or value lets the + // value end its own header and start another (or the body), and values + // reach here from decoded encoded-words and from client parameters, so + // no caller can be relied on to have stripped them. Folding is added + // back by __toString(). + $this->sName = \trim(\preg_replace('/[\r\n\0]+/', ' ', $sName)); + $this->sFullValue = \trim(\preg_replace('/[\r\n\0]+/', ' ', $sValue)); $this->sEncodedValue = ''; if (\strlen($sEncodedValueForReparse) && ($this->IsEmail() || $this->IsSubject() || $this->IsParameterized())) { diff --git a/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Raw.php b/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Raw.php index ef73f9c7f..d7f1c7cdb 100644 --- a/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Raw.php +++ b/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Raw.php @@ -212,6 +212,21 @@ trait Raw } } + // The type comes from the message: whoever sent the mail decides it. + // Viewed inline under this origin, text/html (or SVG, XML, ...) + // becomes a page of the sender's choosing; the CSP stops scripts + // but not forms, meta refresh or remote styles. Only a few types + // are shown inline as declared. + if (!$bDownload) { + $sInlineType = static::inlineContentType($sContentType); + if (null === $sInlineType) { + $bDownload = true; + $sContentType = 'application/octet-stream'; + } else { + $sContentType = $sInlineType; + } + } + if (!\headers_sent()) { \header('Content-Type: '.$sContentType); \MailSo\Base\Http::setContentDisposition($bDownload ? 'attachment' : 'inline', ['filename' => $sFileName]); @@ -262,6 +277,27 @@ trait Raw ); } + /** + * The Content-Type to view an attachment inline with, or null to make it a + * download. Raster images, PDF, audio, video and plain text are shown as + * declared; any other text/* is shown as text/plain, so an HTML attachment + * displays its source instead of rendering as a page on this origin. + */ + public static function inlineContentType(string $sContentType) : ?string + { + $sType = \strtolower(\trim(\explode(';', $sContentType, 2)[0])); + if (\in_array($sType, ['image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/avif', 'image/bmp', + 'application/pdf', 'text/plain'], true) + || \preg_match('#^(audio|video)/[a-z0-9.+-]+$#', $sType) + ) { + return $sType; + } + if (\str_starts_with($sType, 'text/')) { + return 'text/plain'; + } + return null; + } + private static function loadImage($resource, bool $bDetectImageOrientation = false, int $iThumbnailBoxSize = 0) : \SnappyMail\Image { if (\extension_loaded('gmagick')) { $handler = 'gmagick'; } diff --git a/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/SMime.php b/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/SMime.php index b835126a8..a6f06b06a 100644 --- a/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/SMime.php +++ b/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/SMime.php @@ -9,6 +9,7 @@ use MailSo\Imap\Enumerations\FetchType; trait SMime { private $SMIME = null; + public function SMIME() : OpenSSL { if (!$this->SMIME) { @@ -85,7 +86,7 @@ trait SMime { $sFolderName = $this->GetActionParam('folder', ''); $iUid = (int) $this->GetActionParam('uid', 0); - $sPartId = $this->GetActionParam('partId', ''); + $sPartId = $this->smimePartIdParam('partId'); $sCertificate = $this->GetActionParam('certificate', ''); $sPrivateKey = $this->GetActionParam('privateKey', ''); $oPassphrase = new \SnappyMail\SensitiveString($this->GetActionParam('passphrase', '')); @@ -138,7 +139,7 @@ trait SMime public function DoSMimeVerifyMessage() : array { $sBody = $this->GetActionParam('bodyPart', ''); - $sPartId = $this->GetActionParam('partId', ''); + $sPartId = $this->smimePartIdParam('partId'); $bDetached = !empty($this->GetActionParam('detached', 0)); if (!$sBody && $sPartId) { $iUid = (int) $this->GetActionParam('uid', 0); @@ -153,7 +154,7 @@ trait SMime // Import the certificates automatically $sBody = $this->GetActionParam('sigPart', ''); - $sPartId = $this->GetActionParam('sigPartId', '') ?: $sPartId; + $sPartId = $this->smimePartIdParam('sigPartId') ?: $sPartId; if (!$sBody && $sPartId && $oImapClient) { $sBody = $oImapClient->Fetch( [FetchType::BODY_PEEK.'['.$sPartId.']'], @@ -190,7 +191,7 @@ trait SMime /* $sBody = $this->GetActionParam('sigPart', ''); if (!$sBody) { - $sPartId = $this->GetActionParam('sigPartId', '') ?: $this->GetActionParam('partId', ''); + $sPartId = $this->smimePartIdParam('sigPartId') ?: $this->smimePartIdParam('partId'); $this->initMailClientConnection(); $oImapClient = $this->ImapClient(); $oImapClient->FolderExamine($this->GetActionParam('folder', '')); @@ -213,4 +214,18 @@ trait SMime return $this->DefaultResponse($certificates); */ } + + /** + * A part id from the client goes into an IMAP FETCH section, BODY.PEEK[...], + * as is. Accept only a section number (1, 1.2.3) or TEXT, so the value cannot + * close the bracket or the line and add commands of its own. + */ + private function smimePartIdParam(string $sKey): string + { + $sPartId = \trim((string) $this->GetActionParam($sKey, '')); + if ('' !== $sPartId && !\preg_match('/^(TEXT|[1-9][0-9]*(\.[1-9][0-9]*)*)$/D', $sPartId)) { + throw new \RainLoop\Exceptions\ClientException(\RainLoop\Notifications::InvalidInputArgument); + } + return $sPartId; + } } diff --git a/snappymail/v/0.0.0/app/libraries/RainLoop/ServiceActions.php b/snappymail/v/0.0.0/app/libraries/RainLoop/ServiceActions.php index a1dda54e7..a24a2e225 100644 --- a/snappymail/v/0.0.0/app/libraries/RainLoop/ServiceActions.php +++ b/snappymail/v/0.0.0/app/libraries/RainLoop/ServiceActions.php @@ -345,8 +345,12 @@ class ServiceActions $sMethodName = 'Raw'.$sAction; if (\method_exists($this->oActions, $sMethodName)) { \header('X-Raw-Action: '.$sMethodName); - \header('Content-Security-Policy: script-src \'none\'; child-src \'none\'; object-src \'none\''); - + // Raw responses carry content from the message. Besides scripts and + // frames, deny forms, base and everything not needed to show an + // image, media, a PDF or plain text. + \header('Content-Security-Policy: default-src \'none\'; script-src \'none\'; child-src \'none\'; ' + . 'form-action \'none\'; base-uri \'none\'; img-src \'self\' data:; media-src \'self\'; ' + . 'style-src \'self\' \'unsafe-inline\'; object-src \'self\''); $sRawError = ''; $this->oActions->SetActionParams(array( 'RawKey' => empty($this->aPaths[3]) ? '' : $this->aPaths[3], diff --git a/snappymail/v/0.0.0/app/libraries/snappymail/crypt.php b/snappymail/v/0.0.0/app/libraries/snappymail/crypt.php index 066352dcc..86aab11bd 100644 --- a/snappymail/v/0.0.0/app/libraries/snappymail/crypt.php +++ b/snappymail/v/0.0.0/app/libraries/snappymail/crypt.php @@ -64,8 +64,21 @@ abstract class Crypt ): mixed { if (3 === \count($data) && isset($data[0], $data[1], $data[2]) && \strlen($data[0])) { - $fn = "{$data[0]}Decrypt"; - if (!\method_exists(__CLASS__, $fn)) { + // The algorithm name comes from the token, so it must not be able to + // pick a weaker one than Encrypt() would have used. Xxtea is only + // written when neither sodium nor openssl is available; anywhere else + // an xxtea token was not made by this server, and before the key + // derivation fix below its 16 byte salt was the entire key, so anyone + // could mint one. + $algo = \strtolower($data[0]); + if ('xxtea' === $algo + && (\is_callable('sodium_crypto_aead_xchacha20poly1305_ietf_decrypt') || \is_callable('openssl_decrypt')) + ) { + Log::warning('Crypt', 'xxtea token refused, a stronger algorithm is available'); + return null; + } + $fn = "{$algo}Decrypt"; + if (!\in_array($algo, ['sodium', 'openssl', 'xxtea'], true) || !\method_exists(__CLASS__, $fn)) { Log::warning('Crypt', "{$fn} does not exists"); } else { try { @@ -261,6 +274,22 @@ abstract class Crypt return $result; } + /** + * XXTEA takes a 128 bit key and reads only the first 16 bytes of what it is + * given. The key used to be $salt . Passphrase(), and the salt is 16 random + * bytes stored in the token, so the server secret never took part: xxtea + * tokens could be read and forged by anyone. Hash the two together instead. + * Tokens made under the old derivation no longer decrypt; they were never + * keyed by this server. + */ + private static function XxteaKey(string $salt, + #[\SensitiveParameter] + ?string $key + ) : string + { + return \substr(\hash('sha256', $salt . static::Passphrase($key), true), 0, 16); + } + public static function XxteaDecrypt(string $data, string $salt, #[\SensitiveParameter] ?string $key = null @@ -269,7 +298,7 @@ abstract class Crypt if (!$data || !$salt) { throw new \ValueError('$data or $salt is empty string'); } - $key = $salt . static::Passphrase($key); + $key = static::XxteaKey($salt, $key); return \is_callable('xxtea_decrypt') ? \xxtea_decrypt($data, $key) : \MailSo\Base\Xxtea::decrypt($data, $key); @@ -286,7 +315,7 @@ abstract class Crypt if (!$data || !$salt) { throw new \ValueError('$data or $salt is empty string'); } - $key = $salt . static::Passphrase($key); + $key = static::XxteaKey($salt, $key); $result = \is_callable('xxtea_encrypt') ? \xxtea_encrypt($data, $key) : \MailSo\Base\Xxtea::encrypt($data, $key); diff --git a/snappymail/v/0.0.0/app/libraries/snappymail/host.php b/snappymail/v/0.0.0/app/libraries/snappymail/host.php new file mode 100644 index 000000000..18e119593 --- /dev/null +++ b/snappymail/v/0.0.0/app/libraries/snappymail/host.php @@ -0,0 +1,312 @@ + self::UnwrapEmbeddedIPv4($ip) ?: $ip, $ips); + + // Numeric IPv4 literals in non-standard forms (http://2130706433/, + // http://017700000001/, http://0x7f000001/, http://127.1/, + // http://1.2.3.04/): the HTTP client parses these as IPs directly + // (verified: curl connects to 127.0.0.1 for the hex forms with no + // DNS involved) while dns_get_record() below would query them as + // hostnames, so normalize to dotted decimal before validation. + $ips = \array_map(fn($ip) => self::NormalizeNumericIPv4($ip) ?: $ip, $ips); + + // Check them all + if (\array_all($ips, fn($ip) => self::IsPublicIP($ip))) { + return true; + } + + return false; + } + + private static ?array $aLocalAddresses = null; + + private static function PrefixMatches(string $bin, string $net, int $bits) : bool + { + $bytes = \intdiv($bits, 8); + if (\substr($bin, 0, $bytes) !== \substr($net, 0, $bytes)) { + return false; + } + $rest = $bits % 8; + if (!$rest) { + return true; + } + $mask = (0xff << (8 - $rest)) & 0xff; + return (\ord($bin[$bytes]) & $mask) === (\ord($net[$bytes]) & $mask); + } + + /** + * Normalize a numeric IPv4 literal in non-standard form to dotted + * decimal, following inet_aton semantics: 1-4 dot-separated parts, + * each decimal, octal (leading zero) or hex (0x prefix); the last + * part takes all remaining bits (32/24/16/8). Returns null when the + * host is not such a literal. A part like "08" is not parsed as + * numeric by the resolver, so it is left for the hostname path below. + */ + private static function NormalizeNumericIPv4(string $host) : ?string + { + $aParts = \explode('.', $host); + $iCount = \count($aParts); + if ($iCount < 1 || $iCount > 4) { + return null; + } + $aNums = array(); + foreach ($aParts as $sPart) { + if ('' === $sPart + || !\preg_match('/^(0[xX][0-9a-fA-F]+|[0-9]+)$/', $sPart)) { + return null; + } + if (\preg_match('/^0[xX]/', $sPart)) { + if (10 < \strlen($sPart)) { + return null; // longer than 0x + 8 digits: not a 32-bit value + } + $aNums[] = \hexdec($sPart); + continue; + } + if (1 < \strlen($sPart) && '0' === $sPart[0] + && !\preg_match('/^0[0-7]+$/', $sPart)) { + return null; + } + if (12 < \strlen($sPart)) { + return null; // longer than 037777777777: not a 32-bit value + } + $aNums[] = 1 < \strlen($sPart) && '0' === $sPart[0] + ? \octdec($sPart) : (int) $sPart; + } + $iMax = array(4294967295, 16777215, 65535, 255); + for ($i = 0; $i < $iCount - 1; $i++) { + if ($aNums[$i] > 255) { + return null; + } + } + if ($aNums[$iCount - 1] > $iMax[$iCount - 1]) { + return null; + } + switch ($iCount) { + case 1: + $long = $aNums[0]; + break; + case 2: + $long = ($aNums[0] << 24) | $aNums[1]; + break; + case 3: + $long = ($aNums[0] << 24) | ($aNums[1] << 16) | $aNums[2]; + break; + default: + $long = ($aNums[0] << 24) | ($aNums[1] << 16) | ($aNums[2] << 8) | $aNums[3]; + break; + } + return \long2ip($long); + } + + /** + * Unwrap an IPv6 literal with an embedded IPv4 address to dotted + * decimal, or null when the host is not such a literal. Covers the + * IPv4-mapped range ::ffff:0:0/96 (dotted or hex tail), the NAT64 + * well-known prefix 64:ff9b::/96, and the deprecated + * IPv4-compatible form ::a.b.c.d (textual dotted-quad tail only, + * so ::1 stays loopback and is never mistaken for ::0.0.0.1). + * Needed because FILTER_FLAG_NO_RES_RANGE does not treat these + * ranges as reserved even though they reach IPv4 addresses in + * practice. + */ + private static function UnwrapEmbeddedIPv4(string $ip) : ?string + { + $sBin = \inet_pton($ip); + if (false === $sBin || 16 !== \strlen($sBin)) { + return null; // not an IPv6 literal + } + $sPrefix = \substr($sBin, 0, 12); + if ("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff" === $sPrefix // ::ffff:0:0/96 + || "\x00\x64\xff\x9b\x00\x00\x00\x00\x00\x00\x00\x00" === $sPrefix) { // 64:ff9b::/96 + return (string) \inet_ntop(\substr($sBin, 12, 4)); + } + // 6to4, 2002:V4ADDR::/48 (RFC 3056): the address sits in bytes 2 to 5, so + // 2002:7f00:1:: is 127.0.0.1. Deprecated by RFC 7526 and unreachable + // without a configured tunnel, but a host that has one would route it. + if ("\x20\x02" === \substr($sBin, 0, 2)) { + return (string) \inet_ntop(\substr($sBin, 2, 4)); + } + $sLower = \strtolower($ip); + if (\str_starts_with($sLower, '::') + && false !== \filter_var(\substr($sLower, 2), \FILTER_VALIDATE_IP, \FILTER_FLAG_IPV4)) { + return \substr($sLower, 2); // deprecated ::a.b.c.d form + } + return null; + } +} diff --git a/snappymail/v/0.0.0/app/libraries/snappymail/integrity.php b/snappymail/v/0.0.0/app/libraries/snappymail/integrity.php index 0abf9dab4..81d238712 100644 --- a/snappymail/v/0.0.0/app/libraries/snappymail/integrity.php +++ b/snappymail/v/0.0.0/app/libraries/snappymail/integrity.php @@ -10,6 +10,16 @@ abstract class Integrity */ public static function test(): void { + // The integrity test sends HEAD requests to a URL built from the request's + // Host header and prints back every response header. Open to anyone, that + // lets a client choose the target (nginx ignores the port when matching + // server_name, so Host: : reaches it) and read the result. + // It is an admin diagnostic; an admin's own browser sends the real Host. + if (!\RainLoop\Api::Actions()->IsAdminLoggined(false)) { + \MailSo\Base\Http::StatusHeader(404); + return ''; + } + $result = static::phpVersion(); if ($result) { echo '

' . $result . '

'; 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 a7c4e892d..0c0563c0f 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/crypt-forgery.php b/test/crypt-forgery.php new file mode 100644 index 000000000..9ef7ebf19 --- /dev/null +++ b/test/crypt-forgery.php @@ -0,0 +1,71 @@ + 'victim@example.com', 'admin' => true]), $salt); + foreach (['xxtea', 'XXTEA', 'Xxtea'] as $algo) { + check(null === $Crypt::Decrypt([$algo, $salt, $forged], 'server-key'), "forged {$algo} token accepted"); + } + + // Unknown algorithm names are refused rather than dispatched. + check(null === $Crypt::Decrypt(['jsonDecode', $salt, $forged], 'server-key'), 'arbitrary method name dispatched'); + + // Normal tokens still round-trip, and only with the right key. + $data = ['e' => 'user@example.com', 'n' => 42]; + check($data === $Crypt::Decrypt($Crypt::Encrypt($data, 'server-key'), 'server-key'), 'sodium round trip broke'); + check($data === $Crypt::DecryptUrlSafe($Crypt::EncryptUrlSafe($data, 'server-key'), 'server-key'), 'url-safe round trip broke'); + check(null === $Crypt::Decrypt($Crypt::Encrypt($data, 'server-key'), 'other-key'), 'decrypted with the wrong key'); + + // The xxtea primitives themselves now depend on the server secret. + $salt = random_bytes(16); + $cipher = $Crypt::XxteaEncrypt('{"a":1}', $salt, 'server-key'); + check('{"a":1}' === $Crypt::XxteaDecrypt($cipher, $salt, 'server-key'), 'xxtea round trip broke'); + check('{"a":1}' !== $Crypt::XxteaDecrypt($cipher, $salt, 'other-key'), 'xxtea key does not depend on the secret'); + check('{"a":1}' !== \MailSo\Base\Xxtea::decrypt($cipher, $salt), 'xxtea still keyed by the salt alone'); + + echo "crypt-forgery: ok\n"; +} diff --git a/test/header-injection.php b/test/header-injection.php new file mode 100644 index 000000000..88943ca5c --- /dev/null +++ b/test/header-injection.php @@ -0,0 +1,48 @@ +\r\nBcc: attacker@example.net"; +$encoded = '=?utf-8?b?'.base64_encode($payload).'?='; +$decoded = \MailSo\Base\Utils::DecodeHeaderValue($encoded); +check(!preg_match('/[\r\n]/', $decoded), 'B-encoded CRLF survived DecodeHeaderValue'); +check(str_contains($decoded, 'Bcc: attacker@example.net'), 'decoded text was lost, not just its line break'); + +$decoded = \MailSo\Base\Utils::DecodeHeaderValue('=?utf-8?q?a=0D=0AX-Injected:_1?='); +check(!preg_match('/[\r\n]/', $decoded), 'Q-encoded CRLF survived DecodeHeaderValue'); + +// The same through header parsing, as for an incoming Message-ID. +$header = \MailSo\Mime\Header::NewInstanceFromEncodedString("Message-ID: {$encoded}"); +check(!preg_match('/[\r\n]/', $header->Value()), 'parsed Message-ID kept a line break'); + +// Outgoing: a value handed straight to the message (In-Reply-To and Subject come +// from client parameters) must serialise as one header, whatever it contains. +$message = new \MailSo\Mime\Message(); +$message->SetInReplyTo("\r\nBcc: attacker@example.net"); +$message->SetSubject("Re: hello\nX-Injected: 1\r\n\r\nbody text"); +$raw = stream_get_contents($message->ToStream()); +[$headers] = explode("\r\n\r\n", $raw, 2); +foreach (explode("\r\n", $headers) as $line) { + check(!preg_match('/^(Bcc|X-Injected):/i', $line), "injected header line: {$line}"); +} +check(!str_contains($headers, "\n\n"), 'blank line injected into the header block'); +check(str_contains($raw, 'In-Reply-To: Bcc: attacker@example.net'), 'In-Reply-To was not kept as one line'); + +// Folding must still work: a long value is wrapped with CRLF + space only. +$header = new \MailSo\Mime\Header('Subject', str_repeat('word ', 40)); +foreach (array_slice(explode("\r\n", (string) $header), 1) as $continuation) { + check(str_starts_with($continuation, ' '), 'folded line does not start with whitespace'); +} + +echo "header-injection: ok\n"; diff --git a/test/raw-inline-type.php b/test/raw-inline-type.php new file mode 100644 index 000000000..dce5af4c3 --- /dev/null +++ b/test/raw-inline-type.php @@ -0,0 +1,45 @@ + 'image/png', + 'IMAGE/JPEG; name="x.jpg"' => 'image/jpeg', + 'application/pdf' => 'application/pdf', + 'video/mp4' => 'video/mp4', + 'audio/ogg; codecs=opus' => 'audio/ogg', + 'text/plain; charset=utf-8' => 'text/plain', + // shown as source, never rendered + 'text/html' => 'text/plain', + 'Text/HTML; charset=utf-8' => 'text/plain', + 'text/xml' => 'text/plain', + // downloaded + 'image/svg+xml' => null, + 'application/xhtml+xml' => null, + 'application/xml' => null, + 'application/octet-stream' => null, + 'multipart/related' => null, + '' => null, +]; +foreach ($cases as $in => $expected) { + $got = RawInlineType::inlineContentType($in); + check($expected === $got, "'{$in}': expected ".var_export($expected, true).', got '.var_export($got, true)); +} + +echo "raw-inline-type: ok\n"; diff --git a/test/smime-partid.php b/test/smime-partid.php new file mode 100644 index 000000000..2cdc80665 --- /dev/null +++ b/test/smime-partid.php @@ -0,0 +1,37 @@ +newInstanceWithoutConstructor(); +$params = new ReflectionProperty(\RainLoop\Actions::class, 'aCurrentActionParams'); +$method = new ReflectionMethod(\RainLoop\Actions::class, 'smimePartIdParam'); +$read = function (string $value) use ($actions, $params, $method) : ?string { + $params->setValue($actions, ['partId' => $value]); + try { + return $method->invoke($actions, 'partId'); + } catch (\RainLoop\Exceptions\ClientException $e) { + return null; + } +}; + +foreach (['', '1', '2', '1.2', '1.2.10', 'TEXT'] as $ok) { + check($ok === $read($ok), "valid part id '{$ok}' refused"); +} +foreach (["1]\r\nA1 LOGOUT\r\n", '1] BODY[HEADER', "1\r\nA1 LOGOUT", '1 2', '0', '1.0', '01', '1.', '.1', 'text', 'HEADER'] as $bad) { + check(null === $read($bad), 'part id '.json_encode($bad).' accepted'); +} +// Surrounding whitespace is trimmed away, and the trimmed value is what is used. +check('1' === $read("1\n"), 'trailing newline not trimmed'); + +echo "smime-partid: ok\n"; diff --git a/test/smime-verify.php b/test/smime-verify.php new file mode 100644 index 000000000..6da2dc8e5 --- /dev/null +++ b/test/smime-verify.php @@ -0,0 +1,61 @@ + 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 \SnappyMail\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"; diff --git a/test/sso.php b/test/sso.php index 604cb55a5..72c0e1deb 100644 --- a/test/sso.php +++ b/test/sso.php @@ -2,7 +2,7 @@ // Enable SnappyMail Api and include index file $_ENV['SNAPPYMAIL_INCLUDE_AS_API'] = true; -require '../index.php'; +require __DIR__ . '/../index.php'; /** * Get SSO hash diff --git a/test/ssrf-ranges.php b/test/ssrf-ranges.php new file mode 100644 index 000000000..83c12f6b7 --- /dev/null +++ b/test/ssrf-ranges.php @@ -0,0 +1,40 @@ + $Request::URIHasPublicHost("http://{$host}/x.png"); + +foreach (['100.64.0.1', '100.100.100.200', '100.127.255.254', '192.0.0.1', '198.18.0.1', + '198.19.255.254', '224.0.0.1', '239.255.255.250', '[fec0::1]', '[ff02::1]', + '127.0.0.1', '10.0.0.1', '169.254.169.254', '[::1]', '[fc00::1]', '[::ffff:100.64.0.1]'] as $host) { + check(!$public($host), "{$host} treated as public"); +} +foreach (['100.63.255.255', '100.128.0.1', '198.17.255.255', '198.20.0.1', '8.8.8.8', + '[2001:4860:4860::8888]'] as $host) { + check($public($host), "{$host} wrongly blocked"); +} + +// The server's own addresses. With --network=none only loopback exists, so +// hand the cache an address to stand for this host's public IP. +$prop = new ReflectionProperty($Request, 'aLocalAddresses'); +$prop->setValue(null, [inet_pton('203.0.113.7'), inet_pton('2001:db8:1::53')]); +check(!$public('203.0.113.7'), "the server's own IPv4 address treated as public"); +check(!$public('[2001:db8:1::53]'), "the server's own IPv6 address treated as public"); +check($public('203.0.113.8'), 'a neighbouring address wrongly blocked'); + +check($public('snappymail.eu'), 'snapymail.eu wrongly blocked'); +check(!$public('localhost'), 'localhost should be blocked'); + +echo "ssrf-ranges: ok\n"; diff --git a/test/two-factor-reauth.php b/test/two-factor-reauth.php new file mode 100644 index 000000000..cd9d5c826 --- /dev/null +++ b/test/two-factor-reauth.php @@ -0,0 +1,75 @@ +info; } + protected function TwoFactorAuthProvider(\RainLoop\Model\MainAccount $oAccount) : ?\TwoFactorAuthInterface + { + return new class implements \TwoFactorAuthInterface { + public function Label() : string { return 'test'; } + public function VerifyCode(string $sSecret, string $sCode) : bool { return 'SECRET' === $sSecret && '123456' === $sCode; } + public function CreateSecret() : string { return 'SECRET'; } + }; + } + protected function Logger() : \MailSo\Log\Logger { return new \MailSo\Log\Logger(false); } + protected function removeBackupCodeFromTwoFactorInfo(\RainLoop\Model\MainAccount $oAccount, string $sCode) : bool { $this->spent[] = $sCode; return true; } +} + +$plugin = (new ReflectionClass(TestTwoFactor::class))->newInstanceWithoutConstructor(); +$actions = (new ReflectionClass(\RainLoop\Actions::class))->newInstanceWithoutConstructor(); +$manager = (new ReflectionClass(\RainLoop\Plugins\Manager::class))->newInstanceWithoutConstructor(); +(new ReflectionProperty(\RainLoop\Plugins\Manager::class, 'oActions'))->setValue($manager, $actions); +(new ReflectionProperty(\RainLoop\Plugins\AbstractPlugin::class, 'oPluginManager'))->setValue($plugin, $manager); +$account = (new ReflectionClass(\RainLoop\Model\MainAccount::class))->newInstanceWithoutConstructor(); +$gate = (new ReflectionMethod(TwoFactorAuthPlugin::class, 'requireCurrentCode')); + +$params = new ReflectionProperty(\RainLoop\Actions::class, 'aCurrentActionParams'); +$allowed = function (array $values) use ($plugin, $actions, $account, $gate, $params) : bool { + $params->setValue($actions, $values); + try { + $gate->invoke($plugin, $account); + return true; + } catch (\RainLoop\Exceptions\ClientException $e) { + return false; + } +}; + +// Not set up, or set up but not enabled: nothing to protect yet. +$plugin->info = ['IsSet' => false, 'Enable' => false, 'Secret' => '']; +check($allowed([]), 'blocked although 2FA is not set up'); +$plugin->info = ['IsSet' => true, 'Enable' => false, 'Secret' => 'SECRET', 'BackupCodes' => '111111111']; +check($allowed([]), 'blocked although 2FA is not enabled'); + +// Enabled: a valid TOTP or backup code is required. +$plugin->info = ['IsSet' => true, 'Enable' => true, 'Secret' => 'SECRET', 'BackupCodes' => '111111111 222222222']; +check(!$allowed([]), 'change allowed with no code while enabled'); +check(!$allowed(['Code' => '000000']), 'change allowed with a wrong code'); +check(!$allowed(['Code' => '33333333']), 'change allowed with an unknown backup code'); +check($allowed(['Code' => '123456']), 'valid TOTP code refused'); +check($allowed(['Code' => '222222222']), 'valid backup code refused'); +check(['222222222'] === $plugin->spent, 'used backup code was not spent'); + +// Backup codes come from random_int now, not rand(). +$source = file_get_contents(dirname(__DIR__).'/plugins/two-factor-auth/index.php'); +check(!preg_match('/\\\\rand\(/', $source) && str_contains($source, '\\random_int('), 'backup codes not from random_int()'); + +echo "two-factor-reauth: ok\n";