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

Optimize nginx configs for performance by default #10151

Merged
merged 13 commits into from
May 31, 2024

Conversation

testaccount90009
Copy link
Contributor

⚠️ Note on feature completeness ⚠️

We are narrowing the scope of acceptable enhancements to DefectDojo in preparation for v3. Learn more here:
https://github.com/DefectDojo/django-DefectDojo/blob/master/readme-docs/CONTRIBUTING.md

Description

worker_processes defaulting to 1 leads to sub-optimal performance. Changing this to 'auto' allows for more performance out-of-the-box at no detriment to anything else. This will help newer users from experiencing performance limitations due to the 1 worker_process, when most modern systems have multiple cores to take advantage of.

Test results

I don't believe tests are needed for a change such as this, as nothing should be impacted except for the capability by default for nginx to utilize available system cores for its processes, in place of the default currently as 1.

Documentation

Please update any documentation when needed in the documentation folder)

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

Extra information

Please clear everything below when submitting your pull request, it's here purely for your information.

Moderators: Labels currently accepted for PRs:

  • Import Scans (for new scanners/importers)
  • enhancement
  • performance
  • feature
  • bugfix
  • maintenance (a.k.a chores)
  • dependencies
  • New Migration (when the PR introduces a DB migration)
  • settings_changes (when the PR introduces changes or new settings in settings.dist.py)

Contributors: Git Tips

Rebase on dev branch

If the dev branch has changed since you started working on it, please rebase your work after the current dev.

On your working branch mybranch:

git rebase dev mybranch

In case of conflict:

 git mergetool
 git rebase --continue

When everything's fine on your local branch, force push to your myOrigin remote:

git push myOrigin --force-with-lease

To cancel everything:

git rebase --abort

Squashing commits

git rebase -i origin/dev
  • Replace pick by fixup on the commits you want squashed out
  • Replace pick by reword on the first commit if you want to change the commit message
  • Save the file and quit your editor

Force push to your myOrigin remote:

git push myOrigin --force-with-lease

worker_processes defaulting to 1 leads to sub-optimal performance.  Changing this to 'auto' allows for more performance out-of-the-box at no detriment to anything else.  This will help newer users from experiencing performance limitations due to the 1 worker_process, when most modern systems have multiple cores to take advantage of.
Copy link

dryrunsecurity bot commented May 7, 2024

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Configured Codepaths Analyzer 0 findings
Sensitive Files Analyzer 0 findings
AppSec Analyzer 0 findings
Authn/Authz Analyzer 0 findings
Secrets Analyzer 0 findings

Note

🟢 Risk threshold not exceeded.

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy 🤖. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.

Summary:

The provided code changes in the nginx.conf and nginx_TLS.conf files appear to be focused on optimizing the performance and security of the Nginx web server configuration. The key changes include adjusting the number of worker processes, enabling Gzip compression, configuring TLS/SSL settings, and setting up metrics and health check endpoints.

From an application security perspective, the changes do not introduce any obvious security vulnerabilities. Instead, they demonstrate security-conscious practices, such as using modern TLS/SSL protocols, disabling server tokens, and protecting the metrics and health check endpoints with basic authentication.

However, it is important to ensure that the Gzip compression is not applied to sensitive data, that the communication between Nginx and the backend application (e.g., Django) is properly secured, and that the client upload size limit is set to an appropriate value to prevent potential abuse or denial-of-service attacks.

Files Changed:

  1. nginx/nginx.conf:

    • Adjusted the worker_processes setting to auto for improved performance.
    • Enabled Gzip compression for various content types.
    • Included endpoints for Django metrics, Nginx status, and Kubernetes health checks.
    • Configured an upstream block for the uwsgi_server.
    • Set the client_max_body_size to 800MB, which should be reviewed to ensure it is not set too high.
  2. nginx/nginx_TLS.conf:

    • Adjusted the worker_processes setting to auto for improved performance.
    • Implemented a comprehensive TLS/SSL configuration, including the use of modern ciphers and protocols.
    • Enabled Gzip compression for various content types.
    • Configured proxy settings, such as forwarding the X-Forwarded-For and X-Forwarded-Proto headers.
    • Included endpoints for Django metrics, Nginx status, and Kubernetes health checks, protected by basic authentication.

Powered by DryRun Security

Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

@cneill cneill merged commit 240bc6b into DefectDojo:dev May 31, 2024
123 checks passed
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.

None yet

5 participants