Merge-up 3.4.x → 4.0.x (security fixes + accumulated backports)#652
Merged
Conversation
* Add Base64UrlSafe utility and refactor code references This commit adds the utility `Base64UrlSafe` to provide methods for encoding and decoding in Base64UrlSafe format. Simultaneously, the commit also updates multiple files across the library to use this utility instead of the previously used `ParagonIE\ConstantTime\Base64UrlSafe`. This change will ensure consistent use of this utility throughout the project, promoting maintainability and ease of updating in the future, if required.
* Add RangeException to Base64UrlSafe The code in src/Library/Core/Util/Base64UrlSafe.php has been updated to include the use of RangeException. This will further expand its capability in terms of handling exceptional scenarios.
* Add failing tests for JWECollector without compression * Fix call function on null error --------- Co-authored-by: Peter Mead <peter.mead@staysafeapp.com>
PBES2AESKW::unwrapKey read the "p2c" (iteration count) parameter directly from the attacker-controlled JOSE header and fed it to hash_pbkdf2() with no upper bound (the only check was is_int && > 0). A single crafted JWE with a large "p2c" (e.g. 100_000_000) could pin a worker for minutes, enabling an unauthenticated denial-of-service. This adds a configurable maximum iteration count (DEFAULT_MAX_COUNT = 1_000_000, generous vs. realistic legitimate values which are a few thousand) enforced in checkHeaderAdditionalParameters() before any PBKDF2 work. The bound is the third constructor argument so operators can tune it. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Chacha20Poly1305 key-encryption algorithm generated the 16-byte Poly1305 authentication tag during encryptKey() but discarded it: it was never written to the header, so it never reached the wire. decryptKey() then called openssl_decrypt() without the tag argument, which makes OpenSSL skip authentication entirely. The AEAD was therefore degraded to unauthenticated ChaCha20: a tampered encrypted CEK was accepted and a single-byte change in the ciphertext propagated unchecked into the CEK. encryptKey() now publishes the tag as the base64url "tag" header parameter (and verifies it is 16 bytes). decryptKey() requires the "tag" header, validates its length, and passes it to openssl_decrypt() so the Poly1305 tag is actually verified; tampering now yields a decryption failure. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RSACrypt::decryptWithRSA15() validated the PKCS#1 v1.5 padding after RSADP and threw InvalidArgumentException as soon as it was malformed, with no implicit-rejection countermeasure. From a JWE caller this exposed a Bleichenbacher/Marvin padding oracle: padding-rejected, padding-valid- wrong-length and padding-valid-full-AEAD ciphertexts all return the same false from JWEDecrypter but perform measurably different amounts of work (amplifiable by enlarging the ciphertext), leaking the PKCS#1 conformance bit through timing. RSACrypt::decrypt()/decryptWithRSA15() now accept an expected key length. When provided, PKCS#1 v1.5 decryption validates the padding in constant time and selects, also in constant time, either the recovered message (valid padding AND expected length) or a freshly generated random key of the expected length. No exception is thrown for padding failures, so the downstream content decryption (AEAD) always runs and fails uniformly. RSA15::decryptKey() supplies the expected CEK length derived from the "enc" header (mirroring the content encryption algorithms' CEK sizes). The legacy throwing behaviour is preserved when no expected length is given, for direct (non-JWE) callers. RSA-OAEP is unaffected. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JWSVerifier::getAlgorithm() merged the protected and unprotected headers with [...$protected, ...$unprotected]; with duplicate keys PHP keeps the LAST value, so the attacker-controlled unprotected header could override the integrity-protected "alg". Combined with HeaderCheckerManager (which validates "alg" from the protected header), this is a TOCTOU algorithm-confusion / downgrade vector (e.g. forcing HS256 against an RSA public key, or HS512 -> HS256), and "alg" placed only in the unprotected header bypassed the duplicate-parameter check entirely. Per RFC 7515 §4.1.1 "alg" MUST be integrity protected. getAlgorithm() now reads "alg" exclusively from the protected header and rejects a JWS whose "alg" is absent from (or present only outside) the protected header. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Following the algorithm-confusion fix (GHSA-jc38-x7x8-2xc8), JWSVerifier reads "alg" only from the integrity-protected header and rejects a JWS whose "alg" is absent from it. Three existing tests asserted the previous behaviour where "alg" could come from the unprotected header (RFC 7520 §4.7 / §4.8 examples); they now assert that such tokens are rejected, and the expected exception message is updated accordingly. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* 3.4.x: Update signature tests for the protected-header "alg" requirement (#651) Merge commit from fork Merge commit from fork Merge commit from fork Merge commit from fork Add sodium support for Base64 URL safe encoding/decoding (#644) Allow `psr/cache` v2 (#620) Fix call function on null (#596) Add RangeException to Base64UrlSafe (#577) Add Base64UrlSafe utility and refactor code references (#576) # Conflicts: # composer.json # phpstan-baseline.neon # src/Bundle/DataCollector/JWECollector.php # src/Experimental/KeyEncryption/AESCTR.php # src/Experimental/KeyEncryption/Chacha20Poly1305.php # src/Experimental/Signature/Blake2b.php # src/Library/Console/GeneratorCommand.php # src/Library/Core/JWK.php # src/Library/Core/Util/Base64UrlSafe.php # src/Library/Encryption/Algorithm/ContentEncryption/AESCBCHS.php # src/Library/Encryption/Algorithm/ContentEncryption/AESGCM.php # src/Library/Encryption/Algorithm/KeyEncryption/AESGCMKW.php # src/Library/Encryption/Algorithm/KeyEncryption/AESKW.php # src/Library/Encryption/Algorithm/KeyEncryption/AbstractECDH.php # src/Library/Encryption/Algorithm/KeyEncryption/Dir.php # src/Library/Encryption/Algorithm/KeyEncryption/PBES2AESKW.php # src/Library/Encryption/Algorithm/KeyEncryption/RSA15.php # src/Library/Encryption/Algorithm/KeyEncryption/Util/ConcatKDF.php # src/Library/Encryption/Algorithm/KeyEncryption/Util/RSACrypt.php # src/Library/Encryption/Serializer/CompactSerializer.php # src/Library/Encryption/Serializer/JSONFlattenedSerializer.php # src/Library/Encryption/Serializer/JSONGeneralSerializer.php # src/Library/KeyManagement/Analyzer/ESKeyAnalyzer.php # src/Library/KeyManagement/Analyzer/HSKeyAnalyzer.php # src/Library/KeyManagement/Analyzer/OctAnalyzer.php # src/Library/KeyManagement/Analyzer/RsaAnalyzer.php # src/Library/KeyManagement/Analyzer/ZxcvbnKeyAnalyzer.php # src/Library/Signature/Algorithm/EdDSA.php # src/Library/Signature/Algorithm/HMAC.php # src/Library/Signature/Serializer/CompactSerializer.php # src/Library/Signature/Serializer/JSONFlattenedSerializer.php # src/Library/Signature/Serializer/JSONGeneralSerializer.php # src/Library/composer.json # tests/Bundle/JoseFramework/Functional/Encryption/JWECollectorTest.php # tests/Bundle/JoseFramework/Functional/KeyManagement/JWKLoaderTest.php # tests/Component/Encryption/RFC7520/A128KWAndA128GCMEncryptionWithCompressionTest.php # tests/Component/KeyManagement/JWKFactoryTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Cascade 3.4.x → 4.0.x. Brings up the 4 security fixes (advisories) already on
3.4.x, together with 5 older commits that had never been merged up (#576, #577, #596, #620, #644).Security fixes brought up
p2c(CPU-amplification DoS).algis read only from the integrity-protected header (RFC 7515 §4.1.1); the 3 RFC 7520 tests were updated into rejection tests (Update signature tests for the protected-header "alg" requirement #651).Conflict resolution (37 files)
use Override;) → kept the 4.0.x version.composer.json×2,phpstan-baseline.neon) → kept the 4.0.x version (PHP ≥ 8.2, 4.x constraints).Base64UrlSafe.php→ took the4.1.xversion (reconciles the sodium support from Add sodium support for Base64 URL safe encoding/decoding #644 with thefinal readonlystyle already adopted on 4.x).A128KW…WithCompressionTest) → removed (compression was dropped on 4.x — no deprecation carried over).strleninstead ofmb_*,readonly,#[Override])..gitsplit.yml→ kept the 4.0.x version (the^\d+\.\d+\.x$origins are required to split the 4.x tags; the(1|2|3)restriction from 3.4.x must NOT be propagated).Validation
OK (234 tests, 933 assertions).4.0.x(thesymfony/property-infodependency is missing from the local vendor), identical between the pristine branch and the merge → no regression introduced.