Log to stdout
Fix for #8120
Hi there :wave:, @dryrunsecurity here, below is a summary of our analysis and findings.
| DryRun Security | Status | Findings |
|---|---|---|
| Sensitive Functions Analyzer | :white_check_mark: | 0 findings |
| Configured Sensitive Files Analyzer | :x: | 1 findings |
| Sensitive Files Analyzer | :white_check_mark: | 0 findings |
[!Note] :red_circle: Risk threshold exceeded. Adding a reviewer if one is configured in
.dryrunsecurity.yaml.notification list: @mtesauro @grendel513
[!Tip] Get answers to your security questions. Add a comment in this PR starting with @dryrunsecurity. For example...
@dryrunsecurity What are common security issues with web application cookies?
Powered by DryRun Security
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
DryRun Security Summary
The pull request updates the configuration and logging settings for the uWSGI and Celery components of the DefectDojo application, improving the overall logging and monitoring capabilities, without introducing any obvious security concerns, but requiring a review of the broader application context and security requirements to ensure the ongoing security and integrity of the application.
Expand for full summary
Summary:
The code changes in this pull request are focused on updating the configuration and logging settings for the uWSGI and Celery components of the DefectDojo application. The changes do not introduce any obvious security concerns, but there are a few areas that should be reviewed to ensure the ongoing security and integrity of the application.
The key changes include:
- Addition of the
--log-masteroption to the uWSGI commands, which improves the overall logging and monitoring capabilities of the application. - Modification of the Celery worker and Celery Beat configurations to redirect their logs to the standard output, which can be useful for monitoring and troubleshooting.
- Handling of environment variables and configuration overrides, which should be reviewed to ensure that no sensitive information is being exposed.
- File permissions and ownership, which should be carefully managed to prevent potential security issues.
While these changes do not introduce any immediate security vulnerabilities, it's important to review the broader application context and security requirements to ensure that the application is properly secured and configured. This includes regular review of dependencies, libraries, and frameworks, implementation of secure coding practices, and comprehensive security testing throughout the development and deployment lifecycle.
Files Changed:
docker/entrypoint-uwsgi-dev.sh: This file sets up and runs the uWSGI server for the Django-based application in a development environment. The key change is the addition of the--log-masteroption to the uWSGI command, which ensures that the master process logs all the output.docker/entrypoint-celery-worker.sh: This file configures the Celery worker's logging and worker pool settings. The changes include adding the--logfile=stdoutparameter to direct the Celery worker's logs to the standard output.docker/entrypoint-uwsgi.sh: This script sets up and starts the uWSGI server for the DefectDojo application. The changes include the addition of the--log-masteroption to the uWSGI command, which can improve logging and monitoring capabilities.docker/entrypoint-celery-beat.sh: This file serves as the entrypoint for the Celery Beat process in the Docker environment. The changes include adding the--logfile=stdoutoption to the Celery Beat command, which redirects the logs to the standard output.
Code Analysis
We ran 9 analyzers against 4 files and 0 analyzers had findings. 9 analyzers had no findings.
Riskiness
:green_circle: Risk threshold not exceeded.
@kiblik we are also interested in the fix. Did you abandon the PR?
At the time, when I opened this PR, I was doing multiple tests with log-master, logto, logto2, ... to be able to achieve that Errors will be logged to stderr and everything else to stdout.
I checked many online resources (like https://github.com/unbit/uwsgi/issues/1601) but had no success.
I hoped I would have time in the future to come back with a fresh mind and test some new approaches but I had never time.
If I'm not wrong, log-master = True (a.k.a. --log-master) will push all logs only tostdout. If it is a suitable solution, I might test it one more time (to be sure about my previous sentence) and I can offer it as solution (change draft PR to open PR).
I quickly asked in the issue #8120 if still somebody else works on it, and prefers a different solution. From my point that sounds definitely like a valid approach and according to the discussion, currently the only one, which is easily implementable. The only question is, do we add just another flag or do we switch to .ini file like mozilla does https://github.com/mozilla/remote-settings/commit/a9f4da48944b8b3d8f59a235aa4cd0c1f6931387. But the latter would need some more discussion.
So after a small test: Yes, log-master redirects all application logs to stdout (instead of stderr). In my opinion, it might not be best but better than all logs in stderr.
However, log-master does not work for celery(worker|beat). I tried --logfile /dev/stdout but without any luck.
Unfortunately, I do not have the capacity to investigate alternatives. But I will be happy if somebody is willing to help here.
@kiblik I have tried with --logfile=stdout and logs became info logs as they should be. Would you like to test it on your side?
Not sure if I'm doing sth wrong or if I'm testing with wrong methods but when I apply this: https://github.com/DefectDojo/django-DefectDojo/pull/9708/commits/0fc5d0815896453724da73c62efaa9249d9d4b24 I still see logs like
[12/Sep/2024 16:42:19] INFO [celery.beat:632] beat: Starting...
in stderr, not in stdout.
@kiblik I tested my custom image on the instance deployed in Kubernetes. I tried to check the files, but I could not open them. However, I see in my Monitoring tool that logs are now shown as INFO, and before they were shown as ERRORS.
Here is an example of my change for the beat:
exec celery --app=dojo \ beat \ --logfile=stdout \ --pidfile=/var/run/defectdojo/celery-beat.pid \ --schedule=/var/run/defectdojo/celerybeat-schedule
It looks like there has not been any activity here for a while. In order to keep the list of pull requests in a manageable state, we are closing this one for now. If we are making a mistake here, please reopen the pull request, and leave us a note 😄