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

[Bug]: Passwordless authentication does not ask for PIN when using security keys. #41599

Open
5 of 8 tasks
chryana opened this issue Nov 18, 2023 · 8 comments
Open
5 of 8 tasks
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 27-feedback bug feature: authentication

Comments

@chryana
Copy link

chryana commented Nov 18, 2023

⚠️ This issue respects the following points: ⚠️

Bug description

Hello,
I created a post on this topic on July last year but I never got a reply. The passwordless authentication seems to work in an unusual manner. It works well when I create a passkey with my browser (I find it unusual that it asks for a username, but there is already an issue open on this topic). However, if I add a YubiKey as a login device, it doesn't ask for a PIN; touching the security key button allows the login. Thus, anyone who would borrow my security key could access my account. Furthermore, I notice YubiKeys which do not support passwordless login can be added. I was able to add a YubiKey 4 WebAuthn device passwordless authentication device; no PIN is set on this key since it doesn't support FIDO2.

Steps to reproduce

  1. Add a security key (tested with a few YubiKeys of models 4 and 5) WebAuthn device to a Nextcloud account. Logout.
  2. Login with a device
  3. Type in the username
  4. There should be a window appearing to ask for the security PIN, but none appears, and the login proceeds. I think this is incorrect. (There is a dialog box requesting to touch the YubiKey though, as if the key was setup as a second factor.)

Expected behavior

The few passwordless implementations I know (Microsoft 365, Google) ask for a PIN; ownership of the security key is not sufficient to access the account.

Installation method

Community Manual installation with Archive

Nextcloud Server version

27

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.2

Web server

Apache (supported)

Database engine version

PostgreSQL

Is this bug present after an update or on a fresh install?

Updated from a MINOR version (ex. 22.1 to 22.2)

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "www.chryana.org"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "pgsql",
        "default_phone_region": "CA",
        "version": "27.1.3.2",
        "overwrite.cli.url": "https:\/\/www.chryana.org\/nextcloud",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "filelocking.enabled": true,
        "memcache.local": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 0,
            "timeout": 0
        },
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpmode": "smtp",
        "mail_smtpsecure": "tls",
        "mail_sendmailmode": "smtp",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpauthtype": "PLAIN",
        "mail_smtpauth": 1,
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "587",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "loglevel": 2,
        "maintenance": false,
        "theme": ""
    }
}

List of activated Apps

Enabled:
  - activity: 2.19.0
  - bruteforcesettings: 2.7.0
  - calendar: 4.5.2
  - circles: 27.0.1
  - cloud_federation_api: 1.10.0
  - comments: 1.17.0
  - contacts: 5.4.2
  - contactsinteraction: 1.8.0
  - dashboard: 7.7.0
  - dav: 1.27.0
  - federatedfilesharing: 1.17.0
  - federation: 1.17.0
  - files: 1.22.0
  - files_pdfviewer: 2.8.0
  - files_reminders: 1.0.0
  - files_rightclick: 1.6.0
  - files_sharing: 1.19.0
  - files_trashbin: 1.17.0
  - files_versions: 1.20.0
  - firstrunwizard: 2.16.0
  - logreader: 2.12.0
  - lookup_server_connector: 1.15.0
  - nextcloud_announcements: 1.16.0
  - notes: 4.8.1
  - notifications: 2.15.0
  - oauth2: 1.15.1
  - password_policy: 1.17.0
  - photos: 2.3.0
  - privacy: 1.11.0
  - provisioning_api: 1.17.0
  - recommendations: 1.6.0
  - related_resources: 1.2.0
  - serverinfo: 1.17.0
  - settings: 1.9.0
  - sharebymail: 1.17.0
  - support: 1.10.0
  - survey_client: 1.15.0
  - systemtags: 1.17.0
  - text: 3.8.0
  - theming: 2.2.0
  - twofactor_backupcodes: 1.16.0
  - twofactor_totp: 9.0.0
  - twofactor_webauthn: 1.2.0
  - updatenotification: 1.17.0
  - user_status: 1.7.0
  - viewer: 2.1.0
  - weather_status: 1.7.0
  - workflowengine: 2.9.0
Disabled:
  - admin_audit: 1.17.0
  - encryption: 2.15.0
  - files_external: 1.19.0
  - suspicious_login: 5.0.0
  - user_ldap: 1.17.0

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

Let me know if you have any specific questions regarding my setup.

@chryana chryana added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Nov 18, 2023
@ChristophWurst
Copy link
Member

Duplicate

@tushev
Copy link

tushev commented Nov 21, 2023

Well, I would not say that this is a duplicate of my issue, despite being very close.

Here, now, Nextcloud seems to create a non-resident credential, and without PIN protection. This is a valid security threat: if someone finds a lost Yubikey with an email (stored in a field in PIV or GPG apps, or whatever), and tries to login with that email (if it matches NC user), all they would have to do is to touch the key.

PIN protection should be required by default for non-resident FIDO2 credentials, with an option to turn it off for those who know what they are doing.

My issue #41191 is about going fully-WebAuthn, without having to type a username, with user identification only by resident FIDO2 credential (aka passkey).

In my vision, users should have 3 options when going passwordless:

  • non-resident credential, requires PIN (and thus requiring username/email) - default option
  • non-resident credential, without PIN (and thus requiring username/email), with a warning - this is current implementation
  • resident credential, requires PIN, no usernames

@ChristophWurst
Copy link
Member

See #21215 and #34389

@p1gp1g
Copy link
Contributor

p1gp1g commented Apr 15, 2024

While using a fido2 token without user verification (UV) as a 2nd factor is fine, using it without UV as a login method can lead to unauthorized authentication if someone get a physical access to the token. Until a fix is merged, users should avoid webauthn if they doesn't have a hardware token with UV required.

As a workaround, if you self host your own Nextcloud instance you can add this hook:

20_FIDO2_UV.sh (must be executable)

#!/bin/sh
sed -i 's/USER_VERIFICATION_REQUIREMENT_DISCOURAGED/USER_VERIFICATION_REQUIREMENT_REQUIRED/g' /var/www/html/lib/private/Authentication/WebAuthn/Manager.php

docker-compose.yaml

    [...]
    volumes:
        - ./20_FIDO_UV.sh:/docker-entrypoint-hooks.d/before-starting/20_FIDO2_UV.sh:z

@tushev
Copy link

tushev commented Apr 15, 2024

@p1gp1g does #44442 have an option to keep the old way (without user verification)?

While the current implementation is a valid security threat to most users (thank you for fixing it!), it's OK for some threat models (i.e. for one of my Nexclouds that's available from within the LAN only: I prioritize convenience of not doing UV here over a highly improbable event of a stranger using my webauthn token to access resources on my network - at least until #41191 is resolved).

@p1gp1g
Copy link
Contributor

p1gp1g commented Apr 16, 2024

@tushev , as mentioned in the PR:

  • Recommend UV (user verification with a PIN) during device registration
  • Require UV during authentication if all tokens are registered with UV
  • Discourage UV during authentication if any token is registered without UV (current behavior)
  • Previously registered token are considered without UV to avoid locking out a user

Regarding the convenience VS security : having a single character password is convenient, but it should not be allowed for regular users. Advanced users know they have to use a strong password and can in some very specific context (like an instance accepting requests from localhost only) use a weak one. So they don't need a technical solution to avoid weak passwords. But if there isn't a technical solution enforcing strong passwords, regular users will end with weak ones. That is why, by default you should technically enforce the use of strong password.
For the FIDO tokens it is the same. The challenges should be USER_VERIFICATION_REQUIREMENT_REQUIRED until there is a server setting to allow use of USER_VERIFICATION_REQUIREMENT_DISCOURAGED (with a UI check during registration and a warning) for advanced users

@tushev
Copy link

tushev commented Apr 16, 2024

@p1gp1g thank you!

in some very specific context (like an instance accepting requests from localhost only) use a weak one.
by default you should technically enforce the use of strong password.

Agreed. Ideally, such things should be hidden in a configuration file: this way most inexperienced users are barred out from disabling passwords strength enforcement. I guess the same may be implemented for UV at some point in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 27-feedback bug feature: authentication
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants