Fix postgresql hostaddr semantics #4102
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
mbj force-pushed the fix/hostaddr-semantics branch 3 times, most recently from 743da8b to df9a233 Compare 
mbj force-pushed the fix/hostaddr-semantics branch 3 times, most recently from 4dcf7d1 to 96a356c Compare * Since 3.x extensions in the CSR are not automatically carried * Needed as rustls does not use the CN to verify hostnames it uses subject alternative DNS names only.
mbj force-pushed the fix/hostaddr-semantics branch from 96a356c to 207c342 Compare 
mbj commented Nov 12, 2025
| 25iUuef8MB8GA1UdIwQYMBaAFAsNqVaRis9sZCI21R2IdC583ZFuMAUGAytlcANB | ||
| AKyosmZvuCIrWkvb4QN8k2Fwf09LICCNjh571XwNxp9eUEEwJOjl956o6SFxDlgK | ||
| Cr1llASvz5cPm6jUV2wlaQc= | ||
| -----END CERTIFICATE----- |
Contributor Author
There was a problem hiding this comment.
Before:
Certificate: Data: Version: 3 (0x2) Serial Number: b6:c7:1f:4b:11:c0:a1:8f Signature Algorithm: ED25519 Issuer: C=us, ST=california, O=Internet Widgits Pty Ltd, CN=Austin Bonander Validity Not Before: Jul 1 05:21:56 2025 GMT Not After : Jun 29 05:21:56 2035 GMT Subject: C=us, ST=california, O=SQLx.rs, CN=sqlx.rs Subject Public Key Info: Public Key Algorithm: ED25519 ED25519 Public-Key: pub: 0d:f7:4b:6a:ac:aa:96:6c:b1:47:18:ae:99:5e:31: 70:e3:e7:f9:a1:6f:9e:87:3d:a3:b7:08:82:20:63: 97:3c X509v3 extensions: X509v3 Subject Key Identifier: 3D:4A:67:F7:91:87:16:E3:1E:EF:ED:A9:1B:9A:DB:98:94:B9:E7:FC X509v3 Authority Key Identifier: 0B:0D:A9:56:91:8A:CF:6C:64:22:36:D5:1D:88:74:2E:7C:DD:91:6E Signature Algorithm: ED25519 Signature Value: 13:11:0e:cd:af:48:ad:27:21:a1:0b:35:37:03:f1:7c:27:5f: 5c:c1:e2:5e:9b:20:31:38:43:ba:28:3e:f6:16:ae:5b:e6:f5: 2a:50:51:dd:97:96:f0:03:88:c1:ac:cc:85:db:2d:04:53:a1: ba:66:96:3d:0f:f0:64:d5:24:0a After:
$ openssl x509 -text -noout -in server.crt Certificate: Data: Version: 3 (0x2) Serial Number: 2c:da:80:96:d8:8e:10:fe:5f:79:0a:e3:20:15:d8:03:a6:02:d1:f5 Signature Algorithm: ED25519 Issuer: C=us, ST=california, O=Internet Widgits Pty Ltd, CN=Austin Bonander Validity Not Before: Nov 12 21:49:58 2025 GMT Not After : Nov 10 21:49:58 2035 GMT Subject: C=us, ST=california, O=SQLx.rs, CN=sqlx.rs Subject Public Key Info: Public Key Algorithm: ED25519 ED25519 Public-Key: pub: 0d:f7:4b:6a:ac:aa:96:6c:b1:47:18:ae:99:5e:31: 70:e3:e7:f9:a1:6f:9e:87:3d:a3:b7:08:82:20:63: 97:3c X509v3 extensions: X509v3 Subject Alternative Name: DNS:sqlx.rs X509v3 Subject Key Identifier: 3D:4A:67:F7:91:87:16:E3:1E:EF:ED:A9:1B:9A:DB:98:94:B9:E7:FC X509v3 Authority Key Identifier: 0B:0D:A9:56:91:8A:CF:6C:64:22:36:D5:1D:88:74:2E:7C:DD:91:6E Signature Algorithm: ED25519 Signature Value: ac:a8:b2:66:6f:b8:22:2b:5a:4b:db:e1:03:7c:93:61:70:7f: 4f:4b:20:20:8d:8e:1e:7b:d5:7c:0d:c6:9f:5e:50:41:30:24: e8:e5:f7:9e:a8:e9:21:71:0e:58:0a:0a:bd:65:94:04:af:cf: 97:0f:9b:a8:d4:57:6c:25:69:07 Note the subject alternative name is now present (was absent before)!
mbj commented Nov 12, 2025
| | ||
| ``` | ||
| openssl x509 -req -CA ca.crt -CAkey keys/ca.key -in server.csr -out server.crt -days 3650 -CAcreateserial | ||
| openssl x509 -req -CA ca.crt -CAkey keys/ca.key -in server.csr -out server.crt -days 3650 -CAcreateserial -copy_extensions copy |
Contributor Author
There was a problem hiding this comment.
OT: I consider it a terrible design descision to silently truncate extensions, instead they should have bailed "unacknowledged extension A, B, C present".
Contributor Author
There was a problem hiding this comment.
context: rustls does not use the CN field to populate present hostnames for hostname checks. Instead it exclusively looks at the subject alternative DNS names. All modern CAs always emit subject alternative names, so we need to properly create one to allow verify-full in tests (followup commit does so after fixing the hostaddr bug).
mbj force-pushed the fix/hostaddr-semantics branch from 1d469ca to b3c90e1 Compare mbj commented Nov 12, 2025
| .unwrap_or_else(|| default_host(port)); | ||
| let host = var("PGHOST").ok().unwrap_or_else(|| default_host(port)); | ||
| | ||
| let host_addr = var("PGHOSTADDR").ok(); |
Contributor Author
There was a problem hiding this comment.
Now not all strings are valid here, only numeric IPs are. But we are not doing any validation on the strings at all for all the sibling fields, so I did not start to do that here also. In fact there is not even an error path in this API.
That I do not agree to the defaulting to ENVs in the first place was discussed in previous PRs, for the moment I consider the bugfix more important than improving tghe API so I just keep it "as wrong in my eyes as the siblings" while fixing host != hostaddr.
mbj commented Nov 12, 2025
| None => net::connect_tcp(&options.host, options.port, MaybeUpgradeTls(options)).await?, | ||
| None => { | ||
| net::connect_tcp( | ||
| options.host_addr.as_ref().unwrap_or(&options.host), |
Contributor Author
There was a problem hiding this comment.
I've modified the integration tests to cover this behavior, they now:
- instruct the ssl tests to connect to a host:
sqlx.rsthat does not resolve to a postgresql instance on the public DNS. - but provide the
hostaddrto point it to127.0.0.1bypassing DNS lookups - proof that cert validation fully works in this scenario in upgrading to
verify-fullfromverify-ca.
I use sqlx.rs as hostname for this test as this is the hostname present in the test certificates.
* Before hostaddr was used to just be an alternative way to set `host`. But this is not how libpq actually behaves or intents hostaddr to be used. * Instead `hostaddr` is meant to contain a numeric IP address to connect to, bypassing DNS lookups. This can be useful if the DNS lookups are slow, or if the DNS lookup has to be bypassed as the port is proxied to localhost for ifrastructure reasons. The TLS verification is still always done against the `host` parameter. Basically it allows to do `verify-full` on proxied ports.
mbj force-pushed the fix/hostaddr-semantics branch from b3c90e1 to f795fe9 Compare Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
host. But this is not how libpq actually behaves or intents hostaddr to be used.hostaddris meant to contain a numeric IP address to connect to, bypassing DNS lookups. This can be useful if the DNS lookups are slow, or if the DNS lookup has to be bypassed as the port is proxied to localhost for ifrastructure reasons. The TLS verification is still always done against thehostparameter. Basically it allows to doverify-fullon proxied ports.Libpq docs:
https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-PARAMKEYWORDS
Does your PR solve an issue?
Yes but issue was not reported yet, the original implementation of
hostaddrparameters only misleading/bad semantics.Is this a breaking change?
Can be argued both ways. Its definitively a bugfix, but could be that the bug is loadbearing. I'm fine either way, going to use a fork till release.