Re: [HACKERS] Support for Secure Transport SSL library on macOS as OpenSSL alternative

> On 02 Mar 2018, at 03:10, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2018-01-26 23:30:08 +0100, Daniel Gustafsson wrote:
>> And another rebase and update after the refactoring in c1869542b3a4da4b12ca.
>> Also fixed some typos in comments. The other patches originally posted in this
>> patchset are either committed or made redundant.
>
> Could you provide a quick summary about where you think this patch
> stands currently? The cover letter you had for this thread was great…

Below is a summary of the current state of the patch.

Current State
=============
I’ve tried to keep this patch current with the SSL work that Peter E has been
doing, and I believe it is using the new generic infrastructure provided
wherever possible. The new SCRAM channel binding support is supported in the
sense that it isn’t supported (errors out “gracefully”) - due to Secure
Transport lacking the needed APIs.

Keychain support
================
In an earlier version this patch was not supporting Keychains for the backend
part, but I have since implemented that such that both frontend and backend can
load identities from Keychains. The common way to use Keychains in macOS apps
is to always include the user default Keychain in addition to any specific
ones. My opinion, which is what I’ve implemented, is that we (at least) in the
backend don’t want to include the user default Keychain by default, but instead
have a config setting which turns that on should the user want to. My
reasoning is that a) server applications want more fine grained control and; b)
including a Keychain we don’t control when running tests seems a like a
terrible idea. Allowing the default Keychain to be turned on with a config
knob seems a better option, so that’s what I’ve done. (this config knob was
added last week on the plane and was thus not in the last revision published, I
will post a new revision shortly but I want to read this code again first to
ensure I’m not wasting reviewer time with bugs/things I should’ve caught.)

Certificate Revocation
======================
There is no API for CRL files in Secure Transport, they are expected to be
loaded into the Keychain. The ssl_crl_file setting is erroring out with a hint
to avoid silently dropping important configuration. This is not a change from
any previously published version, but a background reminder for the next
section..

Testing
=======
Due to lack of better ideas, I’d added an ugly kludge in the tests to bypass
CRL tests on Secure Transport. I think however that I figured out how to
properly programmatically create Keychains that contain the identity and CRL
info to match the current SSL tests. I’m coding that right now, and hopefully
this means that there will be no (or few) diffs required against the tests,
only the infrastructure.

Commitfest Status
=================
Do I think this patch is realistic to target for v11? Well. Given where we
are in the cycle, I don’t think any new TLS implementation going in is
realistic at this point since none of the proposed ones have had enough tyre
kicking done. That might change should there be lots of interest and work
started soon, but as has been discussed elsewhere recently the project has
limited resources. I have time to work on this, and support reviewers of it,
but that’s only piece of the puzzle.

If no new TLS library is supported in v11, we still got cleaner SSL support out
of it due to the work performed to further remove our dependency on OpenSSL, so
we still come out on top IMO. Thanks Peter et.al!

The current revision of the patch applies cleanly on HEAD except for the tests,
but I hope to post a new version with reworked tests properly using Keychains
shortly (as in, early this week)

So, TL;DR: both frontend and backend API are implemented except for SCRAM
channel binding and CRL file support, it needs tests and documentation, it does
not implement pgcrypto, it will be fixed to support whichever GUC strategy the
project decides on for multiple TLS library support.

cheers ./daniel