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: do not force sslmode at infrastructure level #337

Merged
merged 3 commits into from
May 7, 2024

Conversation

ships
Copy link
Collaborator

@ships ships commented May 2, 2024

Issue(s) Resolved

Errors like SELF_SIGNED_CERT_IN_CHAIN when connecting with Pg module in Node.js to RDS instances; this was coming up when clicking Create Pub. I am not quite clear why some DB actions were working (such as referring users to the correct integration URLs).

You can now see this working in https://blake.duqduq.org/ "create pub" button.

This change includes:

  • include RDS CA certs in all docker images by default, so TLS "just works" to RDS DNS names.
  • do not set sslmode at the DATABASE_URL layer, as this stomps any configuration we set on Next.js.

With this config, TLS is "preferred" by the database and so should be used by default, but we can enforce it if we want by setting things in the creation of the pg.Pool.

@ships ships requested review from 3mcd and kalilsn May 2, 2024 22:17
@ships ships force-pushed the ships/fix/db-ssl-bugs-pgpool branch from 047b223 to 8cbfc6a Compare May 2, 2024 22:29
Copy link
Member

@kalilsn kalilsn left a comment

Choose a reason for hiding this comment

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

I'm still seeing the self-signed cert error, and I agree that it doesn't really make sense that some connections to the db would be working.

I also don't follow this exactly, would you mind elaborating?

However, that still requires this change because ?sslmode=require overrides that block of config regardless.

are you saying we'd still need to use http even if we trusted the CA?

@ships ships force-pushed the ships/fix/db-ssl-bugs-pgpool branch from e1983c9 to 5f400b9 Compare May 6, 2024 19:59
@ships
Copy link
Collaborator Author

ships commented May 7, 2024

@kalilsn updated comment now that i have included the CAs in hte docker image and HTTP is not required in any setting. To answer the rest of your question, there is a config block when you create a pg Pool where you can specify sslmode, but also Client cert and CA if you are using custom values or mutual TLS. This whole block is ignored if you specify sslmode in the URL, so it is preferable not to do that so those more specific fields are available (though we don't immediately foresee using them).

Copy link
Member

@kalilsn kalilsn left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation! And this looks good to me, assuming pg and prisma both use the system store (not the one bundled with nodejs)?

@ships ships force-pushed the ships/fix/db-ssl-bugs-pgpool branch from 89dd3be to 5418166 Compare May 7, 2024 23:32
@ships ships merged commit 7e8e7cf into main May 7, 2024
7 checks passed
@ships ships deleted the ships/fix/db-ssl-bugs-pgpool branch May 7, 2024 23:40
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.

2 participants