Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webauthn RP ID matching too strict #210

Open
jafri opened this issue Jul 23, 2021 · 5 comments
Open

Webauthn RP ID matching too strict #210

jafri opened this issue Jul 23, 2021 · 5 comments

Comments

@jafri
Copy link

jafri commented Jul 23, 2021

As per W3C RP ID specs at https://w3c.github.io/webauthn/#relying-party-identifier

"Given a Relying Party whose origin is https://login.example.com:1337, then the following RP IDs are valid: login.example.com (default) and example.com, but not m.login.example.com and not com."

However, fc does a rpIdHash === sha(clientDataJSON.origin) match which is too strict and does not cover the case where the origin may be login.example.com while the rp id is example.com:

FC_ASSERT(memcmp(c.auth_data.data(), fc::sha256::hash(rpid).data(), sizeof(fc::sha256)) == 0, "webauthn rpid hash doesn't match origin");

@tbfleming
Copy link
Contributor

It would be nice to relax that. It will take a hard fork to do so.

@jafri
Copy link
Author

jafri commented Jul 23, 2021

A hard fork is fine, best to do it now before more usage of webauthn in EOSIO

@DenisCarriere
Copy link

When mixing subdomains, push transaction on Nodeos causes the following error:

{
  "code": 500,
  "message": "Internal Service Error",
  "error": {
    "code": 10,
    "name": "assert_exception",
    "what": "Assert Exception",
    "details": [
      {
        "message": "memcmp(c.auth_data.data(), fc::sha256::hash(rpid).data(), sizeof(fc::sha256)) == 0: webauthn rpid hash doesn't match origin",
        "file": "elliptic_webauthn.cpp",
        "line_number": 213,
        "method": "public_key"
      },
      {
        "message": "",
        "file": "transaction.cpp",
        "line_number": 78,
        "method": "get_signature_keys"
      }
    ]
  }
}

@smlu
Copy link

smlu commented Aug 9, 2021

char required_origin_scheme[] = "https://";
size_t https_len = strlen(required_origin_scheme);
FC_ASSERT(handler.found_origin.compare(0, https_len, required_origin_scheme) == 0, "webauthn origin must begin with https://");

Also relaxing restriction for localhost to require security context would be great. This would allow local apps to run in browser without the need for custom certificate and TLS connection.

@spoonincode
Copy link
Contributor

I believe this was deliberate. EOSIO is modeled such that a single key is recovered from a signature, and a key must exactly match a key that is in an authority. Recovering multiple keys from a native signature (such a recovering a bunch of PUB_WA keys with all the valid rpids), or allowing a recovered key to "fuzzy match" a key in an authority, were both considered too much at odds with existing EOSIO design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants