On Mon, Jan 22, 2018 at 12:08:03AM +0100, Daniel Gustafsson wrote: > The attached patchset rebases Secure Transport support over HEAD and adds stub > functions for that the SCRAM support added to make everything compile and run > the SSL testsuite. There are no new features or bugfixes over the previously > posted patches. > > Wrt SCRAM, I’m probably thick but I can’t really see what I need to do to > handle SCRAM, so I wouldn’t mind some cluesticks on that. The Secure > Transport > API doesn’t allow for getting the TLS Finished message (at least I haven’t > been > able to find a way), so channel binding can’t be supported afaict.
OK, thanks. That sucks, perhaps Apple will improve things in the future, this is the kind of areas where there is always interest. > The testcode has been updated to handle Secure Transport, but it’s not > in a clean form, rather a quick hack to get something running while the > project > settles on how to handle multiple SSL implementations. > > I have for now excluded the previous doc changes awating the discussion on the > patch in 1f34fa82-52a0-1682-87ba-4c3c3d0af...@2ndquadrant.com, once that > settles I’ll revive and write the documentation. The same goes for GUCs etc > which are discussed in other threads. > > As per before, my patch for running tests against another set of binaries is > included as well as a fix for connstrings with spaces, but with the recent > hacking by Peter I assume this is superfluous. It was handy for development > so > I’ve kept it around though. Yes that looks useful to test. be-secure-securetransport.c is quite a long name by the way, perhaps we could just live with be-secure-osx.c or be-secure-mac.c? Here are some comments about the SCRAM/channel binding part.. +be_tls_get_peer_finished(Port *port, size_t *len) +{ + ereport(ERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("channel binding is not supported by this build"))); + return NULL; Those should mention the channel binding type. In CheckSCRAMAuth(), you are missing the fact that SCRAM-SHA-256-PLUS is still published to the client. I think that this is a mistake as no channel binding types are supported. We may want to add an API for each SSL implementation to help with that as I want to keep the code paths fetching the channel binding data only invoked when necessary. So adding a new API which returns a list of channel binding types supported by a given backend would make the most sense to me. If the list is empty, then -PLUS is not published by the backend. This is not a problem related to your patch, more a problem that I need to solve as gnutls only supports tls-unique. So I'll create a new thread on the matter with a proper patch. note "setting up data directory"; -my $node = get_new_node('master'); +my $node = get_new_node('master', $ENV{SSLTEST_SERVER_BIN}); $node->init; This bit is in 0001, but this concept is introduced in 0002. (Not sure how to feel about that yet, there are similar use-cases with pg_upgrade's TAP tests where you may want to enforce a PATH.) Nit: patch has some whitespaces. You may want to run a git diff --check or such before sending. -- Michael
signature.asc
Description: PGP signature