-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add no-verify ssl option #2198
Add no-verify ssl option #2198
Conversation
…tgres into benhjames-bhsj/noverify
case 'disable': | ||
return false | ||
case 'prefer': | ||
case 'require': | ||
case 'verify-ca': | ||
case 'verify-full': | ||
return true | ||
// no-verify is not standard to libpq but allows specifying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the intent is to follow the libpq behavior, then prefer
should also skip certificate validation, and require
only needs to validate the certificate if the root CA file is available.
Also, the checkServerIdentity
should be disabled (set to no-op) if the mode is not verify-full
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah totally...but I'm probably not gonna change these - it's a semver major to change these and they've behaved this way since...kinda forever w/ pg
. Slight inconsistencies w/ the native bindings aren't ideal, but they're less not-ideal than breaking changes. I will do a better job of documenting these options though in the docs and what they mean and how they diverge.
This adds no-verify ssl option to
connection string parsing. You can pass
?ssl=no-verify
on the pg connection string & it will properly use{ rejectUnauthorized: false }
in the ssl config passed totls.connect
. Connecting with connection strings is common in heroku, and without this patch the connection info cannot be entirely contained within the connection string for heroku environments.environment variables. added support for
PGSSLMODE=no-verify
.manually passing
new Client({ ssl: 'no-verify' })
. The 3rd option isn't very common, but it will technically work.This is a backwards compatible, semver minor change.
Thanks to @benhjames for takin' the lead on this one 😄