Skip to content

Conversation

@JonBendtsen
Copy link
Contributor

the status page is not needed for dolibarr, AND it might be revealed if you have a proxy in front of dolibarr because traffic will seem to be local because it comes from the proxy :-(

@creekorful creekorful self-assigned this Mar 20, 2025
@creekorful
Copy link
Member

Hello @JonBendtsen,

I don't think we need this PR if we add a way to disable the modules here in this PR #47. That would be much more convenient.

@creekorful creekorful added the question Further information is requested label Mar 20, 2025
@JonBendtsen
Copy link
Contributor Author

Hello @JonBendtsen,

I don't think we need this PR if we add a way to disable the modules here in this PR #47. That would be much more convenient.

I totally disagree, this is actually even more needed than PR#47 because this is a potential leak of information about the Dolibarr server setup.

  • It makes no diffference for Dolibarr if apache has this status page or not
  • the status page will reveal information about the setup, information that people don't need. Yes this status page by default is for local connections only - but more and more people run containers in Kubernetes or some other container behind a proxy, and unless the setup is done correctly, to apache inside the dolibarr container the proxy will seem to be a local connection and then it will respond with the status page - something we don't want. So let's close this information leak.

Those that need the status page can enable it.

@cibero42
Copy link
Contributor

Hey @creekorful ,

I need to agree with @JonBendtsen on disabling this module, since it may provide an adversary with information that can be used to refine exploits that depend on measuring server load, as per CIS Benchmark recomendations.

I'm a defensor of the "Secure by design" philosophy, and I think that we shouldn't expect a regular user to think about disabling this by themselves.

I disagree with PR #47 though - as @tuxgasy mentioned, we already have a feature that allow advanced users to enable modules and do their modifications.

@JonBendtsen
Copy link
Contributor Author

I disagree with PR #47 though - as @tuxgasy mentioned, we already have a feature that allow advanced users to enable modules and do their modifications.

Then why not do the same with the PHP settings?

@tuxgasy
Copy link
Contributor

tuxgasy commented Mar 22, 2025

I disagree with PR #47 though - as @tuxgasy mentioned, we already have a feature that allow advanced users to enable modules and do their modifications.

Then why not do the same with the PHP settings?

I agree to apply the same rule for PHP. PHP settings was integrated before the init script.

@JonBendtsen
Copy link
Contributor Author

@tuxgasy I dont think that PHP settings should be removed

@creekorful
Copy link
Member

(Copy paste of my answer from this other PR)

Since there is a consensus for the base image to provide sane (and safe) defaults, let's move on with this PR.

On a side note I would be great to check the list of enabled apache modules and use this PR to disable all modules who does not need to be enabled.

If the user is willing to enable a module this could be done using the custom init scripts like @tuxgasy explained in another PR.

Cheers,

vim-tiny \
cron \
&& apt-get autoremove -y \
&& a2dismod status \
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to another RUN instruction just after this one and before the "Get Dolibarr" one?

Something like this:

# Disable useless Apache modules to provide safe defaults
RUN a2disconf status

@creekorful creekorful added enhancement New feature or request and removed question Further information is requested labels Mar 22, 2025
@cibero42
Copy link
Contributor

@tuxgasy I think that removing current PHP envs will lead to the creation of issues and confusion, from users unaware of these changes.

Sadly envs is one of the things that it is hard to remove without creating a little "chaos".

We can assume that those who use this envs are advanced users, and we can create a big text on the beginning of the README regarding these changes, but we will for sure disrupt some production environments by doing so.

@cibero42
Copy link
Contributor

On a side note I would be great to check the list of enabled apache modules and use this PR to disable all modules who does not need to be enabled.

Perhaps we can have a look on CIS Benchmark recomendations - it should be a good start.

@JonBendtsen
Copy link
Contributor Author

replaced by #54

@creekorful creekorful closed this Mar 22, 2025
@creekorful
Copy link
Member

Closing as superseded by #54.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants