Bugfix: Ensure only the allowed elements are passed into KeyInfo

simplesamlphp · Sep 16, 2024 · 71aa204 · 71aa204
1 parent 764a2a7
commit 71aa204
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 16 deletions.
diff --git a/src/XML/ds/KeyInfo.php b/src/XML/ds/KeyInfo.php
@@ -34,30 +34,32 @@ final class KeyInfo extends AbstractDsElement
      *     \SimpleSAML\XMLSecurity\XML\ds\KeyValue|
      *     \SimpleSAML\XMLSecurity\XML\ds\RetrievalMethod|
      *     \SimpleSAML\XMLSecurity\XML\ds\X509Data|
-     *     \SimpleSAML\XMLSecurity\XML\dsig11\KeyInfoReference|
-     *     \SimpleSAML\XMLSecurity\XML\xenc\EncryptedData|
-     *     \SimpleSAML\XMLSecurity\XML\xenc\EncryptedKey
+     *     \SimpleSAML\XML\SerializableElementInterface
      * )[] $info
-     * @param \SimpleSAML\XML\SerializableElementInterface[] $children
      * @param string|null $Id
      */
     public function __construct(
         protected array $info,
-        array $children = [],
         protected ?string $Id = null,
     ) {
-        $combi = array_merge($info, $children);
-
-        Assert::notEmpty($combi, 'ds:KeyInfo cannot be empty', InvalidArgumentException::class);
-        Assert::maxCount($combi, C::UNBOUNDED_LIMIT);
+        Assert::notEmpty($info, 'ds:KeyInfo cannot be empty', InvalidArgumentException::class);
+        Assert::maxCount($info, C::UNBOUNDED_LIMIT);
         Assert::allIsInstanceOf(
-            $combi,
+            $info,
             SerializableElementInterface::class,
             InvalidArgumentException::class,
         );
         Assert::nullOrValidNCName($Id);
 
-        $this->setElements($children);
+        foreach ($info as $item) {
+            if ($item->getNamespaceURI() === static::NS) {
+                Assert::isInstanceOfAny(
+                    $item,
+                    [KeyName::class, KeyValue::class, RetrievalMethod::class, X509Data::class],
+                    SchemaViolationException::class,
+                );
+            }
+        }
     }
 
 
@@ -79,7 +81,7 @@ public function getId(): ?string
      */
     public function getInfo(): array
     {
-        return array_merge($this->info, $this->getElements());
+        return $this->info;
     }
 
 
@@ -106,6 +108,7 @@ public static function fromXML(DOMElement $xml): static
         //$pgpData = PGPData::getChildrenOfClass($xml);
         //$spkiData = SPKIData::getChildrenOfClass($xml);
         //$mgmtData = MgmtData::getChildrenOfClass($xml);
+        $other = self::getChildElementsFromXML($xml);
 
         $info = array_merge(
             $keyName,
@@ -115,10 +118,10 @@ public static function fromXML(DOMElement $xml): static
             //$pgpdata,
             //$spkidata,
             //$mgmtdata,
+            $other,
         );
 
-        $children = self::getChildElementsFromXML($xml);
-        return new static($info, $children, $Id);
+        return new static($info, $Id);
     }
 
 

diff --git a/tests/XML/ds/KeyInfoTest.php b/tests/XML/ds/KeyInfoTest.php
@@ -94,8 +94,6 @@ public function testMarshalling(): void
                         new X509SubjectName(self::$certData['name']),
                     ],
                 ),
-            ],
-            [
                 new Chunk(DOMDocumentFactory::fromString(
                     '<ssp:Chunk xmlns:ssp="urn:x-simplesamlphp:namespace">some</ssp:Chunk>',
                 )->documentElement),