bmarsh9 / gapps

Security compliance platform - SOC2, CMMC, ASVS, ISO27001, HIPAA, NIST CSF, NIST 800-53, CSC CIS 18, PCI DSS, SSF tracking. https://gapps.darkbanner.com
Other
414 stars 94 forks source link

Wrong parameters used for testing connection using psycopg2 #42

Closed ArjenR closed 1 year ago

ArjenR commented 1 year ago

Hi,

Normally I would take some time to test and provide a pull request. But currently no time sorry.

The issue is that I noticed the gapps-worker container failing to test the connection to the database. I disabled the container and the system works ok for just one user.

I did look for a cause and I think it is in tools/check_db_connection.py psycopg2 is used with the sqlalchemy uri, but what I have read it is psycopg version 3 (in beta I think) that can do that. psycopg2 take a dict as parameter to connect to the database.

I used these 2 discussions from stackoverflow as reference. https://stackoverflow.com/questions/15634092/connect-to-an-uri-in-postgres https://stackoverflow.com/questions/62113733/how-connect-postgresql-database-with-sqlalchemy-python

Regards, Arjen

bmarsh9 commented 1 year ago

hmm could you describe the behavior a bit more and share any error logs? The gapps-worker will check if it can query the database models before it starts. If it can't successfully query the model, it will exit (and the container will restart).

ArjenR commented 1 year ago

On the road at the moment, so I cannot provide you logs. But the gapps-worker would crash with an error about missing database and the progres container would show the error like "Database does not exist". The fact that it showed the username and not the database name is quite a tell tale

bmarsh9 commented 1 year ago

The username and database name are the same for the default set up (db1). The "crash" is really just a restart of the container. A better implementation would be a more robust health check.. but the gapps-worker should successfully start after (at worst) the 2nd restart. If this is not the case, please let me know

ArjenR commented 1 year ago

Yes, it crashes continuously. And I changed all information for the database to unique values (as a matter of principle.)

bmarsh9 commented 1 year ago

Ok then its likely a authentication issue. If you paste any error logs I can probably help debug

bmarsh9 commented 1 year ago

Also worth noting that currently (this will change soon) the gapps-worker container doesn't actually do anything.

ArjenR commented 1 year ago

Here is my test.

Clean test environment. Stock docker-compose.yml

$ cat .env 
POSTGRES_USER=pguser
POSTGRES_PASSWORD=pgpassword
POSTGRES_DB=gappsdb

$ export SETUP_DB=yes; docker-compose up

full log attached. Around line 359 the database was created gapps | [INFO] Database has been initialized.

But the issue is already clear beforehand: postgres | 2023-03-04 13:25:50.733 UTC [64] FATAL: database "pguser" does not exist gapps-worker | psycopg2.OperationalError: FATAL: database "pguser" does not exist

testfordbconnectionerror.txt

update: line number was missing

bmarsh9 commented 1 year ago

Are those env variables set on all the containers? If you want to change the default connection settings, you need to change the env variables for each container (there are 3 containers) - specifically these 4 env variables. Easiest way is just updating editing the docker-compose.yml file

SQLALCHEMY_DATABASE_URI
POSTGRES_USER
POSTGRES_PASSWORD
POSTGRES_DB
ArjenR commented 1 year ago

Hi,

Easiest way to work with environment variables like you have defined them in the docker-compose.yml is to place them in a .env file in the same place as the docker-compose.yml. The docker-compose will read them and use those. This way you can place the .env in your git ignore and then there is less chance of accidentally publishing them online / committing them in a repository.

It is also out of a security pov why I never go with the default values...

bmarsh9 commented 1 year ago

Im fully aware of what a env file is. This is part of debugging. We aren't talking about a production deployment. This is why I'm recommending to share/update the docker-compose file so I know what specific files you are working with.

bmarsh9 commented 1 year ago

If this is still an issue - please let me know. Closing for now