Laragear / WebAuthn

Authenticate users with Passkeys: fingerprints, patterns and biometric data.
MIT License
295 stars 37 forks source link

[1.2.1] Custom Relying Party Id does not pass CheckRelyingPartyIdContained pipe #50

Closed Bubka closed 6 months ago

Bubka commented 1 year ago

PHP & Platform

8.1.22

Database

No response

Laravel version

10.16.1

Have you done this?

Expectation

Setting a custom Relying Party Id in the .env file should allow to register new webauthn devices.

Description

Following the Laragear/webauthn documentation: https://github.com/Laragear/WebAuthn/blob/7e62ec927e860ba73f0d2778f193ff34baa128eb/README.md?plain=1#L605-L609

If I set WEBAUTHN_ID=https://my.domain.com the registration ceremony fails because the RP-ID is not valid. Indeed, regarding the Webauthn W3C recommandation, the RP ID should be a domain, not an URL. Using such an URL makes the webauthn API throwing a SecurityError (see https://www.w3.org/TR/webauthn-2/#CreateCred-DetermineRpId)

But if I set WEBAUTHN_ID=my.domain.com, the registration ceremony also fails but this time because the CheckRelyingPartyIdContained does not pass because of the way $current is defined:

https://github.com/Laragear/WebAuthn/blob/7e62ec927e860ba73f0d2778f193ff34baa128eb/src/SharedPipes/CheckRelyingPartyIdContained.php#L46-L48

Using parse_url() with the PHP_URL_HOST flag will return nothing, causing the next evaluation to fail:

https://github.com/Laragear/WebAuthn/blob/7e62ec927e860ba73f0d2778f193ff34baa128eb/src/SharedPipes/CheckRelyingPartyIdContained.php#L51-L53

Reproduction

// Set WEBAUTHN_ID=my.domain.com and try to register a new device

Stack trace & logs

No response

DarkGhostHunter commented 1 year ago

Okay. Basically, I need to fix this by extracting the domain from the APP_URL, and if it's empty, use WEBAUTHN_ID verbatim if it doesn't start with https://.

asivaneswaran commented 1 year ago

I am going to add also other steps.

So I was able to get through this (for now) by doing this:

$current = parse_url(
            'http://' . $this->config->get('webauthn.relying_party.id') ?? $this->config->get('app.url'), PHP_URL_HOST
        );

I would suggest maybe adding a config for this or something. But I run into another issue here:

 public function hasNotSameRPIdHash(string $relyingPartyId, bool $hash = true): bool
    {
        return ! $this->hasSameRPIdHash($relyingPartyId, $hash);
    }

File: Attestation/AuthenticatorData Line 117 Because the relyingPartId is null and it comes from Attestation/Validator/Pipes/CheckRelyingPartyHashSame.php Line 39:

    protected function relyingPartyId(AssertionValidation|AttestationValidation $validation): string
    {
        return 'http://' . $this->config->get('webauthn.relying_party.id') ?? $this->config->get('app.url');
    }

This registers properly. I know this isn't the cleanest, but I had to work on my development while waiting on a fix.

EDIT: Also need to change this in Assertion/Validator/Pipes/CheckRelyingPartyHashSame.php:

  /**
     * Return the Relying Party ID from the config or credential.
     *
     * @param  \Laragear\WebAuthn\Assertion\Validator\AssertionValidation|\Laragear\WebAuthn\Attestation\Validator\AttestationValidation  $validation
     * @return string
     */
    protected function relyingPartyId(AssertionValidation|AttestationValidation $validation): string
    {
        return 'http://' . $validation->credential->rp_id;
    }
asivaneswaran commented 1 year ago

@DarkGhostHunter Will you be able to create the fix for this or would you like me to create a PR for this as well? Thanks!

DarkGhostHunter commented 1 year ago

Please, help me with the PR. Currently hands full.