Thread: [HACKERS] LDAPS
Hi hackers, I've run into a few requests for $SUBJECT in the field. I understand that this is a bit controversial: LDAP + StartTLS (what we already support) is better than LDAPS because it's a proper standard, and LDAP auth in general is not as good as some other authentication methods that we should be encouraging. I don't actually have a strong view on whether we should support it myself, but I took a swing at it to see if there would be any technical obstacles. I did not find any. That said, I've only tested the attached lightly on FreeBSD + OpenLDAP and don't know if it'll work elsewhere. Thoughts? -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On Sat, Nov 4, 2017 at 2:05 AM, Thomas Munro <thomas.munro@enterprisedb.com> wrote: > That > said, I've only tested the attached lightly on FreeBSD + OpenLDAP and > don't know if it'll work elsewhere. Oops, that version's TAP test was a little too dependent on my system's ldap.conf file. Here's a version that sets the LDAPCONF env var to fix that. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On Sat, Nov 4, 2017 at 2:05 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> I've only tested the attached lightly on FreeBSD + OpenLDAP and
> don't know if it'll work elsewhere.
While rebasing this on top of a nearby changes, I looked into how
portable it is. The previous version unconditionally used
ldap_initialize() instead of ldap_init() in order to be able to pass
in ldap or ldaps. According to the man pages on my system:
At this time, ldap_open() and ldap_init() are deprecated in favor of
ldap_initialize(), essentially because the latter allows to specify a
schema in the URI and it explicitly returns an error code.
But:
1. It looks like ldap_initialize() arrived in OpenLDAP 2.4 (2007),
which means that it won't work with RHEL5's OpenLDAP 2.3. That's a
vintage still found in the build farm. This new version of the patch
has a configure test so it can fall back to ldap_init(), dropping
ldaps support. That is possibly also necessary for other
implementations.
2. Windows doesn't have ldap_initialize(), but it has
ldap_sslinit()[1] which adds an SSL boolean argument. I've included
(but not tested) code for that. I would need a Windows + LDAP savvy
person to help test that. I'm not sure if it should also do an
LDAP_OPT_SSL check to see if the server forced the connection back to
plaintext as shown in the Microsoft docs[2], or if that should be
considered OK, or it should be an option.
BTW, Stephen Layland posted a patch for ldaps years ago[3]. It must
have worked some other way though, because he mentions RHEL 4 and
OpenLDAP 2.2/2.3. Unfortunately the patch wasn't attached and the
referenced webserver has disappeared from the intertubes.
I've added this to the January Commitfest.
[1] https://msdn.microsoft.com/en-us/library/aa366996(v=vs.85).aspx
[2] https://msdn.microsoft.com/en-us/library/aa366105(v=vs.85).aspx
[3] https://www.postgresql.org/message-id/20080426010240.GS5734@68k.org
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
This patch looks reasonable to me. I have also seen occasional requests for this in the field. If someone could test this on Windows, I think we could move ahead with it. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 12/26/17 15:53, Peter Eisentraut wrote: > This patch looks reasonable to me. I have also seen occasional requests > for this in the field. > > If someone could test this on Windows, I think we could move ahead with it. A small point on the test changes. You change the test under "diagnostic message", but I'm not sure why. Do the changes invalidate the existing test? We should probably also add another "note" call to introduce the LDAPS tests section. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Jan 3, 2018 at 5:31 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > On 12/26/17 15:53, Peter Eisentraut wrote: >> This patch looks reasonable to me. I have also seen occasional requests >> for this in the field. >> >> If someone could test this on Windows, I think we could move ahead with it. Thanks for looking at this. > A small point on the test changes. You change the test under > "diagnostic message", but I'm not sure why. Do the changes invalidate > the existing test? Yeah. In master, I was relying on the server rejecting ldaptls=1 requests due to lack of configured certificate in order to generate a diagnostic message. Now that there is a certificate, I needed to find another way to get requests rejected with a diagnostic message. I have added a brief note to the commit message about this. > We should probably also add another "note" call to introduce the LDAPS > tests section. I realised that I should probably also include a new test for ldaptls=1, so that we can see that both ways of doing TLS are working. I added that test, and added a "note" to label the whole section as "TLS". Please see attached. -- Thomas Munro http://www.enterprisedb.com
Attachment
On 1/2/18 14:56, Thomas Munro wrote: >> A small point on the test changes. You change the test under >> "diagnostic message", but I'm not sure why. Do the changes invalidate >> the existing test? > > Yeah. In master, I was relying on the server rejecting ldaptls=1 > requests due to lack of configured certificate in order to generate a > diagnostic message. Now that there is a certificate, I needed to find > another way to get requests rejected with a diagnostic message. I > have added a brief note to the commit message about this. > >> We should probably also add another "note" call to introduce the LDAPS >> tests section. > > I realised that I should probably also include a new test for > ldaptls=1, so that we can see that both ways of doing TLS are working. > I added that test, and added a "note" to label the whole section as > "TLS". Please see attached. Committed. I added a test case for combining LDAPS with StartTLS. The OpenLDAP library sensibly rejects that, so we don't need to do anything ourselves to prevent that. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services