Switch openssl for phpseclib

[joonlabs]

Hey @robrichards,

thank you very much for providing this important library. It is used in many different other libraries - for example by the SAML2 library SAML-Toolkits/php-saml. Requirements on SAML have changed a lot in the past couple of years and many PHP developers find themselves in the situation, where the IDPs require algoithms like http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1, that are not supported by the openssl biding in PHP in the current version, which is based on PKCS #1 v1.5:

Currently in PHP it is only possible to make and verify OpenSSL based signatures based on cryptography defined in PKCS #1 v1.5.

(see. https://bugs.php.net/bug.php?id=80495&edit=3)

Since there is no update of the binding planned, I wanted to quickly check and ask if you accept a rewrite of the openssl methods using the phpseclib/phpseclib library. I would offer to change the implementation and create a pull request. I am not asking to change the methods or the structure of the library but only to switch from the openssl implementation to the phpseclib one. This would enable the additional support for the missing RSA OAEP and MFG algorithms by keeping the other encryption algorithms at the same time.

We are currently switching out this library by an internal fork which does exactly that, because the Elster IDP of the german government we connect to (using the SAML library mentioned above) only supports the http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1 algorithm. It would be awesome if this library would keep up with modern algorithms and enable developers SAML bindings to modern IDPs.

Looking forward to your answer,
Julius

[he247]

@joonlabs would you be willing to share your fork? I'm currently struggling to update this library locally in order to be able to connect to BundID which will, from the end of June, also only support the http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1 algorithm.

[robrichards]

@joonlabs I also must have missed your original message. I have recently been working on switching it out and doing a 4.0 release for it. If you already have the changes completed please share the changes otherwise I am hoping to have a first pass alpha out early next month.

[he247]

@robrichards That sounds great! In any case I am eager to help. On my own I'm really having a hard time figuring out what I would need to change without breaking anything else.

[tvdijen]

@he247 It's probably a good thing for not wanting to break too much, but remember a new major version also offers the opportunity to break things that were 'broken' in the first place..

[he247]

@tvdijen You're right. In my case I was more afraid of breaking important security features due to the lack of background knowledge for this project.

[he247]

@robrichards do you have already a version I could test? I just wrote @joonlabs an Email, maybe we'll get an answer from him :)

[he247]

I didn't get a response yet, but it might be that this is the code @joonlabs referred to:
rsa-pss feature tree in the optiGov project

[he247]

When I try using it in my setup (adopted the Utils.php in simplesaml/saml2 repo to recognize the algorithm) I get this error message:

SimpleSAML\Error\Error: UNHANDLEDEXCEPTION
Backtrace:
2 public/_include.php:34 (SimpleSAML_exception_handler)
1 /vufind/vendor/symfony/error-handler/ErrorHandler.php:619 (Symfony\Component\ErrorHandler\ErrorHandler::handleException)
0 [builtin] (N/A)
Caused by: TypeError: strlen(): Argument #1 ($string) must be of type string, OpenSSLAsymmetricKey given
Backtrace:
18 /vufind/vendor/phpseclib/phpseclib/phpseclib/File/ASN1.php:1458 (strlen)
17 /vufind/vendor/phpseclib/phpseclib/phpseclib/File/ASN1.php:1458 (phpseclib3\File\ASN1::extractBER)
16 /vufind/vendor/phpseclib/phpseclib/phpseclib/File/X509.php:452 (phpseclib3\File\X509::loadX509)
15 /vufind/vendor/phpseclib/phpseclib/phpseclib/Crypt/PublicKeyLoader.php:55 (phpseclib3\Crypt\PublicKeyLoader::load)
14 /vufind/vendor/robrichards/xmlseclibs/src/XMLSecurityKey.php:647 (RobRichards\XMLSecLibs\XMLSecurityKey::decryptData)
13 /vufind/vendor/robrichards/xmlseclibs/src/XMLSecEnc.php:263 (RobRichards\XMLSecLibs\XMLSecEnc::decryptNode)
12 /vufind/vendor/robrichards/xmlseclibs/src/XMLSecEnc.php:351 (RobRichards\XMLSecLibs\XMLSecEnc::decryptKey)
11 /vufind/vendor/simplesamlphp/saml2/src/SAML2/Utils.php:436 (SAML2\Utils::doDecryptElement)
10 /vufind/vendor/simplesamlphp/saml2/src/SAML2/Utils.php:532 (SAML2\Utils::decryptElement)
9 /vufind/vendor/simplesamlphp/saml2/src/SAML2/EncryptedAssertion.php:122 (SAML2\EncryptedAssertion::getAssertion)
8 modules/saml/src/Message.php:382 (SimpleSAML\Module\saml\Message::decryptAssertion)
7 modules/saml/src/Message.php:647 (SimpleSAML\Module\saml\Message::processAssertion)
6 modules/saml/src/Message.php:615 (SimpleSAML\Module\saml\Message::processResponse)
5 modules/saml/src/Controller/ServiceProvider.php:317 (SimpleSAML\Module\saml\Controller\ServiceProvider::assertionConsumerService)
4 /vufind/vendor/symfony/http-kernel/HttpKernel.php:163 (Symfony\Component\HttpKernel\HttpKernel::handleRaw)
3 /vufind/vendor/symfony/http-kernel/HttpKernel.php:75 (Symfony\Component\HttpKernel\HttpKernel::handle)
2 /vufind/vendor/symfony/http-kernel/Kernel.php:202 (Symfony\Component\HttpKernel\Kernel::handle)
1 src/SimpleSAML/Module.php:234 (SimpleSAML\Module::process)
0 public/module.php:17 (N/A)

Maybe I need to adopt the XMLSecurityDSig as well and not only XMLSecurityKey, but I'm not sure if that is the cause of this error. Or my current simplesaml version (2.0.15) is not compatible. I'll dig deeper and will be thankful for any suggestions.

[joonlabs]

Hey @ALL, sorry for the late reply. I was very busy.
To summarize, yes. The fork https://github.com/optiGov/xmlseclibs/tree/feature/rsa-pss is correct. But please keep in mind, that this is not a complete switch, rather a workaround which adds the necessary algorithms and associated security backend (phpseclib) to enable support for the BundID and MUK IDPs.
@he247 We do not use simplesaml, so unfortunately, I can't really help you out here.

[joonlabs]

@robrichards Since I put a lot of effort in understanding all the SAML mechanics and flow, I would be happy to help you out. If you are interested in a bit support, let me know how I can help you best.
Imo a full switch to phpseclib would be the cleanest option. Then the algorithms I added in my fork can be added as well and the saml libraries can "natively" support the modern encryption algorithms.

[he247]

Dear all,
I was able to get it working by slightly adapting the XMLSecurityKey.php from optiGov:

diff --git a/tmp/git_remote_diff b/src/XMLSecurityKey.php
index 2891e0e..6d69d43 100644
--- a/tmp/git_remote_diff
+++ b/src/XMLSecurityKey.php
@@ -410,6 +410,19 @@ class XMLSecurityKey
                 default:
                     throw new Exception('Unknown type');
             }
+        } else if ($this->cryptParams['library'] === 'phpseclib') {
+            // Important: PublicKeyLoader::load() expects a PEM string.
+            if (!is_string($this->key)) {
+                // If $this->key is, for example, an OpenSSLAsymmetricKey, we try to export it.
+                $pem = '';
+                if (openssl_pkey_export($this->key, $pem)) {
+                    $this->key = $pem;
+                } else {
+                    throw new Exception('Unable to export key to PEM format');
+                }
+            }
+            // Now we load the key using phpseclib.
+            $this->key = PublicKeyLoader::load($this->key);
         }
     }

@@ -644,6 +657,17 @@ class XMLSecurityKey
                     return $this->decryptPrivate($data);
             }
         } else if($this->cryptParams['library'] === 'phpseclib') {
+            // Important: PublicKeyLoader::load() expects a PEM string.
+            if (!is_string($this->key)) {
+                // If $this->key is, for example, an OpenSSLAsymmetricKey, we try to export it.
+                $pem = '';
+                if (openssl_pkey_export($this->key, $pem)) {
+                    $this->key = $pem;
+                } else {
+                    throw new Exception('Unable to export key to PEM format');
+                }
+            }
+            // Now we load the key using phpseclib.
             $private = PublicKeyLoader::load($this->key);
             $result = $private
                 ->withPadding($this->cryptParams['padding'])

[he247]

I know that this is still just a hack and I'll be thankful for any suggestions, if I may have missed something or could break security features with this change.

[robrichards]

@he247 @joonlabs Just a heads up that work has pulled me away. Ive been trying to work on this in any spare time in the background but just wanted to let you know that its still in progress but I'll be out of pocket with little comms I'm guessing for at least the next 3-4 weeks. Will hopefully have some progress to show then. Keep sending or adding to branches and I'll see what I can do about combining all the work together.

[danielk117-work]

The fork https://github.com/optiGov/xmlseclibs/tree/feature/rsa-pss

happy too see, that i'm not the only one maintaining an own fork for this 😉 https://github.com/brain-SCC/xmlseclibs