libpq ssl -> clear fallback looses error messages
I noticed while working on general fixes for the certificate handling
that if we have a connection being attempted with sslmode=prefer (which
happens to be our default), we will loose error messages.
Basically, if we fail the SSL connection, we will throw away the error
message and try a cleartext connection. Now, if the server is configured
to require SSL (using hostssl), you will get an error message that says
"there is no pg_hba, etc, SSL off". Which is totally misleading, because
I *tried* to connect with SSL, but failed.
If I set sslmode=require, the error message is properly reported.
AFAIK we don't actually have a way to pass back an intermediate result
here, but we really need to report this error *somehow*.
It may even be to the point that if we connect and get a client side SSL
error, we should just report it and abort, and only retry if the error
is actually a server error saying there is no pg_hba for SSL here?
(or I'm missing something obvious :-P)
//Magnus
Magnus Hagander <magnus@hagander.net> writes:
I noticed while working on general fixes for the certificate handling
that if we have a connection being attempted with sslmode=prefer (which
happens to be our default), we will loose error messages.
Yeah, this came up awhile ago. I don't see any easy solution that
isn't just moving the bad cases around ... although maybe moving them
away from the default/common cases could be a good thing anyway.
Basically, if we fail the SSL connection, we will throw away the error
message and try a cleartext connection.
Maybe the answer is to not throw away the first error message? But
presenting both messages could be confusing too.
regards, tom lane
Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
I noticed while working on general fixes for the certificate handling
that if we have a connection being attempted with sslmode=prefer (which
happens to be our default), we will loose error messages.Yeah, this came up awhile ago. I don't see any easy solution that
isn't just moving the bad cases around ... although maybe moving them
away from the default/common cases could be a good thing anyway.
I think it would.
Basically, if we fail the SSL connection, we will throw away the error
message and try a cleartext connection.Maybe the answer is to not throw away the first error message? But
presenting both messages could be confusing too.
Do we have the infrastructure to report more than one error? I thought
we didn't...
//Magnus
Magnus Hagander <magnus@hagander.net> writes:
Tom Lane wrote:
Maybe the answer is to not throw away the first error message? But
presenting both messages could be confusing too.
Do we have the infrastructure to report more than one error? I thought
we didn't...
I was thinking of merging the reports into a single PGresult. The
tricky part is to figure out how to present it, and also when to throw
away one of the two reports --- if one is obviously bogus you don't want
to distract the user with it.
regards, tom lane
Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
Tom Lane wrote:
Maybe the answer is to not throw away the first error message? But
presenting both messages could be confusing too.Do we have the infrastructure to report more than one error? I thought
we didn't...I was thinking of merging the reports into a single PGresult. The
tricky part is to figure out how to present it, and also when to throw
away one of the two reports --- if one is obviously bogus you don't want
to distract the user with it.
Um, PGresult? These errors occur on connection, so it needs to go in PGconn.
I guess the easy way would be to just append the error reports with a \n
between them. I think it would be good to keep both error messages in
*all* cases. If the second connection succeeds, you will not get the
error condition anyway, and thus not look at the error message from the
first one.
//Magnus
Magnus Hagander wrote:
Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
Tom Lane wrote:
Maybe the answer is to not throw away the first error message? But
presenting both messages could be confusing too.Do we have the infrastructure to report more than one error? I thought
we didn't...I was thinking of merging the reports into a single PGresult. The
tricky part is to figure out how to present it, and also when to throw
away one of the two reports --- if one is obviously bogus you don't want
to distract the user with it.Um, PGresult? These errors occur on connection, so it needs to go in PGconn.
I guess the easy way would be to just append the error reports with a \n
between them. I think it would be good to keep both error messages in
*all* cases. If the second connection succeeds, you will not get the
error condition anyway, and thus not look at the error message from the
first one.
Here's an ugly attempt towards this. Though I'm unsure if we can change
the "const" on the PQerrorMessage parameter without messing with library
versions and such?
Another option could be to change all the calls that set the error
message to *append* to the error message instead. Thoughts on that?
//Magnus
Attachments:
libpq_err.patchtext/x-diff; name=libpq_err.patchDownload+42-19
Magnus Hagander <magnus@hagander.net> writes:
Here's an ugly attempt towards this. Though I'm unsure if we can change
the "const" on the PQerrorMessage parameter without messing with library
versions and such?
That's a bad idea in any case --- PQerrorMessage shouldn't be changing
the state of anything. The merging needs to happen earlier.
Another option could be to change all the calls that set the error
message to *append* to the error message instead. Thoughts on that?
That might work ... but we'd probably have to add some "clear the
message" calls in places. It might be worth trying to restrict this
convention to the connection-startup code.
regards, tom lane
Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
Here's an ugly attempt towards this. Though I'm unsure if we can change
the "const" on the PQerrorMessage parameter without messing with library
versions and such?That's a bad idea in any case --- PQerrorMessage shouldn't be changing
the state of anything. The merging needs to happen earlier.
That was my general thought, but since I'd written it, I just threw it
out there..
Another option could be to change all the calls that set the error
message to *append* to the error message instead. Thoughts on that?That might work ... but we'd probably have to add some "clear the
message" calls in places. It might be worth trying to restrict this
convention to the connection-startup code.
How about something like this?
//Magnus