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

Cannot register U2F key #10231

Closed
2 of 7 tasks
thmo opened this issue Feb 11, 2020 · 16 comments · Fixed by #12990
Closed
2 of 7 tasks

Cannot register U2F key #10231

thmo opened this issue Feb 11, 2020 · 16 comments · Fixed by #12990
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR type/bug
Milestone

Comments

@thmo
Copy link

thmo commented Feb 11, 2020

  • Gitea version (or commit ref): 1.11.0
  • Git version: 2.20.1
  • Operating system: Debian GNU/Linux 9 (stretch)
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant

Description

Trying to register a Yubikey Touch U2F Security Key (1050:0120) to my account with Firefox 72.0.2. There's a Firefox popup telling me that my Gitea instance "wants to register an account with one of your security keys", and the blue light goes on. Gitea also shows the "Add Security Key" dialog.

However, when touching the key, only the Firefox popup vanishes, but the Gitea dialog stays open (until a timeout occurs later).

The log shows:

2020/02/11 09:23:12 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT `id`, `lower_name`, `name`, `full_name`, `email`, `keep_email_private`, `email_notifications_preference`, `passwd`, `passwd_hash_algo`, `must_change_password`, `login_type`, `login_source`, `login_name`, `type`, `location`, `website`, `rands`, `salt`, `language`, `description`, `created_unix`, `updated_unix`, `last_login_unix`, `last_repo_visibility`, `max_repo_creation`, `is_active`, `is_admin`, `allow_git_hook`, `allow_import_local`, `allow_create_organization`, `prohibit_login`, `avatar`, `avatar_email`, `use_custom_avatar`, `num_followers`, `num_following`, `num_stars`, `num_repos`, `num_teams`, `num_members`, `visibility`, `repo_admin_change_team_access`, `diff_view_style`, `theme` FROM `user` WHERE `id`=? LIMIT 1 []interface {}{1} - took: 837.2µs
2020/02/11 09:23:12 ...s/context/context.go:330:func1() [D] Session ID: xxxxxxxxxxxxxxxx
2020/02/11 09:23:12 ...s/context/context.go:331:func1() [D] CSRF Token: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
2020/02/11 09:23:12 .../xorm/session_raw.go:78:queryRows() [I] [SQL] SELECT count(*) FROM `notification` WHERE (user_id = ?) AND (status = ?) []interface {}{1, 0x1} - took: 247.142µs
2020/02/11 09:23:12 ...ting/security_u2f.go:70:U2FRegisterPost() [E] u2f.Register: u2f: untrusted facet id
2020/02/11 09:23:12 ...s/context/context.go:139:HTML() [D] Template: status/500
@sapk
Copy link
Member

sapk commented Feb 11, 2020

Does gitea is accessible via https:// page ? and have you security.webauth.u2f set to true in about:config page ?

@thmo
Copy link
Author

thmo commented Feb 11, 2020

Does gitea is accessible via https:// page ?

Yes.

and have you security.webauth.u2f set to true in about:config page ?

Yes. I am using the very same key on a bunch of other sites, for example right here on GH.

@sapk
Copy link
Member

sapk commented Feb 11, 2020

Might be similar to #5143
There seems to have an issue with some user (we couldn't figure the exact conditions) with the legacy U2F api.
There is works to implement the new WebAuthN api #9451 but it is not yet ready.
Have you try to setup on try.gitea.io to check that it doesn't work ? (it might help discover what is the root cause)

@thmo
Copy link
Author

thmo commented Feb 11, 2020

Hmm, with try.gitea.io it seems to work.

@thmo
Copy link
Author

thmo commented Feb 11, 2020

FWIW, with Chrome it also fails directly with "Could not read your security key." (against our custom Gitea instance).

@sapk
Copy link
Member

sapk commented Feb 11, 2020

Can you check the value of ROOT_URL in your config file ?

@sapk
Copy link
Member

sapk commented Feb 11, 2020

Have a look at #10113 (comment) to check it.

@thmo
Copy link
Author

thmo commented Feb 11, 2020

The ROOT_URL is correct afaict (https://my.domain.tld/gitea/). The corresponding nginx config snippet looks like this:

location /gitea/ {
  proxy_pass http://localhost:3000/;
}

Looking at the Firefox Console, I can see successful (i.e. status 200) POSTs to request_register followed by failing (i.e. status 500) POSTs to register.

@thmo
Copy link
Author

thmo commented Feb 11, 2020

Seems it has something to do with the fact I am running Gitea on a sub-path.

Configuring

[U2F]
APP_ID = https://my.domain.tld
TRUSTED_FACETS = https://my.domain.tld

(both needed) fixes the issue for me.

I think the current defaults are not correct.

@sapk
Copy link
Member

sapk commented Feb 12, 2020

We should be able to define a subpath in app_id (https://fidoalliance.org/specs/fido-u2f-v1.0-nfc-bt-amendment-20150514/fido-appid-and-facets.html).

...
If the caller's FacetID is an https:// Origin sharing the same host as the AppID, (e.g. if an application hosted at https://fido.example.com/myApp set an AppID of https://fido.example.com/myAppId), no additional processing is necessary and the operation may proceed. This algorithm may be continued asynchronously for purposes of caching the Trusted Facet List, if desired.
...

But that also mean that we could set only the protocol:hostname:port and the same security rules are applied and that could allow device (or browser ?) with some custom implementation to validate.

The default values can be updated here:

U2F.TrustedFacets, _ = shellquote.Split(sec.Key("TRUSTED_FACETS").MustString(strings.TrimRight(AppURL, "/")))

And use .MustString(defaultAppURL) in place.

@thmo
Copy link
Author

thmo commented Feb 12, 2020

We should be able to define a subpath in app_id (https://fidoalliance.org/specs/fido-u2f-v1.0-nfc-bt-amendment-20150514/fido-appid-and-facets.html).

...
If the caller's FacetID is an https:// Origin sharing the same host as the AppID, (e.g. if an application hosted at https://fido.example.com/myApp set an AppID of https://fido.example.com/myAppId), no additional processing is necessary and the operation may proceed. This algorithm may be continued asynchronously for purposes of caching the Trusted Facet List, if desired.
...

Also saw this, so maybe it's a bug in the u2f library then? It compares against origin:
https://github.com/tstranex/u2f/blob/d21a03e0b1d9fc1df59ff54e7a513655c1748b0c/util.go#L107-L116

@lpapier
Copy link

lpapier commented Feb 13, 2020

For the record. I'm also hosting Gitea on a sub-path.

My Solokey works fine after adding the same kind of config as @thmo

@techknowlogick techknowlogick modified the milestones: 1.11.1, 1.11.2 Feb 17, 2020
@th0mcat
Copy link

th0mcat commented Feb 27, 2020

Can confirm @thmo 's fix also worked for me.

Gitea v1.12.0+dev-357-gf4370639b
Firefox v73.0.1

@sapk sapk added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Feb 27, 2020
@lunny
Copy link
Member

lunny commented Mar 3, 2020

@thmo could you send a PR to upstream?

@lunny lunny added status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR and removed backport/v1.11 labels Mar 3, 2020
@lunny lunny modified the milestones: 1.11.2, 1.11.3 Mar 3, 2020
@lunny lunny modified the milestones: 1.11.3, 1.11.4 Mar 10, 2020
@lafriks lafriks modified the milestones: 1.11.4, 1.11.5 Apr 1, 2020
@lunny lunny modified the milestones: 1.11.5, 1.11.6 May 1, 2020
@techknowlogick techknowlogick modified the milestones: 1.11.6, 1.11.7 Jun 3, 2020
@lafriks lafriks modified the milestones: 1.11.7, 1.12.1, 1.12.2 Jun 18, 2020
@lafriks lafriks modified the milestones: 1.12.2, 1.12.3 Jul 11, 2020
@techknowlogick techknowlogick modified the milestones: 1.12.3, 1.12.4 Jul 28, 2020
@lunny lunny modified the milestones: 1.12.4, 1.12.5 Sep 4, 2020
@zeripath
Copy link
Contributor

#10231 (comment)

Seems to say that the answer would be to default the [u2f] TRUSTED_FACETS to the host of the app URL.

In which case this is would be an extremely easy pr for Gitea.

I guess we could add some code to not set it if the appsuburl is empty but I suspect we don't need that.

@zeripath
Copy link
Contributor

zeripath commented Sep 30, 2020

U2F.TrustedFacets, _ = shellquote.Split(sec.Key("TRUSTED_FACETS").MustString(strings.TrimRight(AppURL, "/")))

This likely just needs to become:

shellquote.Split(sec.Key("TRUSTED_FACETS").MustString(strings.TrimSuffix(AppURL, AppSubURL + "/"))) 

Edit: That should be Suffix not Right(!)

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants