SonarSource / docker-sonarqube

:whale: SonarQube in Docker
https://hub.docker.com/_/sonarqube/
GNU Lesser General Public License v3.0
1.38k stars 1.02k forks source link

Fix docker:S2612 while setting permissions to SQ folders #663

Closed carminevassallo closed 9 months ago

jCOTINEAU commented 9 months ago

I think we need to extend a bit the permission system, as the following block

chown -R sonarqube:sonarqube "${SQ_DATA_DIR}" "${SQ_EXTENSIONS_DIR}" "${SQ_LOGS_DIR}" "${SQ_TEMP_DIR}"; \
chmod -R 774 "${SQ_DATA_DIR}" "${SQ_EXTENSIONS_DIR}" "${SQ_LOGS_DIR}" "${SQ_TEMP_DIR}"; \

will break installation where uid is not 1000 by not allowing random uid to write into the temp folders.

docker run --rm -it -u 1001 will raise permission issue.

While looking at openshift documentation, which seems to be the best practice in terms of random uid setup

It seems we should be using the root group as a fixed entity to give permissions to. while being more elegant than having full open, I don't really see how it improves security ?

If another app that follows the same permission pattern manages to escape to the host, then they will have access to your filesystem anyway by being part of the root group.

And if it is your app that has a flaw then anyway the intruder will have the same permission as the user running the app, therefore gaining access.

Anyway we should follow best practices wdyt ?

carminevassallo commented 9 months ago

Mmm, very good catch! Supporting Openshift's arbitrary user ids means we need to add the sonarqube user to the root group and then only assign permissions at the group level.

I would vote for changing the permissions of those folders for two reasons:

carminevassallo commented 9 months ago

I drafted a change in the 9/community/Dockerfile only. Let me know what you think @jCOTINEAU, however, we need to adapt parts of the helm chart probably...

jCOTINEAU commented 9 months ago

hmm that's true, in order to not change too much the helm chart I would suggest something like this:

    chown sonarqube:root ${SONARQUBE_HOME}; \
    chmod -R 550 ${SONARQUBE_HOME}; \
    chown -R sonarqube:root "${SQ_DATA_DIR}" "${SQ_EXTENSIONS_DIR}" "${SQ_LOGS_DIR}" "${SQ_TEMP_DIR}"; \
    chmod -R 770 "${SQ_DATA_DIR}" "${SQ_EXTENSIONS_DIR}" "${SQ_LOGS_DIR}" "${SQ_TEMP_DIR}"; \

With that, the default helm chart parameter should work (have not tested yet) but if a user changed both runAsUser and runAsGroup in the helm chart it will break.

I would suggest using this setup and changing helm default with runAsUser: 1000 runAsGroup: 0 to follow closely what would happen on OpenShift while still having broad compatibility.

It would look like the best tradeoff in terms of user-friendliness and security, but we will have to clearly communicate the change as it matters a lot

Wdyt ?

sonarqube-next[bot] commented 9 months ago

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube