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

Cache the fernet key in the /config volume #55

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jchonig
Copy link

@jchonig jchonig commented Nov 13, 2024

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

When the fernet key is not provided in an environment variable, cache the generated key in /config/fernet.key. If this is a persistent volume, the key will be reused on container restarts.

This change will also accept a fernet key with or w/o enclosing "b'" and "'". This avoids confusion as /app/fernet-key.py prints it with the byte-string quotes.

Benefits of this PR and context:

This is an easier method to preserve the key to prevent issues when the container restarts. Even on occasional container restarts I was having problems getting the login page w/o clearing cookies.

How Has This Been Tested?

I've been running this change for several days in several docker compose container stacks on x86_64 with a /config volume mounted and FERNETKEY not defined and a volume mounted. My gateway errors on the login page have gone away.

I also switched between defining FERNETKEY (to one generated by and cached in /config) and mounting the /config volume to ensure that the same key was being used (by reloading the web page and watching for tracebacks in the container logs).

Source / References:

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request! Be sure to follow the pull request template!

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-213d9213-dev-bc3357c0460341d94d809e004e1493e8a8c24d90-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-213d9213-dev-bc3357c0460341d94d809e004e1493e8a8c24d90-pr-55/shellcheck-result.xml

Tag Passed
amd64-3.4.4-pkg-213d9213-dev-bc3357c0460341d94d809e004e1493e8a8c24d90-pr-55
arm64v8-3.4.4-pkg-213d9213-dev-bc3357c0460341d94d809e004e1493e8a8c24d90-pr-55

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-213d9213-dev-85980e86daa7eaa4f791b18a9d79b8741cbe77f4-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-213d9213-dev-85980e86daa7eaa4f791b18a9d79b8741cbe77f4-pr-55/shellcheck-result.xml

Tag Passed
amd64-3.4.4-pkg-213d9213-dev-85980e86daa7eaa4f791b18a9d79b8741cbe77f4-pr-55
arm64v8-3.4.4-pkg-213d9213-dev-85980e86daa7eaa4f791b18a9d79b8741cbe77f4-pr-55

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-213d9213-dev-5a0c2ee4f4b9540412a2a0677311ebe2e12855f6-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-213d9213-dev-5a0c2ee4f4b9540412a2a0677311ebe2e12855f6-pr-55/shellcheck-result.xml

Tag Passed
amd64-3.4.4-pkg-213d9213-dev-5a0c2ee4f4b9540412a2a0677311ebe2e12855f6-pr-55
arm64v8-3.4.4-pkg-213d9213-dev-5a0c2ee4f4b9540412a2a0677311ebe2e12855f6-pr-55

@aptalca
Copy link
Member

aptalca commented Nov 25, 2024

This container is meant to be fully ephemeral, in other words no need for a bind mounted volume.

Your issue of sessions being invalidated should only occur when the container is recreated, not when it's restarted. And that issue can be solved by passing in a fernet key via env var.

@jchonig
Copy link
Author

jchonig commented Nov 25, 2024

@aptalca A bind mounted volume is optional with my patch.

Whenever I docker compose down on the stack (say to pull a newer version of an image), I would have to manually remove the old cookies from my browser session. I don't know fi the 10m cache is supposed to help, but it doesn't always. Maybe because some of the pages have periodic reloads scheduled.

Yes, I could write CM code to create the key and pass it in the environment. However that is extra code I have to write to automate this on a bunch of containers. Vs just adding a volume to the container config.

In addition, the key returned by /app/fernet-key.py is not accepted as a value for FERNETKEY because .decode() was not called on it, so it includes the b'...'.

Thanks.

Jeff

@aptalca
Copy link
Member

aptalca commented Nov 25, 2024

You only need to create the key once and add it to your compose as an env var. That's it.

You can create the fernet key however you like. You're not meant to use the internal script as that's for container use only. There are tons of different ways to create a fernet key, including online generators (make sure to use a local one if you do)

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-dea0df46-dev-f8defefe0ad037d9d9736d3723071aa05ec5d11e-pr-55/index.html
https://ci-tests.linuxserver.io/lspipepr/ldap-auth/3.4.4-pkg-dea0df46-dev-f8defefe0ad037d9d9736d3723071aa05ec5d11e-pr-55/shellcheck-result.xml

Tag Passed
amd64-3.4.4-pkg-dea0df46-dev-f8defefe0ad037d9d9736d3723071aa05ec5d11e-pr-55
arm64v8-3.4.4-pkg-dea0df46-dev-f8defefe0ad037d9d9736d3723071aa05ec5d11e-pr-55

@LinuxServer-CI
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants