-
Notifications
You must be signed in to change notification settings - Fork 822
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
Allow disabling SNI on SSL Connections #5543
Comments
Interesting. Could you please share what kind of problems did you encounter (you did mention poolers misbehaving)? I'm not really opposed to adding an option to disable SNI hostname, just would like to know more which actual problems it's going to help with (and as a bonus we'll be able to guide other users to use that option in the future in case they'll also encounter it). |
In my specific case it was to do with an inhouse PG pooler implementation (can't divulge too much about it for now), which used standard Rust libraries which do NOT allow SNIs to be IP addresses at all. And we connect to it through an IP address. The connection ended with an SSL error. My case is kinda specific, but the devs of the said pooler said that they were relying on the behaviour of the considered to be conformant libpq, which doesn't send any SNI. |
There's another take to this problem: detecting that |
@vonzshik According to RFC 4366 TLS Extensions
and
Indeed, SslClientAuthenticationOptions.TargetHost property states
but if that value actually ends up in SNI Overall, I think it would be suitable to have connection string option to disable SNI similar to libpq's one |
OK. So, on one hand, we have RFC which does explicitly say that IP address shouldn't be used as a hostname. On the other, .NET documentation states that it can be both hostname and IP address (though it also says that's it's used for certificate validation, so it might be doing something else beside sending the hostname to the server?). |
Hmm, I also think it's reasonable for us to have a connection string parameter to disable sending the host, like the libpq option. However, it seems that most AuthenticateAsClient overloads accept a non-nullable Host parameter, except one which doesn't accept a Host, but also doesn't accept any parameter aside from the SslClientAuthenticationOptions. So how would one e.g. pass a custom client certificate list (or enable certificate revocation) while at the same time stop sending the Host? We may want to better understand the AuthenticateAsClient behavior and the meaning of the Host parameter.. I'd actually post this as a question on the runtime repo to see what they say... |
I actually did dive down the call stack of AuthenticateAsClient in the internals of Dotnet. All you need to do is pass an empty string (aka string.Empty) for it to not generate any SNI yet not being nullable. It is similar for Windows interop. Even stated in the comment. |
@Evengard thanks for looking into it - in that case it does sound reasonable to add the option. I'd still start a conversation in the runtime repo about possibly making the Host parameter nullable - special handling for an empty string seems like somewhat squishy/imprecise behavior to me (and we may learn a bit more about why including/not including the target host is a good idea etc.). |
Technically, the actual code seems to accept null strings for at least OpenSSL interop, although haven't checked other kinds of interop. A clarification would be good, that's for sure. |
(please link the runtime thread here when you open it, it would be good to follow) |
Hrm, after further investigation, it seems like the IP address is not sent as SNI in .NET 8.0, that's good to know (ref dotnet/runtime#89289 and linked issues in that pull request). Seems like string.Empty may have side effects (even though their own initial fix was to actually make TargetHost a string.Empty the first time). I've opened a discussion here: dotnet/runtime#97443 |
FYI converted the discussion to an issue (dotnet/runtime#97444), which I think it more appropriate (incorrect nullability annotations?) and will get more attention (discussions are generally more for the community). |
It probably would, although using a configuration string parameter seems to be better. See, we're using a wrapper around Npgsql. With a config variable, that wrapper would need no adjustments, with that callback it would need to be adjusted. It sure is better than no way of configuring it at all though! |
@NinoFloris this should do as long as it's exposed via DataSourceBuilder |
FWIW I think this is a good case for a connection string parameter, rather than an NpgsqlDataSourceBuilder config option... I think in general, environmental config should be modifiable without rebuilding the application (and this seems very environmental). |
Discussed this issue with @roji and @NinoFloris offline yesterday.
Even then, there is still a problem of a user experience. Even if that parameter was added, they would have to either:
Both options seem quite restrictive, would have to be documented (and in case of Npgsql 9, will only be released in 10 months). So instead I propose that we do the same thing that was done in dotnet/runtime#81631: we check whether the host is a valid address, and if it's not, replace with |
A backport would be appreciated, I think fixing just the part when the IP is sent as SNI is enough for a backport and general resolution of this issue, with additional SslStream customizations for npgsql 9.x+ only. |
@vonzshik a backport to Npgsql 6 and 7 is much desired, thank you. |
Sorry to ping about this, but is there a planned backported release planned with this fix? |
@Evengard we're planning for a 8.0.3 release by the end of this month, most likely we'll release the other versions at the same time. |
When connecting through SSL, Npgsql sends along the SNI hostname, yet this is not always desired. The "conformant" libpq library doesn't seem to send SNI on SSL connections at all, and some server software (poolers, etc) are not playing well when there is an IP address sent in SNI (when you connect not to a hostname, but to an IP address).
It seems like there is no way to disable the SNI, as it seems to be just set from the ConnectionString
Host
parameter.It would be appreciated, if there would be some way to disable the passing of SNI to the remote server.
Ref to this:
https://github.com/npgsql/npgsql/blob/eb050c0a0fd4b70f9d11c6d41027076e9e4e01c1/src/Npgsql/Internal/NpgsqlConnector.cs#L879C17-L883C1
Here not passing through
Host
toAuthenticateAsClient
and instead passing an empty string will effectively disable SNI.The text was updated successfully, but these errors were encountered: