diff --git a/passbolt/compose.yml b/passbolt/compose.yml index a2de06e..b613c1d 100644 --- a/passbolt/compose.yml +++ b/passbolt/compose.yml @@ -7,7 +7,6 @@ services: PASSBOLT_SSL_FORCE: "false" TZ: ${TIMEZONE:-Europe/Amsterdam} PASSBOLT_REGISTRATION_PUBLIC: "false" - PASSBOLT_PLUGINS_METADATA_ENABLE_FOR_NEW_INSTANCES: "false" DATASOURCES_DEFAULT_HOST: passbolt_db DATASOURCES_DEFAULT_PORT: "5432" DATASOURCES_DEFAULT_DATABASE: passbolt @@ -25,6 +24,8 @@ services: - ../.data/passbolt/gpg:/etc/passbolt/gpg - ../.data/passbolt/jwt:/etc/passbolt/jwt - ../.data/passbolt/gnupg:/var/lib/passbolt/.gnupg + # Patched to allow 300s clock-skew tolerance in isNotCreatedInTheFutureRule + - ./patches/PublicKeyValidationService.php:/usr/share/php/passbolt/src/Service/OpenPGP/PublicKeyValidationService.php:ro depends_on: db: condition: service_healthy diff --git a/passbolt/patches/PublicKeyValidationService.php b/passbolt/patches/PublicKeyValidationService.php new file mode 100644 index 0000000..fe6bf5a --- /dev/null +++ b/passbolt/patches/PublicKeyValidationService.php @@ -0,0 +1,487 @@ + [ + self::IS_PARSABLE_ARMORED_KEY_RULE => __('The public key could not be parsed.'), + ], + ]); + } + + // Other rules are recommended but not mandatory + // As one may want to see what's inside the key info for debugging purpose + $keyInfo = self::getPublicKeyInfo($armoredKey); + $validationErrors = []; + foreach ($rules as $ruleName) { + switch ($ruleName) { + case self::IS_VALID_ALGORITHM_RULE: + if (!self::isValidAlgorithm($keyInfo['type'], false)) { + $validationErrors[$ruleName] = __('The algorithm is invalid.'); + } + break; + case self::IS_VALID_ALGORITHM_STRICT_RULE: + if (!self::isValidAlgorithm($keyInfo['type'])) { + $validationErrors[$ruleName] = __('The algorithm is invalid.'); + } + break; + case self::IS_VALID_FINGERPRINT_RULE: + if (!self::isValidFingerprint($keyInfo['fingerprint'])) { + $validationErrors[$ruleName] = __('The fingerprint id is invalid.'); + } + break; + case self::IS_VALID_KEY_ID_RULE: + if (!self::isValidKeyId($keyInfo['key_id'])) { + $validationErrors[$ruleName] = __('The public key id is invalid.'); + } + break; + case self::IS_VALID_KEY_SIZE_RULE: + if (!self::isRecommendedSize($keyInfo['type'], $keyInfo['bits'], false)) { + $validationErrors[$ruleName] = __('The key size is not valid.'); + } + break; + case self::IS_VALID_KEY_SIZE_STRICT_RULE: + if (!self::isRecommendedSize($keyInfo['type'], $keyInfo['bits'])) { + $validationErrors[$ruleName] = __('The key size must match security recommendations.'); + } + break; + case self::IS_NOT_CREATED_IN_THE_FUTURE_RULE: + if (self::isDateInFuture($keyInfo['key_created'])) { + $validationErrors[$ruleName] = __('The key creation date must not be in the future.'); + } + break; + case self::IS_NOT_EXPIRED_RULE: + if (isset($keyInfo['expires']) && !self::isDateInFuture($keyInfo['expires'])) { + $validationErrors[$ruleName] = __('The key must not be expired.'); + } + break; + case self::HAS_NO_EXPIRY_DATE_RULE: + if (isset($keyInfo['expires'])) { + $validationErrors[$ruleName] = __('The key must not include an expiry date.'); + } + break; + case self::IS_REVOKED_RULE: + if (!$keyInfo['revoked']) { + $validationErrors[$ruleName] = __('The key must be revoked.'); + } + break; + case self::IS_NOT_REVOKED_RULE: + if ($keyInfo['revoked']) { + $validationErrors[$ruleName] = __('The key must not be revoked.'); + } + break; + case self::HAS_NO_EXTRA_BREAK_LINE_RULE: + if (self::hasExtraBreakline($armoredKey)) { + $msg = __('The armored key must not contain an extra line before the end block.'); + $validationErrors[$ruleName] = $msg; + } + break; + case self::HAS_MULTIPLE_MAIN_PACKETS_RULE: + if ($keyInfo['public_key_packet_counts'] > 1 || $keyInfo['secret_key_packet_counts'] > 1) { + $msg = __('The armored key must not contain multiple keys.'); + $validationErrors[$ruleName] = $msg; + } + break; + default: + throw new InternalErrorException(__('Unknown key validation rule: {0}', $ruleName)); + } + } + + // Wrap all errors together in a custom validation exception + if (count($validationErrors)) { + throw new CustomValidationException(__('The public key could not be validated.'), [ + 'armored_key' => $validationErrors, + ]); + } + + // Or return key information for further use + // for example for it to be saved in a model + return $keyInfo; + } + + /** + * Return true if the date is in the future, false if in the past or not a valid date + * + * @param string|int|null $datetimeString user provider data + * @return bool + */ + public static function isDateInFuture(string|int|null $datetimeString = null): bool + { + if (!isset($datetimeString) || (!is_string($datetimeString) && !is_int($datetimeString))) { + return false; + } + + $frozenTime = new DateTime($datetimeString); + + return $frozenTime->greaterThan(DateTime::now()->addSeconds(300)); + } + + /** + * Returns true if the key is of the recommended size + * + * @param string $algorithm RSA, ECC, etc. + * @param int $keySize 2048, etc. + * @param bool $strict default true + * @return bool if key size is valid + */ + public static function isRecommendedSize(string $algorithm, int $keySize, bool $strict = true): bool + { + if ($algorithm === 'RSA') { + if ($strict) { + // As recommended in CURE53 audit PBL-01-005 and ANSSI + // Ref. https://www.ssi.gouv.fr/entreprise/guide/mecanismes-cryptographiques/ + return $keySize == 3072 || $keySize == 4096; + } else { + return $keySize == 2048 || $keySize == 3072 || $keySize == 4096; + } + } else { + // No preferences yet for other algorithms such as ECC + return true; + } + } + + /** + * Custom validation rule to validate key id + * + * /!\ Note that short key ids are considered insecure + * In any case, we recommend using fingerprint instead. + * We use this method as a sanity check only + * + * @param string|null $keyId user provided data + * @param bool $strict default false for backward compatibility + * @return bool + */ + public static function isValidKeyId(?string $keyId = null, bool $strict = false): bool + { + if (!$strict) { + return self::isValidShortKeyId($keyId) || self::isValidLongKeyId($keyId); + } else { + return self::isValidLongKeyId($keyId); + } + } + + /** + * Custom validation rule to validate short key id + * Note: short key ids are considered insecure + * + * @param string|null $keyId user provided data + * @return bool + */ + public static function isValidShortKeyId(?string $keyId = null): bool + { + if (!isset($keyId)) { + return false; + } + + return preg_match('/^[A-F0-9]{8}$/', $keyId) === 1; + } + + /** + * Custom validation rule to validate key id + * + * @param string|null $keyId user provided data + * @return bool + */ + public static function isValidLongKeyId(?string $keyId = null): bool + { + if (!isset($keyId)) { + return false; + } + + return preg_match('/^[A-F0-9]{16}$/', $keyId) === 1; + } + + /** + * Custom validation rule to check if armored key block is parsable + * + * @param string|null $armoredKey user provided data + * @return bool + */ + public static function isParsableArmoredPublicKey(?string $armoredKey = null): bool + { + if (!isset($armoredKey)) { + return false; + } + + return OpenPGPBackendFactory::get()->isParsableArmoredPublicKey($armoredKey); + } + + /** + * Get Public Key Info + * + * @param string $armoredKey user provided data + * @return array see OpenPGPBackendInterface::getKeyInfo + * @throws \Cake\Core\Exception\CakeException if the armored key cannot be parsed + */ + public static function getPublicKeyInfo(string $armoredKey): array + { + $key = hash('sha256', $armoredKey); + + if (array_key_exists($key, self::$publicKeyInfo) && !empty(self::$publicKeyInfo[$key])) { + return self::$publicKeyInfo[$key]; + } + + $keyInfo = OpenPGPBackendFactory::get()->getPublicKeyInfo($armoredKey); + // Cache key info, so it can be fetched fast next time + self::$publicKeyInfo[$key] = $keyInfo; + + return $keyInfo; + } + + /** + * Return true if string is a valid fingerprint + * + * @param string|null $fingerprint user provided data + * @return bool + */ + public static function isValidFingerprint(?string $fingerprint = null): bool + { + return OpenPGPBackendFactory::get()->isValidFingerprint($fingerprint); + } + + /** + * Custom validation rule to validate email inside a OpenPGP key UID string + * + * @param string|null $value openpgp key uid + * @return bool + */ + public static function uidContainValidEmail(?string $value = null): bool + { + if (!isset($value)) { + return false; + } + preg_match('/<(\S+@\S+)>$/', $value, $matches); + if (isset($matches[1])) { + return EmailValidationRule::check($matches[1]); + } + + return false; + } + + /** + * Get email from Uid + * + * @param string|null $value openpgp key uid + * @return string|bool + */ + public static function getEmailFromUid(?string $value = null): string|bool + { + if (!isset($value)) { + return false; + } + preg_match('/<(\S+@\S+)>$/', $value, $matches); + if (isset($matches[1])) { + return $matches[1]; + } + + return false; + } + + /** + * Return true if email address is included in key uid + * + * @param string|null $email email + * @param string|null $uid uid + * @return bool + */ + public static function isEmailInUid(?string $email, ?string $uid): bool + { + if (!empty($email) && EmailValidationRule::check($email)) { + $uidEmail = self::getEmailFromUid($uid); + if ($uidEmail !== false) { + return $email == $uidEmail; + } + } + + return false; + } + + /** + * Custom validation rule to validate key algorithm + * + * @param string|null $algorithm RSA, ECC, EdDSA, etc. see OpenPGP_PublicKeyPacket::$algorithms. + * @param bool $strict exclude DSA and ELGAMAL, default true + * @return bool + */ + public static function isValidAlgorithm(?string $algorithm = null, bool $strict = true): bool + { + if (!isset($algorithm)) { + return false; + } + $supported = OpenPGP_PublicKeyPacket::$algorithms; + if ($strict === true) { + // Minus legacy items such as DSA, ELGAMAL + // Default in openpgp.js v5 + unset($supported[16]); + unset($supported[17]); + } + foreach ($supported as $a) { + if ($algorithm === $a) { + return true; + } + } + + return false; + } + + /** + * Key with extra breakline after checksum and before the end block + * are known to cause compatibility issues with gopenpgp + * + * @param string $publicKey armored format + * @return bool + */ + public static function hasExtraBreakLine(string $publicKey): bool + { + return OpenPGPBackendFactory::get()->hasExtraBreakLine($publicKey); + } +}