Inconsistent error handling in the openssl init code
The errorhandling in be_tls_init(), and functions called from it, set the
appropriate elevel by the isServerStart. ssl_protocol_version_to_openssl() is
however erroring out unconditionally with ERROR on invalid TLS versions. The
attached patch adds isServerStart handling to the TLS version handling as well,
to make be_tls_init() consistent in its errorhandling.
cheers ./daniel
Attachments:
openssl_tlsver.patchapplication/octet-stream; name=openssl_tlsver.patch; x-unix-mode=0644Download+23-10
On Wed, Feb 06, 2019 at 11:18:22PM +0100, Daniel Gustafsson wrote:
The errorhandling in be_tls_init(), and functions called from it, set the
appropriate elevel by the isServerStart. ssl_protocol_version_to_openssl() is
however erroring out unconditionally with ERROR on invalid TLS versions. The
attached patch adds isServerStart handling to the TLS version handling as well,
to make be_tls_init() consistent in its errorhandling.
(Adding Peter Eisentraut in CC)
Good catch, this is an oversight from commit e73e67c7, which affects
only HEAD. The comment at the top of ssl_protocol_version_to_openssl
becomes incorrect as the function would not throw an error in a reload
context.
The current comment is that:
* If a version is passed that is not supported by the current OpenSSL
* version, then we throw an error, so that subsequent code can assume it's
* working with a supported version.
Which we could change to that:
..., then we throw an error as FATAL if isServerStart is true so as it
won't return. Otherwise, errors are logged as LOG level and return -1
to indicate trouble, preserving the old SSL state if any.
Peter, could you take care of it? Or should I?
--
Michael
On 7 Feb 2019, at 05:12, Michael Paquier <michael@paquier.xyz> wrote:
On Wed, Feb 06, 2019 at 11:18:22PM +0100, Daniel Gustafsson wrote:
The errorhandling in be_tls_init(), and functions called from it, set the
appropriate elevel by the isServerStart. ssl_protocol_version_to_openssl() is
however erroring out unconditionally with ERROR on invalid TLS versions. The
attached patch adds isServerStart handling to the TLS version handling as well,
to make be_tls_init() consistent in its errorhandling.(Adding Peter Eisentraut in CC)
Good catch, this is an oversight from commit e73e67c7, which affects
only HEAD. The comment at the top of ssl_protocol_version_to_openssl
becomes incorrect as the function would not throw an error in a reload
context.
Doh, managed to completely overlook that. The attached updated patch also
fixes the comment, thanks!
cheers ./daniel
Attachments:
openssl_tlsver-v2.patchapplication/octet-stream; name=openssl_tlsver-v2.patch; x-unix-mode=0644Download+26-12
On Thu, Feb 07, 2019 at 10:03:30AM +0100, Daniel Gustafsson wrote:
Doh, managed to completely overlook that. The attached updated patch also
fixes the comment, thanks!
That looks fine to me. Could you just add it to the next CF as a bug
fix so as we don't forget? I am pretty sure that Peter will look at
that soon.
--
Michael
On 8 Feb 2019, at 01:10, Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Feb 07, 2019 at 10:03:30AM +0100, Daniel Gustafsson wrote:
Doh, managed to completely overlook that. The attached updated patch also
fixes the comment, thanks!That looks fine to me. Could you just add it to the next CF as a bug
fix so as we don't forget? I am pretty sure that Peter will look at
that soon.
Done, thanks! I took the liberty to mark you as reviewer since you’ve already
spent time looking at the patch.
cheers ./daniel
On Fri, Feb 08, 2019 at 09:36:59AM +0100, Daniel Gustafsson wrote:
Done, thanks! I took the liberty to mark you as reviewer since you’ve already
spent time looking at the patch.
Thanks. Please note that I can take care of the patch in a couple of
days if need be.
--
Michael
On 08/02/2019 11:01, Michael Paquier wrote:
On Fri, Feb 08, 2019 at 09:36:59AM +0100, Daniel Gustafsson wrote:
Done, thanks! I took the liberty to mark you as reviewer since you’ve already
spent time looking at the patch.Thanks. Please note that I can take care of the patch in a couple of
days if need be.
Fixed, thanks.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 8 Feb 2019, at 12:01, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
On 08/02/2019 11:01, Michael Paquier wrote:
On Fri, Feb 08, 2019 at 09:36:59AM +0100, Daniel Gustafsson wrote:
Done, thanks! I took the liberty to mark you as reviewer since you’ve already
spent time looking at the patch.Thanks. Please note that I can take care of the patch in a couple of
days if need be.Fixed, thanks.
Thanks, I’ve closed it in the commitfest with you as committer.
cheers ./daniel