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

tls: stop sending ssl_renegotiation_limit in startup message #476

Merged
merged 1 commit into from
Nov 3, 2018

Conversation

tejasmanohar
Copy link
Contributor

@tejasmanohar tejasmanohar commented Oct 30, 2018

This addresses #321 with the
fix @jackc proposed there--

The only reason we send ssl_renegotiation_limit is to avoid mysterious connection errors when connecting to servers that use renegotiation. But PostgreSQL defaults to 0 for all supported versions, and removed the setting for 9.6. So I guess all we need to do is remove the default startup parameter message. Anyone who really needs it can easily add it back with ConnConfig.RuntimeParams.

Redshift users that need to connect
w/ SSL currently fork the library to delete this parameter, e.g.

segmentio@8e0028d

And, that's annoying to keep up-to-date :)

@tejasmanohar
Copy link
Contributor Author

tejasmanohar commented Oct 30, 2018

As expressed in doc.go, I think this fix should go in v3.3.0. While I doubt this breaks connections for many, I suppose it could if a user of this library ignores warnings and has a non-zero value for this parameter set in postgresql.conf in Postgres < 9.6. That said, I don't feel strongly :P

@tejasmanohar tejasmanohar force-pushed the remove_tls_renegotiation_msg branch 2 times, most recently from ad95b87 to 36ab648 Compare October 30, 2018 04:45
This addresses jackc#321 with the
fix @jackc proposed there. Redshift users that need to connect
w/ SSL currently fork the library to delete this parameter, e.g.

segmentio@8e0028d

And, that's annoying to keep up-to-date :)
@jackc jackc merged commit a76c2eb into jackc:master Nov 3, 2018
@jackc
Copy link
Owner

jackc commented Nov 3, 2018

Thanks. I agree it is reasonable to make this change in a minor release. It's really hard to imagine a case where it would cause breakage.

@tejasmanohar
Copy link
Contributor Author

tejasmanohar commented Nov 27, 2018

@jackc Thanks for merging this. Would you please publish a v3.3.0 tag? Or, are you waiting on something else to do that? The docs change merged in this PR mentions this being available as of v3.30, and I'd like to pin vendor of my project that requires this fix to a release of pgx.

@jackc
Copy link
Owner

jackc commented Dec 1, 2018

Okay, I released v3.3.0.

@tejasmanohar
Copy link
Contributor Author

Appreciate it

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.

None yet

2 participants