Implement signature verification, refactor signing

This commit adds support for verifying signatures (and renames the API method to verify(), which is the correct term we should us in this context, instead of validate()).

It also refactors the way we sign, and adds support for canonicalization algorithms with comments, which were in practice not supported before.

Signed-off-by: Jaime Pérez <jaimepc@gmail.com>

Author: jaimeperez

src/XML/SignableElementTrait.php

@@ -5,8 +5,8 @@
 namespace SimpleSAML\XMLSecurity\XML;
 
 use DOMElement;
-use DOMNode;
 use SimpleSAML\Assert\Assert;
+use SimpleSAML\XML\DOMDocumentFactory;
 use SimpleSAML\XMLSecurity\Alg\SignatureAlgorithm;
 use SimpleSAML\XMLSecurity\Constants as C;
 use SimpleSAML\XMLSecurity\Exception\InvalidArgumentException;
@@ -89,10 +89,69 @@ public function sign(
 
 
     /**
-     * @param \DOMElement $xml
-     * @throws \Exception
+     * Get a ds:Reference pointing to this object.
+     *
+     * @param string $digestAlg The digest algorithm to use.
+     * @param \SimpleSAML\XMLSecurity\XML\ds\Transforms $transforms The transforms to apply to the object.
      */
-    private function doSign(DOMElement $xml): void
+    private function getReference(
+        string $digestAlg,
+        Transforms $transforms,
+        DOMElement $xml,
+        string $canonicalDocument
+    ): Reference {
+        $id = $this->getId();
+        $uri = null;
+        if (empty($id)) { // document reference
+            Assert::notNull(
+                $xml->ownerDocument->documentElement,
+                'Cannot create a document reference without a root element in the document.',
+                RuntimeException::class
+            );
+            Assert::true(
+                $xml->isSameNode($xml->ownerDocument->documentElement),
+                'Cannot create a document reference when signing an object that is not the root of the document. ' .
+                'Please give your object an identifier.',
+                RuntimeException::class
+            );
+            if (in_array($this->c14nAlg, [C::C14N_INCLUSIVE_WITH_COMMENTS, C::C14N_EXCLUSIVE_WITH_COMMENTS])) {
+                $uri = '#xpointer(/)';
+            }
+        } elseif (in_array($this->c14nAlg, [C::C14N_INCLUSIVE_WITH_COMMENTS, C::C14N_EXCLUSIVE_WITH_COMMENTS])) {
+            // regular reference, but must retain comments
+            $uri = '#xpointer(id(' . $id . '))';
+        } else { // regular reference, can ignore comments
+            $uri = '#' . $id;
+        }
+
+        return new Reference(
+            new DigestMethod($digestAlg),
+            new DigestValue(Security::hash($digestAlg, $canonicalDocument)),
+            $transforms,
+            null,
+            null,
+            $uri
+        );
+    }
+
+
+    /**
+     * Do the actual signing of the document.
+     *
+     * Note that this method does not insert the signature in the returned \DOMElement. The signature will be available
+     * in $this->signature as a \SimpleSAML\XMLSecurity\XML\ds\Signature object, which can then be converted to XML
+     * calling toXML() on it, passing the \DOMElement value returned here as a parameter. The resulting \DOMElement
+     * can then be inserted in the position desired.
+     *
+     * E.g.:
+     *     $xml = // our XML to sign
+     *     $signedXML = $this->doSign($xml);
+     *     $signedXML->appendChild($this->signature->toXML($signedXML));
+     *
+     * @param \DOMElement $xml The element to sign.
+     * @return \DOMElement The signed element, without the signature attached to it just yet.
+     */
+    protected function doSign(DOMElement $xml): DOMElement
     {
         Assert::notNull(
             $this->signer,
@@ -108,38 +167,18 @@ private function doSign(DOMElement $xml): void
             new Transform($this->c14nAlg)
         ]);
 
-        $refId = $this->getId();
-        $reference = new Reference(
-            new DigestMethod($digest),
-            new DigestValue(Security::hash($digest, XML::processTransforms($transforms, $xml))),
-            $transforms,
-            null,
-            null,
-            ($refId !== null) ? '#' . $refId : null
-        );
+        $canonicalDocument = XML::processTransforms($transforms, $xml);
 
         $signedInfo = new SignedInfo(
             new CanonicalizationMethod($this->c14nAlg),
             new SignatureMethod($algorithm),
-            [$reference]
+            [$this->getReference($digest, $transforms, $xml, $canonicalDocument)]
         );
 
         $signingData = $signedInfo->canonicalize($this->c14nAlg);
         $signedData = base64_encode($this->signer->sign($signingData));
 
         $this->signature = new Signature($signedInfo, new SignatureValue($signedData), $this->keyInfo);
-    }
-
-
-    /**
-     * @param DOMElement $root
-     * @param DOMNode $node
-     * @param DOMElement $signature
-     * @return DOMElement
-     */
-    private function insertBefore(DOMElement $root, DOMNode $node, DOMElement $signature): DOMElement
-    {
-        $root->removeChild($signature);
-        return $root->insertBefore($signature, $node);
+        return DOMDocumentFactory::fromString($canonicalDocument)->documentElement;
     }
 }

src/XML/SignedElementInterface.php

@@ -4,7 +4,9 @@
 
 namespace SimpleSAML\XMLSecurity\XML;
 
-use SimpleSAML\XMLSecurity\XMLSecurityKey;
+use SimpleSAML\XMLSecurity\Alg\SignatureAlgorithm;
+use SimpleSAML\XMLSecurity\Key\AbstractKey;
+use SimpleSAML\XMLSecurity\XML\ds\Signature;
 
 /**
  * An interface describing signed elements.
@@ -14,12 +16,40 @@
 interface SignedElementInterface extends CanonicalizableElementInterface
 {
     /**
-     * Retrieve certificates that sign this element.
+     * Get the ID of this element.
      *
-     * @return array Array with certificates.
+     * When this method returns null, the signature created for this object will reference the entire document.
+     *
+     * @return string|null The ID of this element, or null if we don't have one.
+     */
+    public function getId(): ?string;
+
+
+    /**
+     * Retrieve the signature in this object, if any.
+     *
+     * @return Signature|null
+     */
+    public function getSignature(): ?Signature;
+
+
+    /**
+     * Retrieve the key used to validate the signature in this object.
+     *
+     * Warning:
+     *
+     * @return \SimpleSAML\XMLSecurity\Key\AbstractKey The key that validated the signature in this object.
      * @throws \Exception if an error occurs while trying to extract the public key from a certificate.
      */
-    public function getValidatingCertificates(): array;
+    public function getValidatingKey(): ?AbstractKey;
+
+
+    /**
+     * Whether this object is signed or not.
+     *
+     * @return bool
+     */
+    public function isSigned(): bool;
 
 
     /**
@@ -28,8 +58,14 @@ public function getValidatingCertificates(): array;
      * If no signature is present, false is returned. If a signature is present,
      * but cannot be verified, an exception will be thrown.
      *
-     * @param \SimpleSAML\XMLSecurity\XMLSecurityKey $key The key we should check against.
-     * @return \SimpleSAML\XMLSecurity\XML\SignedElementInterface The signed element if we can verify the signature.
+     * @param \SimpleSAML\XMLSecurity\Alg\SignatureAlgorithm|null $verifier The verifier to use to verify the signature.
+     * If null, attempt to verify it with the KeyInfo information in the signature.
+     * @return \SimpleSAML\XMLSecurity\XML\SignedElementInterface The object processed again from its canonicalised
+     * representation verified by the signature.
+     * @throws \SimpleSAML\XMLSecurity\Exception\NoSignatureFound if the object is not signed.
+     * @throws \SimpleSAML\XMLSecurity\Exception\InvalidArgumentException if no key is passed and there is no KeyInfo
+     * in the signature.
+     * @throws \SimpleSAML\XMLSecurity\Exception\RuntimeException if the signature fails to validate.
      */
-    public function validate(XMLSecurityKey $key): SignedElementInterface;
+    public function verify(SignatureAlgorithm $verifier = null): SignedElementInterface;
 }