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

> On 03 Aug 2017, at 13:06, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> On 08/03/2017 01:02 PM, Daniel Gustafsson wrote:
>>
>> The frontend has support for using PEM files for certificates and keys. It can
>> also look up the key for the certificate in a Keychain, or both certificate and
>> key in a Keychain. The root certificate is still just read from a PEM file.
>
> Why can't you have the root certificate in the keychain, too? Just not implemented yet, or is there some fundamental problem with it?

Just not implemented yet, awaiting Keychain discussions.

>> The existence of an sslcert config trumps a keychain, but when a keychain is
>> used I’m currently using the sslcert parameter (awaiting a discussion on how to
>> properly do this) for the certificate label required to search the keychain.
>> There is a new frontend parameter called “keychain” with which a path to a
>> custom Keychain file can be passed. If set, this Keychain will be searched as
>> well as the default. If not, only the default user Keychain is used. There is
>> nothing that modifies the Keychain in this patch, it can read identities
>> (certificates and its key) but not alter them in any way.
>
> OpenSSL also has a mechanism somewhat similar to the keychain, called "engines". You can e.g. keep the private key corresponding a certificate on a smart card, and speak to it with an OpenSSL "smart card reader" engine. If you do that, the 'sslkey' syntax is "<engine name>:<key name>". Perhaps we should adopt that syntax here as well? For example, to read the client certificate from the key chain, you would use libpq options like "keychain=/home/heikki/my_keychain sslcert=keychain:my_client_cert”.

Thats definitely an option, although it carries a bit redudancy in this case
which can confuse users. With “keychain=/foo/bar.keychain sslcert=my_cert”,
did the user mean a file called my_cert or is it a reference to a cert in the
keychain? Nothing that strict parsing rules, good errormessages and
documentation can’t solve but needs careful thinking.

>> “keychain” is obviously a very Secure Transport specific name, and I personally
>> think we should avoid that. Any new configuration added here should take
>> future potential implementation into consideration such that avoid the need for
>> lots of backend specific knobs. “sslcertstore” comes to mind as an
>> alternative, but we’d also need parameters to point into the certstore for
>> finding what we need. Another option would be to do a URL based scheme
>> perhaps.
>
> I wouldn't actually mind using implementation-specific terms like "keychain" here. It makes it clear that it's implementation-specific. I think it would be confusing, to use the same generic option name, like "sslcertstore", for both a macOS keychain and e.g. the private key store in Windows. Or GNU Keyring. In the worst case, you might even have multiple such "key stores" on the same system, so you'd anyways need a way to specify which one you mean.

Thats a good point.

> Actually, perhaps it should be made even more explicit, and call it "secure_transport_keychain". That's quite long, though.

Yeah, secure_transport_ is a pretty long prefix.

> Wrt. keychains, is there a system-global or per-user keychain in macOS? And does this patch use them? If you load a CRL into a global keychain, does it take effect?

Each user has a default Keychain , and on top of that you can create as many
Keychains as you want (a Keychain is really just a database file containing
secret things). The frontend use the default one for lookups unless the
keychain parameter is set in which case it uses both.

When evaluating trust, Secure Transport will use the default Keychain even if
not explicitly opened (as in the backend code). It does however only search
for intermediate certificates and not root certs/CRLs. Adding a policy object
for using CRLs should force it to afaik, but we’d need to support additional
keychains (if only to be able to test without polluting the default).

>> Testing
>> =======
>> In order to test this we need to provide an alternative to the openssl calls in
>> src/test/ssl/Makefile for Secure Transport.
>
> Those openssl commands are only needed to re-generate the test certificates. The certificates are included in the git repository, so you only need to re-generate them if you want to modify them or add new ones. I think it's OK to require the openssl tool for that, it won't be needed just to run the test suite.

Ah, of course. We still need support for re-generating Keychains (or at all
generate them in case we don’t want binaries in the repository).

>> Documentation
>> =============
>> I have started fiddling with this a little, but to avoid spending time on the
>> wrong thing I have done very little awaiting the outcome of discussions here.
>> I have tried to add lots of comments to the code however, to explain the quirks
>> of Secure Transport.
>
> I think this patch in general is in very good shape, and the next step is to write the documentation. In particular, I'd like to see documentation on how the keychain stuff should work. It'll be easier to discuss the behavior and the interactions, once it's documented.

I’ll try to polish off the patch I have documenting the current behavior.

> In fact, better to write the documentation for that now, and not bother to change the code, until after we've discussed and are happy with the documented behavior.

Yeah, discussion -> documentation -> code was my plan.

cheers ./daniel