From 9c65cfc9e3522b253fad2a5d9bd811e2197116c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Kr=C3=BCger?= Date: Tue, 9 Jun 2026 22:48:41 +0200 Subject: [PATCH] =?UTF-8?q?revert(passbolt):=20remove=20clock-skew=20patch?= =?UTF-8?q?=20=E2=80=94=20metadata=20key=20already=20created?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The patched PublicKeyValidationService.php and its volume mount are no longer needed now that the metadata key exists in the database. Co-Authored-By: Claude Sonnet 4.6 --- passbolt/compose.yml | 2 - .../patches/PublicKeyValidationService.php | 487 ------------------ 2 files changed, 489 deletions(-) delete mode 100644 passbolt/patches/PublicKeyValidationService.php diff --git a/passbolt/compose.yml b/passbolt/compose.yml index b613c1d..8fd9d12 100644 --- a/passbolt/compose.yml +++ b/passbolt/compose.yml @@ -24,8 +24,6 @@ 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 deleted file mode 100644 index fe6bf5a..0000000 --- a/passbolt/patches/PublicKeyValidationService.php +++ /dev/null @@ -1,487 +0,0 @@ - [ - 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); - } -}