Support X509 cert comments

SAML-Toolkits · May 19, 2024 · 4b2417c · 4b2417c
1 parent 2216ccb
commit 4b2417c
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 47 deletions.
diff --git a/src/Saml2/Utils.php b/src/Saml2/Utils.php
@@ -212,28 +212,27 @@ public static function treeCopyReplace(DomNode $targetNode, DomNode $sourceNode,
     /**
      * Returns a x509 cert (adding header & footer if required).
      *
-     * @param string $cert  A x509 unformated cert
-     * @param bool   $heads True if we want to include head and footer
+     * @param string $x509cert  A x509 unformated cert
+     * @param bool   $heads     True if we want to include head and footer
      *
      * @return string $x509 Formatted cert
      */
-    public static function formatCert($cert, $heads = true)
+    public static function formatCert($x509cert, $heads = true)
     {
-        if (is_null($cert)) {
+        if (is_null($x509cert)) {
           return;
         }
 
-        $x509cert = str_replace(array("\x0D", "\r", "\n"), "", $cert);
-        if (!empty($x509cert)) {
-            $x509cert = str_replace('-----BEGIN CERTIFICATE-----', "", $x509cert);
-            $x509cert = str_replace('-----END CERTIFICATE-----', "", $x509cert);
-            $x509cert = str_replace(' ', '', $x509cert);
+        if (strpos($x509cert, '-----BEGIN CERTIFICATE-----') !== false) {
+            $x509cert = static::getStringBetween($x509cert, '-----BEGIN CERTIFICATE-----', '-----END CERTIFICATE-----');
+        }
 
-            if ($heads) {
-                $x509cert = "-----BEGIN CERTIFICATE-----\n".chunk_split($x509cert, 64, "\n")."-----END CERTIFICATE-----\n";
-            }
+        $x509cert = str_replace(["\x0d", "\r", "\n", " "], '', $x509cert);
 
+        if ($heads && $x509cert !== '') {
+            $x509cert = "-----BEGIN CERTIFICATE-----\n".chunk_split($x509cert, 64, "\n")."-----END CERTIFICATE-----\n";
         }
+
         return $x509cert;
     }
 

diff --git a/tests/certs/with.comment.crt b/tests/certs/with.comment.crt
@@ -0,0 +1,17 @@
+# certificate comments should be ignored
+-----BEGIN CERTIFICATE-----
+MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhMC
+Tk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDgYD
+VQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqhkiG
+9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0wNzA4
+MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyBTb2xi
+ZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBAMTD2Zl
+aWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZXR0Lm5v
+MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqKxupQe0LO
+NoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLCn7c8aPHIS
+KOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo078dVhXQ14d
+1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVfp86HObqY+e8
+BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFDbStNomPnQz7n
+bK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJVhTyvLgFVK2Qar
+Q4/67OZfHd7R+POBXhophSMv1ZOo
+-----END CERTIFICATE-----
diff --git a/tests/src/OneLogin/Saml2/AuthTest.php b/tests/src/OneLogin/Saml2/AuthTest.php
@@ -827,25 +827,20 @@ public function testProcessSLORequestRelayState()
         $plainMessage = str_replace('http://stuff.com/endpoints/endpoints/sls.php', $currentURL, $plainMessage);
         $message = base64_encode(gzdeflate($plainMessage));
 
+        unset($_GET['SAMLResponse']);
         $_GET['SAMLRequest'] = $message;
         $_GET['RelayState'] = 'http://relaystate.com';
 
-        try {
-            $this->_auth->setStrict(true);
-            $this->_auth->processSLO(false);
-            $this->assertFalse(true);
-        } catch (Exception $e) {
-            $this->assertContains('Cannot modify header information', $e->getMessage());
-            $trace = $e->getTrace();
-            $targetUrl = getUrlFromRedirect($trace);
-            $parsedQuery = getParamsFromUrl($targetUrl);
+        $this->_auth->setStrict(true);
+        $targetUrl = $this->_auth->processSLO(false, null, false, null, true);
 
-            $sloResponseUrl = $this->_settingsInfo['idp']['singleLogoutService']['responseUrl'];
-            $this->assertContains($sloResponseUrl, $targetUrl);
-            $this->assertArrayHasKey('SAMLResponse', $parsedQuery);
-            $this->assertArrayHasKey('RelayState', $parsedQuery);
-            $this->assertEquals('http://relaystate.com', $parsedQuery['RelayState']);
-        }
+        $parsedQuery = getParamsFromUrl($targetUrl);
+
+        $sloResponseUrl = $this->_settingsInfo['idp']['singleLogoutService']['responseUrl'];
+        $this->assertContains($sloResponseUrl, $targetUrl);
+        $this->assertArrayHasKey('SAMLResponse', $parsedQuery);
+        $this->assertArrayHasKey('RelayState', $parsedQuery);
+        $this->assertEquals('http://relaystate.com', $parsedQuery['RelayState']);
     }
 
     /**
@@ -878,25 +873,19 @@ public function testProcessSLORequestSignedResponse()
         $_GET['SAMLRequest'] = $message;
         $_GET['RelayState'] = 'http://relaystate.com';
 
-        try {
-            $auth->setStrict(true);
-            $auth->processSLO(false);
-            $this->assertFalse(true);
-        } catch (Exception $e) {
-            $this->assertContains('Cannot modify header information', $e->getMessage());
-            $trace = $e->getTrace();
-            $targetUrl = getUrlFromRedirect($trace);
-            $parsedQuery = getParamsFromUrl($targetUrl);
+        $auth->setStrict(true);
+        $targetUrl = $auth->processSLO(false, null, false, null, true);
 
-            $sloResponseUrl = $this->_settingsInfo['idp']['singleLogoutService']['responseUrl'];
-            $this->assertContains($sloResponseUrl, $targetUrl);
-            $this->assertArrayHasKey('SAMLResponse', $parsedQuery);
-            $this->assertArrayHasKey('RelayState', $parsedQuery);
-            $this->assertArrayHasKey('SigAlg', $parsedQuery);
-            $this->assertArrayHasKey('Signature', $parsedQuery);
-            $this->assertEquals('http://relaystate.com', $parsedQuery['RelayState']);
-            $this->assertEquals(XMLSecurityKey::RSA_SHA1, $parsedQuery['SigAlg']);
-        }
+        $parsedQuery = getParamsFromUrl($targetUrl);
+
+        $sloResponseUrl = $this->_settingsInfo['idp']['singleLogoutService']['responseUrl'];
+        $this->assertContains($sloResponseUrl, $targetUrl);
+        $this->assertArrayHasKey('SAMLResponse', $parsedQuery);
+        $this->assertArrayHasKey('RelayState', $parsedQuery);
+        $this->assertArrayHasKey('SigAlg', $parsedQuery);
+        $this->assertArrayHasKey('Signature', $parsedQuery);
+        $this->assertEquals('http://relaystate.com', $parsedQuery['RelayState']);
+        $this->assertEquals(XMLSecurityKey::RSA_SHA256, $parsedQuery['SigAlg']);
     }
 
     /**

diff --git a/tests/src/OneLogin/Saml2/UtilsTest.php b/tests/src/OneLogin/Saml2/UtilsTest.php
@@ -34,8 +34,8 @@ public function testLoadXML()
         try {
             $res1 = Utils::loadXML($dom, $metadataUnloaded);
             $this->assertFalse($res1);
-        } catch (Exception $e) {
-          $this->assertEquals('DOMDocument::loadXML(): Premature end of data in tag EntityDescriptor line 1 in Entity, line: 1', $e->getMessage());
+        } catch (\Exception $e) {
+            $this->assertEquals('DOMDocument::loadXML(): Premature end of data in tag EntityDescriptor line 1 in Entity, line: 1', $e->getMessage());
         }
 
         $metadataInvalid = file_get_contents(TEST_ROOT .'/data/metadata/noentity_metadata_settings1.xml');
@@ -194,6 +194,11 @@ public function testFormatCert()
         $this->assertNotContains('-----END CERTIFICATE-----', $formatedCert6);
         $this->assertEquals(strlen($cert2), 860);
 
+        $cert = file_get_contents(TEST_ROOT.'/certs/with.comment.crt');
+        $formatedCert7 = Utils::formatCert($cert, true);
+        $this->assertContains('-----BEGIN CERTIFICATE-----', $formatedCert7);
+        $this->assertContains('-----END CERTIFICATE-----', $formatedCert7);
+        $this->assertNotContains('comments', $formatedCert7);
     }
 
     /**