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

fix(deps): build psycopg3 instead of using pre-build binary #10491

Merged
merged 1 commit into from
Jul 12, 2024
Merged

fix(deps): build psycopg3 instead of using pre-build binary #10491

merged 1 commit into from
Jul 12, 2024

Conversation

gietschess
Copy link
Contributor

@gietschess gietschess commented Jul 2, 2024

Description

Since the update of the base python docker image from alpine:3.16 to 3.20 we've had segmentation faults while connecting to aws rds postgres databases. With the debug environment variable from the psycopg2 dependency we found connection issues while connecting to our database (see logs below).

Building the psycopg3 postgresql adapter instead of using the pre-build dependency as it's described in the psycopg3 installation documentation the connection could be established again as expected.

Also see the slack thread on this issue.

Test results

I've locally build and tested both the alpine and debian django +nginx dockerimages and tested them against the bitnami and aws rds postgres database.

Extra information

Additional logs:

Debug logs with the alpine image containing the psycopg2-binary 2.9.9 dependency:

[19] psyco_connect: dsn = 'dbname=dd client_encoding=UTF8 user=aws_admin password=bla host=defectdojo-rds-dev port=5432', async = 0
[19] connection_setup: init connection object at 0x7f18ab9af240, async 0, refcnt = [1] connection_setup: init connection object at 0x7f18ab9af240, async 0, refcnt = 11
[19] con_connect: connecting in SYNC mode
[19] conn_connect: new PG connection at 0x7f18aaf8d4b0
[19] conn_connect: PQconnectdb(dbname=dd client_encoding=UTF8 user=aws_admin password=bla host=defectdojo-rds-dev port=5432) returned BAD
[19] connection_init: FAILED
!!! uWSGI process 19 got Segmentation Fault !!!

Debug logs with the alpine image containing the self-build psycopg2 2.9.9 dependency:

[19] psyco_connect: dsn = 'dbname=dd client_encoding=UTF8 user=aws_admin password=bla host=defectdojo-rds-dev port=5432', async = 0
[19] connection_setup: init connection object at 0x7f87c53e3240, async 0, refcnt = 1
[19] con_connect: connecting in SYNC mode
[19] conn_connect: new PG connection at 0x7f87c49248a0
[19] conn_connect: server standard_conforming_strings parameter: on
[19] conn_connect: server requires E'' quotes: NO
[19] conn_connect: using protocol 3
[19] conn_connect: client encoding: UTF8
[19] clear_encoding_name: UTF8 -> UTF8
[19] conn_set_fast_codec: encoding=UTF8
[19] conn_set_fast_codec: PyUnicode_DecodeUTF8
[19] conn_connect: DateStyle ISO, MDY
[19] connection_setup: good connection object at 0x7f87c53e3240, refcnt = 1

@github-actions github-actions bot added the docker label Jul 2, 2024
Copy link

dryrunsecurity bot commented Jul 2, 2024

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

DryRun Security Status Findings
Configured Codepaths Analyzer 0 findings
IDOR Analyzer 0 findings
Sensitive Files Analyzer 1 finding
Server-Side Request Forgery Analyzer 0 findings
SQL Injection 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 cover updates to the requirements.txt, Dockerfile.nginx-alpine, and Dockerfile.django-alpine files for the DefectDojo application. The changes primarily focus on updating dependencies, configuring the Docker build process, and improving the security and reliability of the application deployment.

The key changes include updating the PostgreSQL client version, adding new build dependencies, updating Node.js and Yarn versions, and implementing security-conscious practices such as user and permissions management, environment variable handling, and including additional fixtures and tests in the Docker image.

From an application security perspective, these changes are generally positive as they demonstrate the maintainers' commitment to keeping the application's dependencies up-to-date and addressing potential security vulnerabilities. The updates to the PostgreSQL client and the addition of new build dependencies suggest that the application is being actively maintained and improved.

Additionally, the security-conscious practices, such as user and permissions management, and the inclusion of additional fixtures and tests, indicate that the maintainers are taking steps to harden the application's deployment and ensure its overall security and reliability.

Files Changed:

  1. requirements.txt:

    • The version of the psycopg library has been updated from psycopg[binary]==3.2.1 to psycopg[c]==3.2.1, which likely changes the database driver used by the application.
  2. Dockerfile.nginx-alpine:

    • The PostgreSQL client version is updated from 14 to 16.
    • The python3-dev and libpq-dev packages are added to the list of installed packages.
    • The Node.js version is updated to 20.11.0, and the Yarn version is updated to 1.22.19.
    • Several environment variables related to NGINX configuration are included, which should be carefully reviewed for secure settings.
  3. Dockerfile.django-alpine:

    • The PostgreSQL client version is updated from 14 to 16.
    • The python3-dev and libpq-dev packages are added to the build dependencies.
    • The Dockerfile sets up a dedicated user (appuser) with a specific UID and GID, and sets the appropriate permissions and ownership for the application files and directories.
    • Several environment variables related to the application configuration, such as admin user details, Celery worker settings, and UWSGI settings, are set.
    • Additional fixtures and unit tests are copied to the Docker image.

Powered by DryRun Security

Copy link
Contributor

github-actions bot commented Jul 2, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Dockerfile.django-alpine Show resolved Hide resolved
Dockerfile.nginx-alpine Show resolved Hide resolved
@mtesauro
Copy link
Contributor

mtesauro commented Jul 2, 2024

@gietschess

Note: There's a PR to move to psycopg3 that's closer to being merged as well #10348

@gietschess
Copy link
Contributor Author

@gietschess

Note: There's a PR to move to psycopg3 that's closer to being merged as well #10348

with this pr we would still use the not recommended prebuild binary version of psycopg.

i don't really mind about the postgres clients version, just wanted to keep it up to date as alpine informs about using a old client version when installing version 14:

* Setting postgresql14 as the default version
*
* You are using 'postgresql14'. It's recommended to upgrade to the latest
* major version provided by package 'postgresql16'.
* Use command 'pg_versions' to switch between versions.
*

@gietschess gietschess changed the title fix(depy): build psycopg2 instead of using pre-build binary fix(deps): build psycopg2 instead of using pre-build binary Jul 3, 2024
@kiblik
Copy link
Contributor

kiblik commented Jul 3, 2024

Original tests have not been failing. Is there some way to improve tests to detect SegFault earlier?

@gietschess
Copy link
Contributor Author

hi @kiblik ,
i couldn't reproduce the segfault with the bitnami postgres database from the helmchart and the alpine docker image but with a external postgres database.
my best guess would be that the segfault results due to some encryption configuration of the database but i'm not completly sure about that.

@mtesauro
Copy link
Contributor

mtesauro commented Jul 3, 2024

@gietschess

Note: There's a PR to move to psycopg3 that's closer to being merged as well #10348

with this pr we would still use the not recommended prebuild binary version of psycopg.

Yes, but if we're moving to psycopg3 then maybe do a PR for the non-binary version of psychopg3 instead of the non-binary version of psycopg2. At least that's what I was thinking.

At one point in time we switched to the binary version of psycopg for a specific reason but that was quite some time ago so I don't have a problem with using the non-binary/source version of that module.

I'd prefer not to use non-binary 2 over binary 3 of that module when non-binary 3 is a valid option.

i don't really mind about the postgres clients version, just wanted to keep it up to date as alpine informs about using a old client version when installing version 14:

The one place where this can cause problems is for iron installs - we've planning on deprecating that but it's not "official' yet. I just don't want to optimize for one distro and break others if we don't absolutely have to - from a quick look at building psychopg3, only libpq5. Just checked with the most "interesting" distro (RHEL 8) and pip has no issues building psychopg3 assuming you have the needed Postgres dev packages installed.

So I'll make my comments as resolved as far as the PG 16 is concerned.

Copy link
Contributor

github-actions bot commented Jul 4, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@gietschess gietschess changed the title fix(deps): build psycopg2 instead of using pre-build binary fix(deps): build psycopg3 instead of using pre-build binary Jul 4, 2024
@gietschess
Copy link
Contributor Author

alright, i've tested with the prebuild psycopg3 binary alpine image which also leads into segmentation faults.
with the self build ones both debian and alpine looked fine.

@mtesauro
Copy link
Contributor

mtesauro commented Jul 4, 2024

alright, i've tested with the prebuild psycopg3 binary alpine image which also leads into segmentation faults.
with the self build ones both debian and alpine looked fine.

OK. Perfect. Feel free to change this PR to use the source/you-build-it psycopg3 or close it and open a new one if that's easier for you.

I'm happy to take a PR to move to a built version of psycopg3 instead of the binary version that's currently in place after #10348 was merged.

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.

Just realized you already changed to psycopg[c] from psycopg[binary]

Approved

Copy link
Contributor

github-actions bot commented Jul 5, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…ild binary

Signed-off-by: gietschess <49275246+gietschess@users.noreply.github.com>
Copy link
Contributor

github-actions bot commented Jul 8, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@mtesauro mtesauro merged commit 5730df2 into DefectDojo:dev Jul 12, 2024
125 checks passed
@hajali-amine
Copy link

Do we know when this will be released? We're facing the same issue with OVH managed PG databases 👀

@mtesauro
Copy link
Contributor

Do we know when this will be released? We're facing the same issue with OVH managed PG databases 👀

@hajali-amine This went into the dev branch so will be part of the 2.37.0 (August) release. Minor version releases happen on the 1st Monday of the month or for August 5th.

If you need it sooner than that, you can always change requirements.txt like the PR does and build your own docker images using the dockerfiles in this repo.

mwager added a commit to mwager/django-DefectDojo that referenced this pull request Jul 16, 2024
… kiuwan-sca

# By dependabot[bot] (13) and others
# Via GitHub
* 'kiuwan-sca' of github.com:mwager/django-DefectDojo: (39 commits)
  Deprecate Python-jose and migrate okta to python_social_auth (DefectDojo#10117)
  fix: dockerfile warnings (DefectDojo#10505)
  Ruff: Add and fix Q000 (DefectDojo#10095)
  Fix(django): Upgrade of 4.2 (DefectDojo#10553)
  fix(deps): build python psycopg3 dependency instead of use the pre-build binary (DefectDojo#10491)
  Bump coverage from 7.5.4 to 7.6.0 (DefectDojo#10560)
  Bump asteval from 1.0.0 to 1.0.1 (DefectDojo#10561)
  Bump djangorestframework from 3.14.0 to 3.15.2 (DefectDojo#10431)
  Bump boto3 from 1.34.142 to 1.34.143 (DefectDojo#10558)
  Bump django-debug-toolbar from 4.4.5 to 4.4.6 (DefectDojo#10557)
  Bump boto3 from 1.34.141 to 1.34.142 (DefectDojo#10551)
  Bump packageurl-python from 0.15.2 to 0.15.3 (DefectDojo#10541)
  Bump boto3 from 1.34.140 to 1.34.141 (DefectDojo#10542)
  Update helm lock file
  Update versions in application files
  Update versions in application files
  API: Convert get_filterset calls to get_queryset (DefectDojo#10543)
  Bump django-debug-toolbar from 4.4.4 to 4.4.5 (DefectDojo#10527)
  Fix ruff
  Ruff fix
  ...

# Conflicts:
#	dojo/settings/.settings.dist.py.sha256sum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants