Fix 2FA Remember to actually be 30 days (#6929)

Currently we always regenerate the 2FA Remember token, and always send that back to the client.
This is not the correct way, and in turn causes the remember token to never expire.

While this might be convenient, it is not really safe.
This commit changes the 2FA Remember Tokens from random string to a JWT.
This JWT has a lifetime of 30 days and is validated per device & user combination.

This does mean that once this commit is merged, and users are using this version, all their remember tokens will be invalidated.
From my point of view this isn't a bad thing, since those tokens should have expired already.

Only users who recently checked the remember checkbox within 30 days have to login again, but that is a minor inconvenience I think.

Signed-off-by: BlackDex <black.dex@gmail.com>

.github/workflows/release.yml

@@ -20,23 +20,25 @@ defaults:
   run:
     shell: bash
 
-env:
-  # The *_REPO variables need to be configured as repository variables
-  # Append `/settings/variables/actions` to your repo url
-  # DOCKERHUB_REPO needs to be 'index.docker.io/<user>/<repo>'
-  # Check for Docker hub credentials in secrets
-  HAVE_DOCKERHUB_LOGIN: ${{ vars.DOCKERHUB_REPO != '' && secrets.DOCKERHUB_USERNAME != '' && secrets.DOCKERHUB_TOKEN != '' }}
-  # GHCR_REPO needs to be 'ghcr.io/<user>/<repo>'
-  # Check for Github credentials in secrets
-  HAVE_GHCR_LOGIN: ${{ vars.GHCR_REPO != '' && github.repository_owner != '' && secrets.GITHUB_TOKEN != '' }}
-  # QUAY_REPO needs to be 'quay.io/<user>/<repo>'
-  # Check for Quay.io credentials in secrets
-  HAVE_QUAY_LOGIN: ${{ vars.QUAY_REPO != '' && secrets.QUAY_USERNAME != '' && secrets.QUAY_TOKEN != '' }}
+# A "release" environment must be created in the repository settings
+# (Settings > Environments > New environment) with the following
+# variables and secrets configured as needed.
+#
+# Variables (only set the ones for registries you want to push to):
+#   DOCKERHUB_REPO: 'index.docker.io/<user>/<repo>'
+#   QUAY_REPO: 'quay.io/<user>/<repo>'
+#   GHCR_REPO: 'ghcr.io/<user>/<repo>'
+#
+# Secrets (only required when the corresponding *_REPO variable is set):
+#   DOCKERHUB_REPO => DOCKERHUB_USERNAME, DOCKERHUB_TOKEN
+#   QUAY_REPO      => QUAY_USERNAME, QUAY_TOKEN
+#   GITHUB_TOKEN is provided automatically
 
 jobs:
   docker-build:
     name: Build Vaultwarden containers
     if: ${{ github.repository == 'dani-garcia/vaultwarden' }}
+    environment: release
     permissions:
       packages: write # Needed to upload packages and artifacts
       contents: read
@@ -106,10 +108,10 @@ jobs:
         with:
           username: ${{ secrets.DOCKERHUB_USERNAME }}
           password: ${{ secrets.DOCKERHUB_TOKEN }}
-        if: ${{ env.HAVE_DOCKERHUB_LOGIN == 'true' }}
+        if: ${{ vars.DOCKERHUB_REPO != '' }}
 
       - name: Add registry for DockerHub
-        if: ${{ env.HAVE_DOCKERHUB_LOGIN == 'true' }}
+        if: ${{ vars.DOCKERHUB_REPO != '' }}
         env:
           DOCKERHUB_REPO: ${{ vars.DOCKERHUB_REPO }}
         run: |
@@ -122,10 +124,10 @@ jobs:
           registry: ghcr.io
           username: ${{ github.repository_owner }}
           password: ${{ secrets.GITHUB_TOKEN }}
-        if: ${{ env.HAVE_GHCR_LOGIN == 'true' }}
+        if: ${{ vars.GHCR_REPO != '' }}
 
       - name: Add registry for ghcr.io
-        if: ${{ env.HAVE_GHCR_LOGIN == 'true' }}
+        if: ${{ vars.GHCR_REPO != '' }}
         env:
           GHCR_REPO: ${{ vars.GHCR_REPO }}
         run: |
@@ -138,10 +140,10 @@ jobs:
           registry: quay.io
           username: ${{ secrets.QUAY_USERNAME }}
           password: ${{ secrets.QUAY_TOKEN }}
-        if: ${{ env.HAVE_QUAY_LOGIN == 'true' }}
+        if: ${{ vars.QUAY_REPO != '' }}
 
       - name: Add registry for Quay.io
-        if: ${{ env.HAVE_QUAY_LOGIN == 'true' }}
+        if: ${{ vars.QUAY_REPO != '' }}
         env:
           QUAY_REPO: ${{ vars.QUAY_REPO }}
         run: |
@@ -155,7 +157,7 @@ jobs:
         run: |
           #
           # Check if there is a GitHub Container Registry Login and use it for caching
-          if [[ -n "${HAVE_GHCR_LOGIN}" ]]; then
+          if [[ -n "${GHCR_REPO}" ]]; then
             echo "BAKE_CACHE_FROM=type=registry,ref=${GHCR_REPO}-buildcache:${BASE_IMAGE}-${NORMALIZED_ARCH}" | tee -a "${GITHUB_ENV}"
             echo "BAKE_CACHE_TO=type=registry,ref=${GHCR_REPO}-buildcache:${BASE_IMAGE}-${NORMALIZED_ARCH},compression=zstd,mode=max" | tee -a "${GITHUB_ENV}"
           else
@@ -247,6 +249,7 @@ jobs:
     name: Merge manifests
     runs-on: ubuntu-latest
     needs: docker-build
+    environment: release
     permissions:
       packages: write # Needed to upload packages and artifacts
       attestations: write # Needed to generate an artifact attestation for a build
@@ -269,10 +272,10 @@ jobs:
         with:
           username: ${{ secrets.DOCKERHUB_USERNAME }}
           password: ${{ secrets.DOCKERHUB_TOKEN }}
-        if: ${{ env.HAVE_DOCKERHUB_LOGIN == 'true' }}
+        if: ${{ vars.DOCKERHUB_REPO != '' }}
 
       - name: Add registry for DockerHub
-        if: ${{ env.HAVE_DOCKERHUB_LOGIN == 'true' }}
+        if: ${{ vars.DOCKERHUB_REPO != '' }}
         env:
           DOCKERHUB_REPO: ${{ vars.DOCKERHUB_REPO }}
         run: |
@@ -285,10 +288,10 @@ jobs:
           registry: ghcr.io
           username: ${{ github.repository_owner }}
           password: ${{ secrets.GITHUB_TOKEN }}
-        if: ${{ env.HAVE_GHCR_LOGIN == 'true' }}
+        if: ${{ vars.GHCR_REPO != '' }}
 
       - name: Add registry for ghcr.io
-        if: ${{ env.HAVE_GHCR_LOGIN == 'true' }}
+        if: ${{ vars.GHCR_REPO != '' }}
         env:
           GHCR_REPO: ${{ vars.GHCR_REPO }}
         run: |
@@ -301,10 +304,10 @@ jobs:
           registry: quay.io
           username: ${{ secrets.QUAY_USERNAME }}
           password: ${{ secrets.QUAY_TOKEN }}
-        if: ${{ env.HAVE_QUAY_LOGIN == 'true' }}
+        if: ${{ vars.QUAY_REPO != '' }}
 
       - name: Add registry for Quay.io
-        if: ${{ env.HAVE_QUAY_LOGIN == 'true' }}
+        if: ${{ vars.QUAY_REPO != '' }}
         env:
           QUAY_REPO: ${{ vars.QUAY_REPO }}
         run: |
@@ -357,7 +360,7 @@ jobs:
 
       # Attest container images
       - name: Attest - docker.io - ${{ matrix.base_image }}
-        if: ${{ env.HAVE_DOCKERHUB_LOGIN == 'true' && env.DIGEST_SHA != ''}}
+        if: ${{ vars.DOCKERHUB_REPO != '' && env.DIGEST_SHA != ''}}
         uses: actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0
         with:
           subject-name: ${{ vars.DOCKERHUB_REPO }}
@@ -365,7 +368,7 @@ jobs:
           push-to-registry: true
 
       - name: Attest - ghcr.io - ${{ matrix.base_image }}
-        if: ${{ env.HAVE_GHCR_LOGIN == 'true' && env.DIGEST_SHA != ''}}
+        if: ${{ vars.GHCR_REPO != '' && env.DIGEST_SHA != ''}}
         uses: actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0
         with:
           subject-name: ${{ vars.GHCR_REPO }}
@@ -373,7 +376,7 @@ jobs:
           push-to-registry: true
 
       - name: Attest - quay.io - ${{ matrix.base_image }}
-        if: ${{ env.HAVE_QUAY_LOGIN == 'true' && env.DIGEST_SHA != ''}}
+        if: ${{ vars.QUAY_REPO != '' && env.DIGEST_SHA != ''}}
         uses: actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0
         with:
           subject-name: ${{ vars.QUAY_REPO }}

src/api/identity.rs

@@ -757,7 +757,6 @@ async fn twofactor_auth(
     use crate::crypto::ct_eq;
 
     let selected_data = _selected_data(selected_twofactor);
-    let mut remember = data.two_factor_remember.unwrap_or(0);
 
     match TwoFactorType::from_i32(selected_id) {
         Some(TwoFactorType::Authenticator) => {
@@ -789,13 +788,23 @@ async fn twofactor_auth(
         }
         Some(TwoFactorType::Remember) => {
             match device.twofactor_remember {
-                Some(ref code) if !CONFIG.disable_2fa_remember() && ct_eq(code, twofactor_code) => {
-                    remember = 1; // Make sure we also return the token here, otherwise it will only remember the first time
-                }
+                // When a 2FA Remember token is used, check and validate this JWT token, if it is valid, just continue
+                // If it is invalid we need to trigger the 2FA Login prompt
+                Some(ref token)
+                    if !CONFIG.disable_2fa_remember()
+                        && (ct_eq(token, twofactor_code)
+                            && auth::decode_2fa_remember(twofactor_code)
+                                .is_ok_and(|t| t.sub == device.uuid && t.user_uuid == user.uuid)) => {}
                 _ => {
+                    // Always delete the current twofactor remember token here if it exists
+                    if device.twofactor_remember.is_some() {
+                        device.delete_twofactor_remember();
+                        // We need to save here, since we send a err_json!() which prevents saving `device` at a later stage
+                        device.save(true, conn).await?;
+                    }
                     err_json!(
                         _json_err_twofactor(&twofactor_ids, &user.uuid, data, client_version, conn).await?,
-                        "2FA Remember token not provided"
+                        "2FA Remember token not provided or expired"
                     )
                 }
             }
@@ -826,10 +835,10 @@ async fn twofactor_auth(
 
     TwoFactorIncomplete::mark_complete(&user.uuid, &device.uuid, conn).await?;
 
+    let remember = data.two_factor_remember.unwrap_or(0);
     let two_factor = if !CONFIG.disable_2fa_remember() && remember == 1 {
         Some(device.refresh_twofactor_remember())
     } else {
-        device.delete_twofactor_remember();
         None
     };
     Ok(two_factor)

src/auth.rs

@@ -46,6 +46,7 @@ static JWT_FILE_DOWNLOAD_ISSUER: LazyLock<String> =
     LazyLock::new(|| format!("{}|file_download", CONFIG.domain_origin()));
 static JWT_REGISTER_VERIFY_ISSUER: LazyLock<String> =
     LazyLock::new(|| format!("{}|register_verify", CONFIG.domain_origin()));
+static JWT_2FA_REMEMBER_ISSUER: LazyLock<String> = LazyLock::new(|| format!("{}|2faremember", CONFIG.domain_origin()));
 
 static PRIVATE_RSA_KEY: OnceLock<EncodingKey> = OnceLock::new();
 static PUBLIC_RSA_KEY: OnceLock<DecodingKey> = OnceLock::new();
@@ -160,6 +161,10 @@ pub fn decode_register_verify(token: &str) -> Result<RegisterVerifyClaims, Error
     decode_jwt(token, JWT_REGISTER_VERIFY_ISSUER.to_string())
 }
 
+pub fn decode_2fa_remember(token: &str) -> Result<TwoFactorRememberClaims, Error> {
+    decode_jwt(token, JWT_2FA_REMEMBER_ISSUER.to_string())
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 pub struct LoginJwtClaims {
     // Not before
@@ -440,6 +445,31 @@ pub fn generate_register_verify_claims(email: String, name: Option<String>, veri
     }
 }
 
+#[derive(Serialize, Deserialize)]
+pub struct TwoFactorRememberClaims {
+    // Not before
+    pub nbf: i64,
+    // Expiration time
+    pub exp: i64,
+    // Issuer
+    pub iss: String,
+    // Subject
+    pub sub: DeviceId,
+    // UserId
+    pub user_uuid: UserId,
+}
+
+pub fn generate_2fa_remember_claims(device_uuid: DeviceId, user_uuid: UserId) -> TwoFactorRememberClaims {
+    let time_now = Utc::now();
+    TwoFactorRememberClaims {
+        nbf: time_now.timestamp(),
+        exp: (time_now + TimeDelta::try_days(30).unwrap()).timestamp(),
+        iss: JWT_2FA_REMEMBER_ISSUER.to_string(),
+        sub: device_uuid,
+        user_uuid,
+    }
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 pub struct BasicJwtClaims {
     // Not before

src/db/models/device.rs

@@ -1,6 +1,6 @@
 use chrono::{NaiveDateTime, Utc};
 
-use data_encoding::{BASE64, BASE64URL};
+use data_encoding::BASE64URL;
 use derive_more::{Display, From};
 use serde_json::Value;
 
@@ -67,10 +67,13 @@ impl Device {
     }
 
     pub fn refresh_twofactor_remember(&mut self) -> String {
-        let twofactor_remember = crypto::encode_random_bytes::<180>(&BASE64);
-        self.twofactor_remember = Some(twofactor_remember.clone());
+        use crate::auth::{encode_jwt, generate_2fa_remember_claims};
 
-        twofactor_remember
+        let two_factor_remember_claim = generate_2fa_remember_claims(self.uuid.clone(), self.user_uuid.clone());
+        let two_factor_remember_string = encode_jwt(&two_factor_remember_claim);
+        self.twofactor_remember = Some(two_factor_remember_string.clone());
+
+        two_factor_remember_string
     }
 
     pub fn delete_twofactor_remember(&mut self) {