[PR #4637] [MERGED] Add support for MFA with Duo's Universal Prompt #7104

Closed
opened 2026-03-07 21:11:03 -06:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/dani-garcia/vaultwarden/pull/4637
Author: @0x0fbc
Created: 6/11/2024
Status: Merged
Merged: 7/24/2024
Merged by: @dani-garcia

Base: mainHead: feature/duo_universal


📝 Commits (10+)

  • 849ce1e Add initial working Duo Universal Prompt support.
  • 6f86749 Add db schema and models for Duo 2FA state storage
  • 36f90cd store duo states in the database and validate during authentication
  • 328c4a3 cleanup & comments
  • 41f94e2 bump state/nonce length
  • 90b73c6 replace stray use of TimeDelta
  • f489930 more cleanup
  • 6f06fa9 bind Duo oauth flow to device id, drop redundant device type handling
  • 4df7fcf drop redundant alphanum string generation code
  • e7a058c error handling cleanup

📊 Changes

18 files changed (+720 additions, -16 deletions)

View changed files

📝 .env.template (+13 -3)
migrations/mysql/2024-06-05-131359_add_2fa_duo_store/down.sql (+1 -0)
migrations/mysql/2024-06-05-131359_add_2fa_duo_store/up.sql (+8 -0)
migrations/postgresql/2024-06-05-131359_add_2fa_duo_store/down.sql (+1 -0)
migrations/postgresql/2024-06-05-131359_add_2fa_duo_store/up.sql (+8 -0)
migrations/sqlite/2024-06-05-131359_add_2fa_duo_store/down.sql (+1 -0)
migrations/sqlite/2024-06-05-131359_add_2fa_duo_store/up.sql (+8 -0)
📝 src/api/core/two_factor/duo.rs (+1 -1)
src/api/core/two_factor/duo_oidc.rs (+500 -0)
📝 src/api/core/two_factor/mod.rs (+1 -0)
📝 src/api/identity.rs (+52 -11)
📝 src/config.rs (+5 -1)
📝 src/db/models/mod.rs (+2 -0)
src/db/models/two_factor_duo_context.rs (+84 -0)
📝 src/db/schemas/mysql/schema.rs (+9 -0)
📝 src/db/schemas/postgresql/schema.rs (+9 -0)
📝 src/db/schemas/sqlite/schema.rs (+9 -0)
📝 src/main.rs (+8 -0)

📄 Description

Overview

This adds support for MFA using Duo's "Universal Prompt".

On March 30th, 2024, the Duo MFA integration that Vaultwarden uses, the 'Traditional Prompt' was end-of-lifed. While Duo's API continues to allow the traditional prompt to be used in some cases, it is entirely unsupported, and it will eventually stop working. Some Vaultwarden users have already begun reporting that their Duo API keys are no longer functional in Vaultwarden. Bitwarden introduced Universal Prompt support in release 2024.2.3 on March 5th.

Duo's Universal Prompt uses the OIDC Authorization Code flow. This flow requires us to pack information about the authentication we want to be prompted for MFA into an authorization request (a JWT signed with a secret key provided by Duo) and send the user to their service, where MFA is performed. The user is then returned to our service with a code that we use to call Duo's API and obtain the result of the MFA. Once we validate the information passed back by the user's client and returned by Duo's service, we can log the user in.

Detailed documentation is located at https://duo.com/docs/oauthapi

The web vault handles receiving the redirection from Duo. It has a 'connector' page responsible for communicating the authorization code back to the user's true client so it can be provided in a call to Vaultwarden's API.

Major additions/changes

  • Added a crate, duo_oidc, which implements the Duo Universal Prompt MFA flow.
  • Added a database table, twofactor_duo_ctx, migrations, and a model to represent it. This table stores information about in-progress Duo MFA attempts for validation while an authenticating user completes MFA on Duo's service.
  • Added an internal task to clear the twofactor_duo_ctx table of incomplete Duo MFA attempts.
  • Added a configuration option to globally force Vaultwarden to use the legacy Duo traditional prompt flow instead of the universal prompt.

Specific Review Items

Beyond what you'd normally check, I'd like to highlight a few decisions I made that you should probably consider in your review.

  • The OIDC 'nonce' parameter is optional and intended to harden against replay attacks. The best practice is to bind the nonce to a specific authentication attempt using something like an HttpOnly cookie. Cookies don't seem to be an option without significant changes to Vaultwarden, let alone the Bitwarden clients. Instead of not leveraging it, I tried to win back some of the hardening the parameter is intended to provide. I ended up with a strategy where a random nonce is generated, combined with the device_identifier reported by the client, and hashed to produce the OIDC nonce sent to Duo in the authorization request. Then, the device_identifier reported by the user's client when providing the authorization code is hashed with the saved nonce, and the resulting hash is compared to the OIDC nonce Duo reports when we call their API for the MFA result.
  • Duo's official client libraries for the Universal Prompt pin hard-coded CA certs from Digicert, SecureTrust, and Amazon. Duo's implementation documentation didn't make any recommendations around pinning certs, and it looks like some of them are root CA certs (the pinning of which is controversial), so I did not implement this.
  • Instead of copying or reimplementing the original duo crate's get_duo_keys_email function, which fetches the Duo keys from the database/config for a given user, I set its visibility to pub(crate) and re-used it.

Testing

Tested on the following clients; everything is working well:

  • Vaultwarden-patched web vault
  • Windows, macOS, and Linux (Flatpak) desktop clients
  • Android app (as obtained from Google play)
  • iOS app (as obtained from the Apple app store)
  • Chrome and Firefox browser extensions

The only major client issue I found was with the unpackaged AppImage Linux desktop client. When authenticating users on desktop clients, the connector in the web vault opens a bitwarden:// link, which my system refused to use the AppImage to open. It looks like the AppImage client either doesn't or can't register itself as the x-scheme-handler for bitwarden, preventing the redirect connector from handing off to the client. It worked fine with the Flatpak packaged Bitwarden client, so I'm chalking it up to my not configuring the AppImage client correctly.

There is also a known issue upstream in the web vault where the redirect connector shows 'successful authentication', does not automatically close, and the JS 'close' button does not work. See https://github.com/bitwarden/clients/issues/8554

I also tested this while running PostgreSQL and MariaDB using the versions packaged in Debian 12; no issues.

Fixes #4529


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/dani-garcia/vaultwarden/pull/4637 **Author:** [@0x0fbc](https://github.com/0x0fbc) **Created:** 6/11/2024 **Status:** ✅ Merged **Merged:** 7/24/2024 **Merged by:** [@dani-garcia](https://github.com/dani-garcia) **Base:** `main` ← **Head:** `feature/duo_universal` --- ### 📝 Commits (10+) - [`849ce1e`](https://github.com/dani-garcia/vaultwarden/commit/849ce1ebce7e262c1955ebe0d30d65ff6c248c7c) Add initial working Duo Universal Prompt support. - [`6f86749`](https://github.com/dani-garcia/vaultwarden/commit/6f86749757e9cf9539f0789c7cc70354645ff473) Add db schema and models for Duo 2FA state storage - [`36f90cd`](https://github.com/dani-garcia/vaultwarden/commit/36f90cd8f491d6aa30be3b4254fc9391926a6828) store duo states in the database and validate during authentication - [`328c4a3`](https://github.com/dani-garcia/vaultwarden/commit/328c4a3e3cb0b996a43c690411be305df50621b7) cleanup & comments - [`41f94e2`](https://github.com/dani-garcia/vaultwarden/commit/41f94e2f527f5bec2203bb00f10d938330e272ac) bump state/nonce length - [`90b73c6`](https://github.com/dani-garcia/vaultwarden/commit/90b73c6dbe01d380b5bb230e37fcfaea9ab5c4a3) replace stray use of TimeDelta - [`f489930`](https://github.com/dani-garcia/vaultwarden/commit/f489930482e7c4e49e2b9f392af3a032488ab8ce) more cleanup - [`6f06fa9`](https://github.com/dani-garcia/vaultwarden/commit/6f06fa9da30cfa6ce2176bf412878c1d134a5f47) bind Duo oauth flow to device id, drop redundant device type handling - [`4df7fcf`](https://github.com/dani-garcia/vaultwarden/commit/4df7fcf35c595668efc836d54dc83c36b0703d46) drop redundant alphanum string generation code - [`e7a058c`](https://github.com/dani-garcia/vaultwarden/commit/e7a058cfa8d18e2cf2b2ee611e96d7627c9f9549) error handling cleanup ### 📊 Changes **18 files changed** (+720 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `.env.template` (+13 -3) ➕ `migrations/mysql/2024-06-05-131359_add_2fa_duo_store/down.sql` (+1 -0) ➕ `migrations/mysql/2024-06-05-131359_add_2fa_duo_store/up.sql` (+8 -0) ➕ `migrations/postgresql/2024-06-05-131359_add_2fa_duo_store/down.sql` (+1 -0) ➕ `migrations/postgresql/2024-06-05-131359_add_2fa_duo_store/up.sql` (+8 -0) ➕ `migrations/sqlite/2024-06-05-131359_add_2fa_duo_store/down.sql` (+1 -0) ➕ `migrations/sqlite/2024-06-05-131359_add_2fa_duo_store/up.sql` (+8 -0) 📝 `src/api/core/two_factor/duo.rs` (+1 -1) ➕ `src/api/core/two_factor/duo_oidc.rs` (+500 -0) 📝 `src/api/core/two_factor/mod.rs` (+1 -0) 📝 `src/api/identity.rs` (+52 -11) 📝 `src/config.rs` (+5 -1) 📝 `src/db/models/mod.rs` (+2 -0) ➕ `src/db/models/two_factor_duo_context.rs` (+84 -0) 📝 `src/db/schemas/mysql/schema.rs` (+9 -0) 📝 `src/db/schemas/postgresql/schema.rs` (+9 -0) 📝 `src/db/schemas/sqlite/schema.rs` (+9 -0) 📝 `src/main.rs` (+8 -0) </details> ### 📄 Description ### Overview This adds support for MFA using Duo's "Universal Prompt". On March 30th, 2024, the Duo MFA integration that Vaultwarden uses, the 'Traditional Prompt' was end-of-lifed. While Duo's API continues to allow the traditional prompt to be used in some cases, it is entirely unsupported, and it will eventually stop working. Some Vaultwarden users have already begun reporting that their Duo API keys are no longer functional in Vaultwarden. Bitwarden introduced Universal Prompt support in release 2024.2.3 on March 5th. Duo's Universal Prompt uses the OIDC Authorization Code flow. This flow requires us to pack information about the authentication we want to be prompted for MFA into an authorization request (a JWT signed with a secret key provided by Duo) and send the user to their service, where MFA is performed. The user is then returned to our service with a code that we use to call Duo's API and obtain the result of the MFA. Once we validate the information passed back by the user's client and returned by Duo's service, we can log the user in. Detailed documentation is located at [https://duo.com/docs/oauthapi](https://duo.com/docs/oauthapi) The web vault handles receiving the redirection from Duo. It has a 'connector' page responsible for communicating the authorization code back to the user's true client so it can be provided in a call to Vaultwarden's API. ### Major additions/changes - Added a crate, duo_oidc, which implements the Duo Universal Prompt MFA flow. - Added a database table, twofactor_duo_ctx, migrations, and a model to represent it. This table stores information about in-progress Duo MFA attempts for validation while an authenticating user completes MFA on Duo's service. - Added an internal task to clear the twofactor_duo_ctx table of incomplete Duo MFA attempts. - Added a configuration option to globally force Vaultwarden to use the legacy Duo traditional prompt flow instead of the universal prompt. ### Specific Review Items Beyond what you'd normally check, I'd like to highlight a few decisions I made that you should probably consider in your review. - The OIDC 'nonce' parameter is optional and intended to harden against replay attacks. The best practice is to bind the nonce to a specific authentication attempt using something like an HttpOnly cookie. Cookies don't seem to be an option without significant changes to Vaultwarden, let alone the Bitwarden clients. Instead of not leveraging it, I tried to win back some of the hardening the parameter is intended to provide. I ended up with a strategy where a random nonce is generated, combined with the `device_identifier` reported by the client, and hashed to produce the OIDC nonce sent to Duo in the authorization request. Then, the `device_identifier` reported by the user's client when providing the authorization code is hashed with the saved nonce, and the resulting hash is compared to the OIDC nonce Duo reports when we call their API for the MFA result. - Duo's official client libraries for the Universal Prompt pin hard-coded CA certs from Digicert, SecureTrust, and Amazon. Duo's implementation documentation didn't make any recommendations around pinning certs, and it looks like some of them are root CA certs (the pinning of which is controversial), so I did not implement this. - Instead of copying or reimplementing the original duo crate's `get_duo_keys_email` function, which fetches the Duo keys from the database/config for a given user, I set its visibility to `pub(crate)` and re-used it. ### Testing Tested on the following clients; everything is working well: - Vaultwarden-patched web vault - Windows, macOS, and Linux (Flatpak) desktop clients - Android app (as obtained from Google play) - iOS app (as obtained from the Apple app store) - Chrome and Firefox browser extensions The only major client issue I found was with the unpackaged AppImage Linux desktop client. When authenticating users on desktop clients, the connector in the web vault opens a `bitwarden://` link, which my system refused to use the AppImage to open. It looks like the AppImage client either doesn't or can't register itself as the `x-scheme-handler` for `bitwarden`, preventing the redirect connector from handing off to the client. It worked fine with the Flatpak packaged Bitwarden client, so I'm chalking it up to my not configuring the AppImage client correctly. There is also a known issue upstream in the web vault where the redirect connector shows 'successful authentication', does not automatically close, and the JS 'close' button does not work. See [https://github.com/bitwarden/clients/issues/8554](https://github.com/bitwarden/clients/issues/8554) I also tested this while running PostgreSQL and MariaDB using the versions packaged in Debian 12; no issues. Fixes #4529 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-03-07 21:11:03 -06:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/vaultwarden#7104