simplesamlphp / simplesamlphp-module-webauthn

A module implementing FIDO2 / WebAuthn as a second authentication factor
GNU Lesser General Public License v2.1
15 stars 8 forks source link

docs: note about GMP/BCMath for PHP 7 #50

Closed melanger closed 1 year ago

melanger commented 1 year ago

gmp (preferred) or bcmath extension is in fact required for signature validation

restena-sw commented 1 year ago

I don't think the code makes any use of those libs right now? The signature validation is done with openssl_verify() which has no external dependencies.

What good would bcmath or gmp do here?

melanger commented 1 year ago

I don't think the code makes any use of those libs right now? The signature validation is done with openssl_verify() which has no external dependencies.

What good would bcmath or gmp do here?

It is used by the cose-lib library inside validateSignature. This is the error when I do not have neither gmp nor bcmath installed:

May 30 14:50:45 secondary simplesamlphp[1627084]: 6 [b9640b45cc] FIDO2 - Accessing WebAuthn enrollment validation
May 30 14:50:45 secondary simplesamlphp[1627084]: 7 [b9640b45cc] Loading state: '_5c2433d0ed600768be526f8dc54953ab18ee2717af:https://passkey.grnet.gr/simplesaml/module.php/admin/test/webauthn-ssp-module'
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] SimpleSAML\Error\Error: UNHANDLEDEXCEPTION
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] Backtrace:
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 2 /opt/simplesamlphp-2.0.4/public/_include.php:28 (SimpleSAML_exception_handler)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 1 /opt/simplesamlphp-2.0.4/vendor/symfony/error-handler/ErrorHandler.php:607 (Symfony\Component\ErrorHandler\ErrorHandler::handleException)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 0 [builtin] (N/A)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] Caused by: RuntimeException: Requires GMP or bcmath extension.
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] Backtrace:
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 16 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/Utility/BigInteger.php:62 (FG\Utility\BigInteger::create)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 15 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/ASN1/Base128.php:20 (FG\ASN1\Base128::encode)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 14 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/ASN1/Universal/ObjectIdentifier.php:76 (FG\ASN1\Universal\ObjectIdentifier::getEncodedValue)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 13 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/ASN1/ASNObject.php:112 (FG\ASN1\ASNObject::getBinary)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 12 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/ASN1/Construct.php:111 (FG\ASN1\Construct::getEncodedValue)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 11 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/ASN1/ASNObject.php:112 (FG\ASN1\ASNObject::getBinary)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 10 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/ASN1/Construct.php:111 (FG\ASN1\Construct::getEncodedValue)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 9 /opt/simplesamlphp-2.0.4/vendor/fgrosse/phpasn1/lib/ASN1/ASNObject.php:112 (FG\ASN1\ASNObject::getBinary)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 8 /opt/simplesamlphp-2.0.4/vendor/web-auth/cose-lib/src/Key/Ec2Key.php:126 (Cose\Key\Ec2Key::asPEM)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 7 /opt/simplesamlphp-2.0.4/modules/webauthn/src/WebAuthn/WebAuthnAuthenticationEvent.php:76 (SimpleSAML\Module\webauthn\WebAuthn\WebAuthnAuthenticationEvent::validateSignature)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 6 /opt/simplesamlphp-2.0.4/modules/webauthn/src/WebAuthn/WebAuthnAuthenticationEvent.php:54 (SimpleSAML\Module\webauthn\WebAuthn\WebAuthnAuthenticationEvent::__construct)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 5 /opt/simplesamlphp-2.0.4/modules/webauthn/src/Controller/AuthProcess.php:165 (SimpleSAML\Module\webauthn\Controller\AuthProcess::main)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 4 /opt/simplesamlphp-2.0.4/vendor/symfony/http-kernel/HttpKernel.php:163 (Symfony\Component\HttpKernel\HttpKernel::handleRaw)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 3 /opt/simplesamlphp-2.0.4/vendor/symfony/http-kernel/HttpKernel.php:75 (Symfony\Component\HttpKernel\HttpKernel::handle)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 2 /opt/simplesamlphp-2.0.4/vendor/symfony/http-kernel/Kernel.php:202 (Symfony\Component\HttpKernel\Kernel::handle)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 1 /opt/simplesamlphp-2.0.4/src/SimpleSAML/Module.php:234 (SimpleSAML\Module::process)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] 0 /opt/simplesamlphp-2.0.4/public/module.php:14 (N/A)
May 30 14:50:45 secondary simplesamlphp[1627084]: 3 [b9640b45cc] Error report with id 8f0a93b7 generated.

When I installed gmp, this error disappeared.

restena-sw commented 1 year ago

Isn't this more a bug in cose-lib then, because they don't register the dependency on their side? It feels wrong to cover-up here when the issue is in a dependency.

melanger commented 1 year ago

Isn't this more a bug in cose-lib then, because they don't register the dependency on their side? It feels wrong to cover-up here when the issue is in a dependency.

In the current version (v4), they do not use phpasn1 anymore, so they only have gmp listed in a suggest block "for better performance":

https://github.com/web-auth/cose-lib/blob/4.3.x/composer.json#L58

The problem here is that I am using PHP 7.4 (which should be compatible with this module), which causes installation of cose-lib v3, where fgrosse/phpasn1 is used, which is an unmaintained library with this suggest block:

https://github.com/fgrosse/PHPASN1/blob/master/composer.json#L28-L30

    "suggest": {
        "ext-gmp": "GMP is the preferred extension for big integer calculations",
        "ext-bcmath": "BCmath is the fallback extension for big integer calculations",

What they really meant is that at least one of them is required, but that is probably impossible in composer.

I am not sure about the best solution, because phpasn1 is archived and cose-lib does not have this problem in the current version. But you cannot restrict cose-lib to v4, because that would drop support for PHP <8.

restena-sw commented 1 year ago

7.4 is dead anyway, no security updates any more. We try to maintain compat with 7.4 only because SSP itself does.

So, I don't really have an appetite in polluting the dependencies in simplesaml-module-webauthn for something that happens in a dependency.

You could PR a documentation update instead, reminding people to install either of those two extensions?

Clearly, when SSP drops support for PHP 7.4, happy to raise the depencies to the newest cose-lib and be done with it.

melanger commented 1 year ago

7.4 is dead anyway, no security updates any more. We try to maintain compat with 7.4 only because SSP itself does.

So, I don't really have an appetite in polluting the dependencies in simplesaml-module-webauthn for something that happens in a dependency.

You could PR a documentation update instead, reminding people to install either of those two extensions?

Clearly, when SSP drops support for PHP 7.4, happy to raise the depencies to the newest cose-lib and be done with it.

OK, I have updated this PR.

tvdijen commented 1 year ago

7.4 is dead anyway, no security updates any more. We try to maintain compat with 7.4 only because SSP itself does.

afbeelding

Just because PHP doesn't maintain it anymore, doesn't mean it's not receiving updates anymore. Even non-security updates

restena-sw commented 1 year ago

Let's settle on half-dead then.:-) It's fine to keep supporting it as long as SSP itself decides so. Just not longer than that.