Skip to content

Set SNI for TSL connections - #1088

Merged
rafiss merged 1 commit into
lib:masterfrom
kelvich:sni_support
Sep 6, 2022
Merged

Set SNI for TSL connections#1088
rafiss merged 1 commit into
lib:masterfrom
kelvich:sni_support

Conversation

@kelvich

@kelvich kelvich commented Aug 20, 2022

Copy link
Copy Markdown
Contributor

This allows an SNI-aware proxy to route connections. Patch adds a new
connection option (sslsni) for opting out of the SNI, to have the same
behavior as libpq does. See more in sslsni sections at
https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-PARAMKEYWORDS.

@kelvich

kelvich commented Aug 20, 2022

Copy link
Copy Markdown
Contributor Author

Relevant discussion: #488
libpq sets SNI since v14

@cbandy cbandy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

Comment thread ssl_test.go Outdated
Comment thread ssl_test.go Outdated
@kelvich

kelvich commented Aug 29, 2022

Copy link
Copy Markdown
Contributor Author

@rafiss, any chance you can take a look? This PR is hanging approved for some amount of time.

Comment thread ssl.go Outdated

@rafiss rafiss left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

errors from linting:

Error: ssl_test.go:347:4: this value of err is never used (SA4006)
Error: ssl_test.go:348:4: this value of err is never used (SA4006)
Error: ssl_test.go:392:5: should use !bytes.Equal(startupMessage, []byte{0, 0, 0, 0x8, 0x4, 0xd2, 0x16, 0x2f}) instead (S1004)
Comment thread ssl.go Outdated
@kelvich
kelvich force-pushed the sni_support branch 2 times, most recently from 9927d34 to 0c4600d Compare August 29, 2022 20:48
@kelvich

kelvich commented Aug 29, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for the review, I've fixed mentioned issues

This allows an SNI-aware proxy to route connections. Patch adds a new
connection option (`sslsni`) for opting out of the SNI, to have the same
behavior as `libpq` does. See more in `sslsni` sections at
<https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-PARAMKEYWORDS>.
@rafiss
rafiss self-requested a review September 1, 2022 12:49

@rafiss rafiss left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks for your contribution!

@pschultz

pschultz commented Mar 23, 2023

Copy link
Copy Markdown

This seems to have broken connections with sslmode=verify-ca. This setting is supposed to ignore the DNS names in the server certificate, but now connections fail with "x509: certificate is valid for x, y, z, not a".

From what I can tell, to preserve backward compatibility tlsConf.ServerName should not be assigned if sslsni is absent and sslmode is anything other than verify-full.

@cbandy

cbandy commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

@pschultz That may be #1106.

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

Labels

None yet

6 participants