Skip to content

tls: stop sending ssl_renegotiation_limit in startup message#476

Merged
jackc merged 1 commit into
jackc:masterfrom
tejasmanohar:remove_tls_renegotiation_msg
Nov 3, 2018
Merged

tls: stop sending ssl_renegotiation_limit in startup message#476
jackc merged 1 commit into
jackc:masterfrom
tejasmanohar:remove_tls_renegotiation_msg

Conversation

@tejasmanohar

@tejasmanohar tejasmanohar commented Oct 30, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

tejasmanohar commented Oct 30, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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 mentioned this pull request Oct 30, 2018
Closed
@tejasmanohar
tejasmanohar force-pushed the remove_tls_renegotiation_msg branch 2 times, most recently from ad95b87 to 36ab648 Compare October 30, 2018 04:45
@tejasmanohar
tls: stop sending ssl_renegotiation_limit in startup message
e2010c8 
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 :)
@tejasmanohar
tejasmanohar force-pushed the remove_tls_renegotiation_msg branch from 36ab648 to e2010c8 Compare October 30, 2018 04:50
@jackc
jackc merged commit a76c2eb into jackc:master Nov 3, 2018
@jackc

jackc commented Nov 3, 2018

Copy link
Copy Markdown
Owner

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

tejasmanohar commented Nov 27, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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

jackc commented Dec 1, 2018

Copy link
Copy Markdown
Owner

Okay, I released v3.3.0.

@tejasmanohar

tejasmanohar commented Dec 2, 2018

Copy link
Copy Markdown
Contributor Author

Appreciate it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tejasmanohar @jackc