fix(encryption): enforce the disjoint header requirement of RFC 7516

JWEDecrypter built the complete header with array_merge($sharedProtectedHeader,
$sharedHeader, $recipientHeader). Because of the last-wins behaviour of array_merge,
an unprotected header parameter was able to override an integrity protected one, "alg"
and "enc" included: the HeaderCheckerManager validated the protected values while the
decryption used the unprotected ones.

RFC 7516 section 7.2.1 requires that "the Header Parameter names in the three locations
MUST be disjoint". That requirement is now enforced when the token is read, as it already
was when it is written, and the merge order is reversed so that the protected header wins.

"alg" and "enc" are also no longer read from the shared unprotected header. The RFC puts
no location constraint on them, but that header is not covered by the AAD and, unlike the
per-recipient header, nothing requires those parameters to be located there. As a
consequence the RFC 7520 sections 5.11 and 5.12 vectors are still parsed but are no longer
decrypted; the corresponding tests now assert the refusal.

JWEBuilder could produce a token violating the disjoint requirement: with several
recipients, the header parameters computed by the key encryption algorithm are added to
the per-recipient header without looking at the shared headers. A PBES2 token carrying
"p2c" in the shared protected header ended up with "p2c" in both places, and was already
rejected by the HeaderCheckerManager. Those parameters are now filtered, which is the
behaviour of the single recipient path.

Also adds non-empty string guards on "alg" and "enc" and fixes the error message of
getContentEncryptionAlgorithm which mentioned the key encryption algorithm.

Co-authored-by: Ross Addison <rossaddison@users.noreply.github.com>

Author: Spomky

src/Library/Encryption/JWEBuilder.php

@@ -257,6 +257,12 @@ private function checkAndSetContentEncryptionAlgorithm(array $completeHeader): v
         }
     }
 
+    /**
+     * The header parameters computed by the key encryption algorithm are added to the per-recipient header
+     * when there is more than one recipient. Those already set in a shared header are filtered out: the
+     * header parameter names of the three headers must be disjoint (RFC 7516 section 7.2.1), and a shared
+     * value takes precedence, as it does with a single recipient.
+     */
     private function processRecipient(array $recipient, string $cek, array &$additionalHeader): Recipient
     {
         $completeHeader = array_merge($this->sharedHeader, $recipient['header'], $this->sharedProtectedHeader);
@@ -274,6 +280,7 @@ private function processRecipient(array $recipient, string $cek, array &$additio
         );
         $recipientHeader = $recipient['header'];
         if ((is_countable($additionalHeader) ? count($additionalHeader) : 0) !== 0 && count($this->recipients) !== 1) {
+            $additionalHeader = array_diff_key($additionalHeader, $this->sharedProtectedHeader, $this->sharedHeader);
             $recipientHeader = array_merge($recipientHeader, $additionalHeader);
             $additionalHeader = [];
         }

src/Library/Encryption/JWEDecrypter.php

@@ -18,6 +18,7 @@
 use Jose\Component\Encryption\Algorithm\KeyEncryption\KeyWrapping;
 use Jose\Component\Encryption\Algorithm\KeyEncryptionAlgorithm;
 use Throwable;
+use function count;
 use function is_string;
 use function sprintf;
 use function strlen;
@@ -109,6 +110,15 @@ public function decryptUsingKeySet(
         return false;
     }
 
+    /**
+     * The header parameter names of the shared protected header, the shared unprotected header and the
+     * per-recipient header must be disjoint (RFC 7516 section 7.2.1), as enforced by the JWEBuilder when the
+     * token is created. Otherwise an unprotected parameter is able to redefine a protected one. The headers
+     * are then merged in the same order as the JWEBuilder does, so that the protected header always wins.
+     *
+     * The shared unprotected header is never a valid source for "alg" and "enc": it is not covered by the
+     * AAD and, unlike the per-recipient header, nothing requires those parameters to be located there.
+     */
     private function decryptRecipientKey(
         JWE $jwe,
         JWKSet $jwkset,
@@ -117,15 +127,20 @@ private function decryptRecipientKey(
         ?JWK $senderKey = null
     ): ?string {
         $recipient = $jwe->getRecipient($i);
-        $completeHeader = array_merge(
-            $jwe->getSharedProtectedHeader(),
-            $jwe->getSharedHeader(),
-            $recipient->getHeader()
-        );
+        $sharedProtectedHeader = $jwe->getSharedProtectedHeader();
+        $sharedHeader = $jwe->getSharedHeader();
+        $recipientHeader = $recipient->getHeader();
+
+        $this->checkDuplicatedHeaderParameters($sharedProtectedHeader, $sharedHeader);
+        $this->checkDuplicatedHeaderParameters($sharedProtectedHeader, $recipientHeader);
+        $this->checkDuplicatedHeaderParameters($sharedHeader, $recipientHeader);
+
+        $completeHeader = array_merge($sharedHeader, $recipientHeader, $sharedProtectedHeader);
         $this->checkCompleteHeader($completeHeader);
 
-        $key_encryption_algorithm = $this->getKeyEncryptionAlgorithm($completeHeader);
-        $content_encryption_algorithm = $this->getContentEncryptionAlgorithm($completeHeader);
+        $protectedAndRecipientHeader = array_merge($recipientHeader, $sharedProtectedHeader);
+        $key_encryption_algorithm = $this->getKeyEncryptionAlgorithm($protectedAndRecipientHeader);
+        $content_encryption_algorithm = $this->getContentEncryptionAlgorithm($protectedAndRecipientHeader);
 
         $this->checkIvSize($jwe->getIV(), $content_encryption_algorithm->getIVSize());
 
@@ -253,29 +268,52 @@ private function checkCompleteHeader(array $completeHeaders): void
         }
     }
 
-    private function getKeyEncryptionAlgorithm(array $completeHeaders): KeyEncryptionAlgorithm
+    private function getKeyEncryptionAlgorithm(array $header): KeyEncryptionAlgorithm
     {
-        $key_encryption_algorithm = $this->keyEncryptionAlgorithmManager->get($completeHeaders['alg']);
+        $alg = $header['alg'] ?? null;
+        if (! is_string($alg) || $alg === '') {
+            throw new InvalidArgumentException(
+                'The "alg" parameter must be a non-empty string set in the protected header or in the recipient header.'
+            );
+        }
+        $key_encryption_algorithm = $this->keyEncryptionAlgorithmManager->get($alg);
         if (! $key_encryption_algorithm instanceof KeyEncryptionAlgorithm) {
             throw new InvalidArgumentException(sprintf(
                 'The key encryption algorithm "%s" is not supported or does not implement KeyEncryptionAlgorithm interface.',
-                $completeHeaders['alg']
+                $alg
             ));
         }
 
         return $key_encryption_algorithm;
     }
 
-    private function getContentEncryptionAlgorithm(array $completeHeader): ContentEncryptionAlgorithm
+    private function getContentEncryptionAlgorithm(array $header): ContentEncryptionAlgorithm
     {
-        $content_encryption_algorithm = $this->contentEncryptionAlgorithmManager->get($completeHeader['enc']);
+        $enc = $header['enc'] ?? null;
+        if (! is_string($enc) || $enc === '') {
+            throw new InvalidArgumentException(
+                'The "enc" parameter must be a non-empty string set in the protected header or in the recipient header.'
+            );
+        }
+        $content_encryption_algorithm = $this->contentEncryptionAlgorithmManager->get($enc);
         if (! $content_encryption_algorithm instanceof ContentEncryptionAlgorithm) {
             throw new InvalidArgumentException(sprintf(
-                'The key encryption algorithm "%s" is not supported or does not implement the ContentEncryption interface.',
-                $completeHeader['enc']
+                'The content encryption algorithm "%s" is not supported or does not implement the ContentEncryption interface.',
+                $enc
             ));
         }
 
         return $content_encryption_algorithm;
     }
+
+    private function checkDuplicatedHeaderParameters(array $header1, array $header2): void
+    {
+        $inter = array_intersect_key($header1, $header2);
+        if (count($inter) !== 0) {
+            throw new InvalidArgumentException(sprintf(
+                'The header contains duplicated entries: %s.',
+                implode(', ', array_keys($inter))
+            ));
+        }
+    }
 }