X509 cert comments (#570)

Skip comments in .crt file Co-authored-by: Fredrik Sundblom <[email protected]> Co-authored-by: Sixto Martin <[email protected]>
SAML-Toolkits · May 19, 2024 · 92a9e2d · 92a9e2d
1 parent d3884fb
commit 92a9e2d
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 16 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
@@ -780,7 +780,8 @@ public function testProcessSLORequestRelayState()
         $_GET['RelayState'] = 'http://relaystate.com';
 
         $this->_auth->setStrict(true);
-        $targetUrl = $this->_auth->processSLO(false, null, null, null, true);
+        $targetUrl = $this->_auth->processSLO(false, null, false, null, true);
+
         $parsedQuery = getParamsFromUrl($targetUrl);
 
         $sloResponseUrl = $this->_settingsInfo['idp']['singleLogoutService']['responseUrl'];
@@ -818,7 +819,7 @@ public function testProcessSLORequestSignedResponse()
         $_GET['RelayState'] = 'http://relaystate.com';
 
         $auth->setStrict(true);
-        $targetUrl = $auth->processSLO(false, null, null, null, true);
+        $targetUrl = $auth->processSLO(false, null, false, null, true);
 
         $parsedQuery = getParamsFromUrl($targetUrl);
 

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->assertStringNotContainsString('-----END CERTIFICATE-----', $formatedCert6);
         $this->assertEquals(strlen($cert2), 860);
 
+        $cert = file_get_contents(TEST_ROOT.'/certs/with.comment.crt');
+        $formatedCert7 = Utils::formatCert($cert, true);
+        $this->assertStringContainsString('-----BEGIN CERTIFICATE-----', $formatedCert7);
+        $this->assertStringContainsString('-----END CERTIFICATE-----', $formatedCert7);
+        $this->assertStringNotContainsString('comments', $formatedCert7);
     }
 
     /**