FusionAuth / fusionauth-issues

FusionAuth issue submission project
https://fusionauth.io
92 stars 12 forks source link

Support readonly docker filesystem #1924

Open mooreds opened 2 years ago

mooreds commented 2 years ago

Support readonly docker filesystem

Problem

I want to run the FusionAuth docker image with a readonly filesystem, but I can't. FusionAuth fails to start up, I see a message like:

sed: couldn't open temporary file /opt/openjdk/conf/security/sedxxx: Read-only file system

Solution

Allow me to set readOnlyRootFilesystem: true on my docker image. Maybe do any editing in the first stage?

Alternatives/workarounds

Additional context

Came out of https://github.com/FusionAuth/fusionauth-containers/issues/87 , reported by @GlebKuzmich

This behavior began in 1.37.

Looks related to https://github.com/FusionAuth/fusionauth-issues/issues/1814

Community guidelines

All issues filed in this repository must abide by the FusionAuth community guidelines.

How to vote

Please give us a thumbs up or thumbs down as a reaction to help us prioritize this feature. Feel free to comment if you have a particular need or comment on how this feature should work.

kgogolek commented 2 years ago

Hi

I'm not sure if this will be helpful but the way we got around it so far was to build a Dockerfile based off your docker image and do the sed there.

#####
FROM fusionauth/fusionauth-app:1.40.2

RUN sed -i -E '/^.*disallowAlg.*xmldsig.*$/d' "${JAVA_HOME}/conf/security/java.security"

COPY --chown=fusionauth:fusionauth --chmod=775 ./files/start.sh /usr/local/fusionauth/fusionauth-app/bin/start.sh

Then I copied over the start.sh and removed the sed part and that seems to work for us keeping the functionality the way it was. Obviously this is a bit of a pain going forward as we'll have to check if the start.sh doesn't have any changes beyond the sed.

I appreciate that there's probably a reason why the sed check is in start.sh but I think if you guys were able to remove the sed if:

# The following SHA-1 algorithms were removed in Java 17. If clients want to use them, it is up to them.
# https://github.com/FusionAuth/fusionauth-site/issues/1202
if [[ ${OS} == "Darwin" ]]; then
  sed -i '' -E '/^.*disallowAlg.*xmldsig.*$/d' "${JAVA_HOME}/conf/security/java.security"
else
  sed -i -E '/^.*disallowAlg.*xmldsig.*$/d' "${JAVA_HOME}/conf/security/java.security"
fi

from start.sh and into the original Dockerfile, or have some sort of temp if condition in the start.sh to ignore that logic altogether then that would help us out going forward.

mooreds commented 2 years ago

I think we need the sed command in there because that is required to support some older signing algorithms. I think we can do it here: https://github.com/FusionAuth/fusionauth-containers/blob/master/docker/fusionauth/fusionauth-app/Dockerfile#L81 when we are building the previous stage. That might solve the issue, and let us modify start.sh.

Thanks for sharing your workaround!

michaelholtermann commented 1 week ago

Is there a hard requirement to enable the weak SHA-1 algorithm? They have been disabled by https://bugs.openjdk.org/browse/JDK-8259709.

It is nice to provide FusionAuth's users with a somehow smooth transition to better algorithms, but after 3 years, maybe it's time to say good bye, with some clear note in the release notes?

If, for whatever reason, users would still need to use them, wouldn't it be possible to use -Djava.security.properties instead?

michaelholtermann commented 1 week ago

Introduced with https://github.com/FusionAuth/fusionauth-site/issues/1202

mooreds commented 1 week ago

It is nice to provide FusionAuth's users with a somehow smooth transition to better algorithms, but after 3 years, maybe it's time to say good bye, with some clear note in the release notes?

We broke live deployments the last time we tried to do this. Some details here: https://github.com/FusionAuth/fusionauth-issues/issues/1814

So we're leery of breaking people again. If we did, we'd want to follow our deprecation policy and give plenty of notice.

If, for whatever reason, users would still need to use them, wouldn't it be possible to use -Djava.security.properties instead?

Have you tested that workaround, @michaelholtermann ? That is an interesting approach.

michaelholtermann commented 1 week ago

I see, and totally agree with that approach. Never break things silently 👍 So any change would need some good test coverage.

But unfortunately, I have no test case at hand, especially no SAMLv2 integration. I just read it from the docs in that file.