Skip to content

Feature/persistent sessions#3264

Merged
uruwhy merged 4 commits intomitre:masterfrom
clutester:feature/persistent-sessions
Mar 24, 2026
Merged

Feature/persistent sessions#3264
uruwhy merged 4 commits intomitre:masterfrom
clutester:feature/persistent-sessions

Conversation

@clutester
Copy link
Copy Markdown
Contributor

Description

Fixes #3256

This PR introduces configurable, persistent session cookies across server restarts, addressing the friction developers and operators experience when frequently restarting the Caldera server.

Previously, auth_svc.py generated an ephemeral Fernet key in memory on every boot, immediately invalidating all existing browser sessions. This update shifts the session encryption key to the configuration file while maintaining backwards compatibility.

Key Changes:

  • Added session_cookie_key and session_expiration_days to conf/default.yml.
  • Updated app/utility/config_generator.py to automatically generate a secure, url-safe token for session_cookie_key when a new local.yml is created.
  • Modified app/service/auth_svc.py to read the configured key, properly pad/encode it to meet the strict 32-byte AES requirement of aiohttp_session, and apply the calculated max_age to the browser cookie.
  • Added fallback logic: if the new configuration keys are missing, the server gracefully reverts to the original ephemeral key generation.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update (Adding the new default.yml variables to the configuration guide)

How Has This Been Tested?

Extensive local testing was performed to ensure cryptographic requirements were met and no regressions were introduced to the login flow.

  • Persistence Test: Logged in, completely terminated the server, restarted the server, and verified the browser bypassed the login screen using the persistent cookie.
  • Expiration & Edge Case Test: Configured session_expiration_days to fractional values (e.g., 0.0001). Verified the math correctly converted to seconds, the browser dropped the cookie after the allotted seconds, and the backend safely redirected to the login screen without throwing HTTP 500 errors.
  • Automation Test: Deleted local.yml and ran server.py --fresh (without the --insecure flag). Verified config_generator.py successfully injected a secure, random string into the new configuration and that aiohttp accepted the generated key length.
  • Graceful Degradation Test: Removed the new keys from local.yml entirely. Verified the server gracefully fell back to generating an ephemeral key without crashing.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works

@clutester clutester force-pushed the feature/persistent-sessions branch 2 times, most recently from 78e98e6 to e7bfe0b Compare March 11, 2026 20:26
@clutester clutester requested a review from uruwhy March 14, 2026 04:54
@deacon-mp
Copy link
Copy Markdown
Contributor

Thanks for this work on persistent sessions! A few security concerns we noticed during review:

  1. Key padding with null bytes is dangerous: secret_key.ljust(32, b'\0')[:32] — if the decoded key is shorter than 32 bytes, the remainder is padded with \0. This drastically reduces AES key entropy (e.g., a 16-byte key means half the AES key is zeros).

  2. REPLACE_WITH_RANDOM_VALUE in default.yml — if someone runs with --insecure and the default config, this literal string becomes the session encryption key. That's a static, publicly-known key.

  3. Duplicate log lineself.log.debug('Using persistent session cookie key from config.') appears twice (lines 87 and 95).

  4. Broad exception fallback — the except Exception catches everything, then still uses the raw string as a key via str(raw_session_key).encode('utf-8'), which is then truncated/padded. This fallback path produces a weak key.

  5. Client-side-only expirationmax_age on the cookie is enforced by the browser, but the server has no session invalidation mechanism. A stolen cookie works until it expires.

We have PR #3282 (fix/issue-3256-session-persistence) which addresses the same issue (#3256) with a different approach — atomic file-based session key persistence that avoids the key-management pitfalls above. We'll likely move forward with that approach, but keeping this open for now as reference.

@uruwhy
Copy link
Copy Markdown
Contributor

uruwhy commented Mar 19, 2026

Thanks for this work on persistent sessions! A few security concerns we noticed during review:

  1. Key padding with null bytes is dangerous: secret_key.ljust(32, b'\0')[:32] — if the decoded key is shorter than 32 bytes, the remainder is padded with \0. This drastically reduces AES key entropy (e.g., a 16-byte key means half the AES key is zeros).
  2. REPLACE_WITH_RANDOM_VALUE in default.yml — if someone runs with --insecure and the default config, this literal string becomes the session encryption key. That's a static, publicly-known key.
  3. Duplicate log lineself.log.debug('Using persistent session cookie key from config.') appears twice (lines 87 and 95).
  4. Broad exception fallback — the except Exception catches everything, then still uses the raw string as a key via str(raw_session_key).encode('utf-8'), which is then truncated/padded. This fallback path produces a weak key.
  5. Client-side-only expirationmax_age on the cookie is enforced by the browser, but the server has no session invalidation mechanism. A stolen cookie works until it expires.

We have PR #3282 (fix/issue-3256-session-persistence) which addresses the same issue (#3256) with a different approach — atomic file-based session key persistence that avoids the key-management pitfalls above. We'll likely move forward with that approach, but keeping this open for now as reference.

@clutester We definitely want to be able to build on your existing PR, though you'd need to address some of the above concerns. We're thinking that maybe it doesn't make sense after all to have users specify their own cookie storage keys, since it then becomes the user's responsibility to securely generate random keys of sufficient length to avoid padding and whatnot. We do love the cookie expiration option that you added in the config file, so that can stay. As for the key itself, we can combine the two approaches and go for the following:

  • have caldera check for the cookie key storage file on startup (data/cookie_storage or something along those lines)
    • If it exists, decrypt the file and use the key
    • If it doesn't exist, generate a secure random key as before and save it to disk in an encrypted file (data/cookie_storage`)
  • Use the key expiration settings as currently implemented in your PR

Caldera's file_svc.py already provides methods to write/read encrypted files, so you can use those without having to implement anything new.

@clutester clutester force-pushed the feature/persistent-sessions branch from e7bfe0b to 856b9eb Compare March 20, 2026 01:45
@clutester
Copy link
Copy Markdown
Contributor Author

Thanks for this work on persistent sessions! A few security concerns we noticed during review:

  1. Key padding with null bytes is dangerous: secret_key.ljust(32, b'\0')[:32] — if the decoded key is shorter than 32 bytes, the remainder is padded with \0. This drastically reduces AES key entropy (e.g., a 16-byte key means half the AES key is zeros).
  2. REPLACE_WITH_RANDOM_VALUE in default.yml — if someone runs with --insecure and the default config, this literal string becomes the session encryption key. That's a static, publicly-known key.
  3. Duplicate log lineself.log.debug('Using persistent session cookie key from config.') appears twice (lines 87 and 95).
  4. Broad exception fallback — the except Exception catches everything, then still uses the raw string as a key via str(raw_session_key).encode('utf-8'), which is then truncated/padded. This fallback path produces a weak key.
  5. Client-side-only expirationmax_age on the cookie is enforced by the browser, but the server has no session invalidation mechanism. A stolen cookie works until it expires.

We have PR #3282 (fix/issue-3256-session-persistence) which addresses the same issue (#3256) with a different approach — atomic file-based session key persistence that avoids the key-management pitfalls above. We'll likely move forward with that approach, but keeping this open for now as reference.

@clutester We definitely want to be able to build on your existing PR, though you'd need to address some of the above concerns. We're thinking that maybe it doesn't make sense after all to have users specify their own cookie storage keys, since it then becomes the user's responsibility to securely generate random keys of sufficient length to avoid padding and whatnot. We do love the cookie expiration option that you added in the config file, so that can stay. As for the key itself, we can combine the two approaches and go for the following:

  • have caldera check for the cookie key storage file on startup (data/cookie_storage or something along those lines)

    • If it exists, decrypt the file and use the key
    • If it doesn't exist, generate a secure random key as before and save it to disk in an encrypted file (data/cookie_storage`)
  • Use the key expiration settings as currently implemented in your PR

Caldera's file_svc.py already provides methods to write/read encrypted files, so you can use those without having to implement anything new.

Thank you for the review below are the recent changes per your request:

Removed session_cookie_key from YAML to prevent weak/default keys.

Implemented automated key persistence using file_svc to read/write an encrypted cookie_storage file in the data directory.

Retained the session_expiration_days config as requested.
This ensures high-entropy 32-byte keys are used by default without requiring user intervention."

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@uruwhy
Copy link
Copy Markdown
Contributor

uruwhy commented Mar 24, 2026

verified generated session cookie was saved encrypted at appropriate file location
verified cookie expired (tested with 0.0001 days)
verified cookie persisted during server reboots
verified new key was generated if cookie storage file is removed

added some small changes to fix unit tests, remove unused imports, and update .gitignore to ignore the cookie storage file

Copy link
Copy Markdown
Contributor

@uruwhy uruwhy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you very much for your contribution!

@uruwhy uruwhy merged commit 29706f9 into mitre:master Mar 24, 2026
11 checks passed
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

Successfully merging this pull request may close these issues.

Have login session persist through server restart

3 participants