ldap2pg icon indicating copy to clipboard operation
ldap2pg copied to clipboard

Do not fail if LDAP server doen't support whoami extended operation

Open dani opened this issue 2 years ago • 6 comments

Since ldap2pg 6, ldap connection is validated with whoami (LDAP extended operation 1.3.6.1.4.1.4203.1.11.3). Some servers do not support this operation (at least samba4), so ldap2pg fails

Running ldapwhoami manually gives the same error. This problem makes ldap2pg 6 unusable with samba4 (not sure about AD), while it was working with ldap2pg 5.9

Expectations

Works with LDAP server not implementing ldap extended operation 1.3.6.1.4.1.4203.1.11.3 (either not checking it at all, or not making this a critical error)

Verbose output of ldap2pg execution

Verbose output
10:41:51 INFO   Running as superuser.                            user=postgres super=true server="PostgreSQL 15.4" cluster="" database=postgres
10:41:51 DEBUG  Fallback owner configured.                       role=postgres
10:41:51 DEBUG  Inspecting roles blacklist.                     
10:41:51 DEBUG  Opening Postgres global connection.              database=postgres
10:41:51 DEBUG  Reading values from YAML.                       
10:41:51 DEBUG  Roles blacklist loaded.                          patterns="[pg_* postgres]"
10:41:51 DEBUG  Ignoring configuration file.                     path=/etc/ldap/ldap.conf err="stat /etc/ldap/ldap.conf: no such file or directory"
10:41:51 DEBUG  Ignoring configuration file.                     path=/var/lib/pgsql/ldaprc err="stat /var/lib/pgsql/ldaprc: no such file or directory"
10:41:51 DEBUG  Ignoring configuration file.                     path=/var/lib/pgsql/.ldaprc err="stat /var/lib/pgsql/.ldaprc: no such file or directory"
10:41:51 DEBUG  Ignoring configuration file.                     path=/var/lib/pgsql/ldaprc err="stat /var/lib/pgsql/ldaprc: no such file or directory"
10:41:51 DEBUG  Read LDAP option.                                key=URI value=ldaps://dc1.foo.bar:636 origin=env
10:41:51 DEBUG  Read LDAP option.                                key=TLS_REQCERT value=hard origin=env
10:41:51 DEBUG  Read LDAP option.                                key=NETWORK_TIMEOUT value=30 origin=default
10:41:51 DEBUG  LDAP dial.                                       uri=ldaps://dc1.foo.bar:636
10:41:51 DEBUG  Read LDAP option.                                key=TIMEOUT value=30 origin=default
10:41:51 DEBUG  Read LDAP option.                                key=BINDDN value="CN=Postgres,OU=Apps,DC=foo,DC=bar" origin=env
10:41:51 DEBUG  Read LDAP option.                                key=PASSWORD origin=env
10:41:51 DEBUG  LDAP simple bind.                                binddn="CN=Postgres,OU=Apps,DC=foo,DC=bar"
10:41:51 DEBUG  Running LDAP whoami.                             cmd="ldapwhoami -H ldaps://dc1.foo.bar:636 -D CN=Postgres,OU=Apps,DC=foo,DC=bar -x -w $LDAPPASSWORD"
10:41:51 DEBUG  Closing Postgres global connection.              database=postgres
10:41:51 ERROR  Fatal error.                                     err="LDAP Result Code 2 \"Protocol Error\": Extended Operation(1.3.6.1.4.1.4203.1.11.3) not supported"

dani avatar Oct 04 '23 09:10 dani

Good idea. Note that ldap2pg 5.9 did also a whoami as a connection checkup.

bersace avatar Oct 04 '23 12:10 bersace

Running 5.9 in debug, I can indeed see this

2023-10-04 14:34:20,315 DEBUG:  ldap2pg.ldap: Authenticating: ldapwhoami -x -D CN=Postgres,OU=Apps,DC=foo,DC=bar -W

But I'm not sure it was really done (or just logged), because

  • ldap2pg is working
  • manually running the ldapwhoami command fails

So maybe the return status of the whoami command wasn't checked ?

dani avatar Oct 04 '23 12:10 dani

Ok. This looks like a bug^Wfeature in ldap2pg 5 that v6 fixed. Let's add an explicit error handling there.

bersace avatar Oct 04 '23 12:10 bersace

Just to confirm, commenting the WhoAmI check like

diff --git a/internal/ldap/client.go b/internal/ldap/client.go
index db572e2..9ede7f9 100644
--- a/internal/ldap/client.go
+++ b/internal/ldap/client.go
@@ -78,12 +78,12 @@ func Connect(options OptionsMap) (client Client, err error) {
                return
        }
 
-       slog.Debug("Running LDAP whoami.", "cmd", client.Command("ldapwhoami"))
-       wai, err := client.Conn.WhoAmI(nil)
-       if err != nil {
-               return
-       }
-       slog.Info("Connected to LDAP directory.", "uri", client.URI, "authzid", wai.AuthzID)
+       //slog.Debug("Running LDAP whoami.", "cmd", client.Command("ldapwhoami"))
+       //wai, err := client.Conn.WhoAmI(nil)
+       //if err != nil {
+       //      return
+       //}
+       //slog.Info("Connected to LDAP directory.", "uri", client.URI, "authzid", wai.AuthzID)
        return
 }

And everything is working. Not sure exactly what this check should catch, because if I give wrong credentials, it still fails as expected, before starting any real work

[postgres@pg-dev ~]$ LDAPPASSWORD=foobar /tmp/ldap2pg --config /etc/ldap2pg.yml 
15:32:38 INFO   Starting ldap2pg                                 version=v6.1.0-alpha0 runtime=go1.20.5 commit=96b06904
15:32:38 WARN   Running a prerelease! Use at your own risks!    
15:32:38 INFO   Using YAML configuration file.                   path=/etc/ldap2pg.yml
15:32:38 INFO   Running as superuser.                            user=postgres super=true server="PostgreSQL 15.4" cluster="" database=postgres
15:32:38 ERROR  Fatal error.                                     err="LDAP Result Code 49 \"Invalid Credentials\": 80090308: LdapErr: DSID-0C0903A9, comment: AcceptSecurityContext error, data 52e, v1db1"
[postgres@pg-dev ~]$ 

dani avatar Oct 04 '23 13:10 dani

Just to confirm, commenting the WhoAmI check like

diff --git a/internal/ldap/client.go b/internal/ldap/client.go
index db572e2..9ede7f9 100644
--- a/internal/ldap/client.go
+++ b/internal/ldap/client.go
@@ -78,12 +78,12 @@ func Connect(options OptionsMap) (client Client, err error) {
                return
        }
 
-       slog.Debug("Running LDAP whoami.", "cmd", client.Command("ldapwhoami"))
-       wai, err := client.Conn.WhoAmI(nil)
-       if err != nil {
-               return
-       }
-       slog.Info("Connected to LDAP directory.", "uri", client.URI, "authzid", wai.AuthzID)
+       //slog.Debug("Running LDAP whoami.", "cmd", client.Command("ldapwhoami"))
+       //wai, err := client.Conn.WhoAmI(nil)
+       //if err != nil {
+       //      return
+       //}
+       //slog.Info("Connected to LDAP directory.", "uri", client.URI, "authzid", wai.AuthzID)
        return
 }

And everything is working. Not sure exactly what this check should catch, because if I give wrong credentials, it still fails as expected, before starting any real work

[postgres@pg-dev ~]$ LDAPPASSWORD=foobar /tmp/ldap2pg --config /etc/ldap2pg.yml 
15:32:38 INFO   Starting ldap2pg                                 version=v6.1.0-alpha0 runtime=go1.20.5 commit=96b06904
15:32:38 WARN   Running a prerelease! Use at your own risks!    
15:32:38 INFO   Using YAML configuration file.                   path=/etc/ldap2pg.yml
15:32:38 INFO   Running as superuser.                            user=postgres super=true server="PostgreSQL 15.4" cluster="" database=postgres
15:32:38 ERROR  Fatal error.                                     err="LDAP Result Code 49 \"Invalid Credentials\": 80090308: LdapErr: DSID-0C0903A9, comment: AcceptSecurityContext error, data 52e, v1db1"
[postgres@pg-dev ~]$ 

err is a named return variable. Thus return without setting err = nil effectively propagate the error to the caller. That's implicit error handling. This code should be more explicit.

bersace avatar Oct 04 '23 15:10 bersace

The good news for those running samba and hitting this issue : the missing extended operation is supported starting with samba 4.19 (not mentionned in the changelog, but just upgraded to test and ldap2pg works now, without my ugly patch)

dani avatar Oct 05 '23 13:10 dani