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

NO-JIRA fix Dockerfile security #736

Closed

Conversation

Ant0wan
Copy link

@Ant0wan Ant0wan commented Jan 13, 2025

Description

This PR introduces updates to the Dockerfile for improved runtime security of the SonarQube container. The changes ensure the application can run as a non-root user and avoid writing to the root filesystem, which enhances isolation and security, particularly in Kubernetes environments.


Changes Made

  1. Shell Directive:

    • Introduced the SHELL directive with flags errexit, nounset, and xtrace to ensure robust error handling and debugging across the Dockerfile directives. This change prevents common scripting pitfalls and ensures more predictable behavior in the container build process.
  2. User and Group Management:

    • Replaced the previous useradd command to create a dedicated sonarqube group with GID 3000, and assigned the sonarqube user to this group. This ensures that the sonarqube user does not run with a GID of 0 (root group), enhancing security.
  3. File Ownership:

    • Updated ownership of /opt/sonarqube to the sonarqube user and group using chown. This ensures that the application does not attempt to write to root-owned directories, avoiding potential privilege escalations.
  4. Reordered Dockerfile Instructions:

    • Moved the USER sonarqube declaration closer to where the user is set up to improve readability and maintain consistency.

Motives and Benefits

  • Improved Security:

    • Running the application as a non-root user minimizes potential attack vectors and aligns with best practices for containerized applications.
    • Proper user and group segregation reduces the risk of privilege escalation.
  • Kubernetes Compatibility:

    • Running as non-root makes the container Kubernetes-compliant, as many clusters enforce non-root execution policies for workloads. Still work for already in-place deployments.
  • Filesystem Isolation:

    • Ensuring that sonarqube does not write to the root filesystem enhances container isolation and data integrity.

It allows such Kubernetes directives in deployment manifest (non-mandatory):

          securityContext:
            allowPrivilegeEscalation: false
            privileged: false
            readOnlyRootFilesystem: true
            runAsGroup: 3000
            runAsNonRoot: true
            runAsUser: 1000

Checklist

  • Explain the motive for the contribution: Improving security, isolation, and Kubernetes compatibility.
  • If images were modified, verify they work: Verified via run-tests.sh imagedir..
  • Test on Linux: Ensure images run correctly on Linux.

Many thanks for reviewing this PR and for your time!

@Ant0wan
Copy link
Author

Ant0wan commented Jan 13, 2025

@jCOTINEAU kind ping just to know whether the PR is seen or not :-)

@jCOTINEAU
Copy link
Collaborator

jCOTINEAU commented Jan 15, 2025

hello @Ant0wan, thanks a lot for the time you took to participate and enhance our product.

Nonetheless the current UID/GID "state" of our Dockerfiles is expected, we decided to follow OpenShift bestpractices described here.

We discussed that a lot internally, and we think Openshift made good tradeoffs, the best still would be to use full user-mapping as every other solution has downsides.

The current setup already allow for this securityContext:

containerSecurityContext:
  allowPrivilegeEscalation: false
  runAsNonRoot: true
  runAsUser: 1000
  runAsGroup: 0
  seccompProfile:
    type: RuntimeDefault
  capabilities:
    drop: ["ALL"]

readOnlyRootFs has been reverted has it was causing some issues with some context, but as a fresh user you should be able to activate it.

Nonetheless there are still interesting findings in your comments/changes and i'll take some more time to review it (probably not in the upcoming days, but in a week).

Hope this clarify a bit the situation.

Thanks again a lot for participating in the community, this means a lot to us.

@Ant0wan
Copy link
Author

Ant0wan commented Jan 16, 2025

[Suggestion] Align with Kubernetes Security Standards

Running an App with Elevated Privileges (usergroup 0)

  • Group 0: This is typically the root group in Unix-like systems, granting administrative privileges—which is something we should avoid for security reasons.
    If an app runs as user 1000 but is associated with group 0, it effectively has root-like privileges due to its association with the root group. This approach undermines the principle of least privilege and introduces potential security risks.

Aligning with Kubernetes Standards

As this Dockerfile is tailored to OpenShift, it does not fully adhere to generic Docker or Kubernetes security standards. Would you consider creating an image, such as sonarqube-community-x.x.x-kubernetes, specifically designed to align with Kubernetes and Docker deployment standards?

Here are some areas of improvement:

  • Labels: Ensure proper labels are used to support Kubernetes-native deployments and resource management.
  • Users and Filesystem: Align with Kubernetes best practices by avoiding the use of root or group 0. Instead:
    • Run the app as a non-root user.
    • Use a dedicated group with restricted privileges.
    • Ensure filesystem permissions are configured to support these changes securely.

These changes would make the image more secure and better suited for Kubernetes/Docker environments. What are your thoughts on this?

Regression in Security Practices

I also noticed that in the past, there were efforts to ensure a more secure deployment, such as in this commit:
Dockerfile: Line 23.
These changes aligned better with secure deployment practices. What happened to this secured approach, and why was the configuration reverted to use the root group?
Restoring this earlier practice or enhancing it further would significantly improve security and compliance with Kubernetes standards.

@jCOTINEAU
Copy link
Collaborator

Hello again,

The root group 0 on its own is not a major security problem in the docker/kubernetes context.
Openshift is the most secure kubernetes provider, hence the current setup is compatible with classic kubernetes and we wont be releasing a specific image.

This current setup has also been validated by Docker hub official images maintainers, and they check and enforce best security practices.

Nonetheless it has been a long running discussion, and what we will be providing soon is a way to user either a fix UID or gid=0

You can find more details here as this should acommodate for other use cases.

Thanks a lot for taking the time to participate in the community and provide interesting discussions. I'll be closing the PR and discuss that internally with PMs

@jCOTINEAU jCOTINEAU closed this Jan 20, 2025
@Ant0wan
Copy link
Author

Ant0wan commented Jan 21, 2025

Hi @jCOTINEAU,

Thank you for your response. The purpose of this change was to ensure the application does not run with root group privileges, as the current setup uses the root group. It would be preferable to configure it to run with a non-root group.

Could you let me know if this change might be implemented soon? Unfortunately, I don’t have visibility into the backlog, roadmap, or the internal discussions you mentioned.

At my company, we are unable to use OpenShift and, like many organizations, we rely on Kubernetes. Running workloads with root group privileges is not an option for us due to the significant security risks it poses. I must admit, I am surprised by this approach, especially considering it comes from a security-focused company.

Looking forward to your thoughts!

@jCOTINEAU
Copy link
Collaborator

To be honest we were also really surprised seing how they were doing it. After researching a bit, we figured out that in a modern cloud era, root group alone does not pose any security risk. But still i am surprised as openshift could have picked a very high fixed number and thats it.

The changes that we are implementing right now will allow to either do:

  • ANY UID and strict root group 0 (equivalent to right now)
  • strict UID 1000 and any GID

Hope this will solve your issues

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.

4 participants