From fbd29bcf7f84f5e5ad304de8ad292db3f8b88f6c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 9 Oct 2025 20:09:57 +0200 Subject: [PATCH] fix(LDAP): properly disable are require TLS certificate verification - the old approach lead connection issues, as ldap_set_option was called too late. Specifically it needs to be called before ldap_connect and set globally! - The old approach also connected it to the ldapTLS configuration, which has a misleading naming. It indicates StartTLS usage only, not plain TLS connections. Signed-off-by: Arthur Schiwon --- .github/workflows/integration-sqlite.yml | 3 +- apps/user_ldap/lib/Connection.php | 30 ++++++++++--------- apps/user_ldap/lib/ILDAPWrapper.php | 2 +- .../ldap_features/ldap-openldap.feature | 16 ++++++++++ 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/.github/workflows/integration-sqlite.yml b/.github/workflows/integration-sqlite.yml index 87593829d62..8fbea470911 100644 --- a/.github/workflows/integration-sqlite.yml +++ b/.github/workflows/integration-sqlite.yml @@ -84,9 +84,10 @@ jobs: ports: - 6379:6379/tcp openldap: - image: ghcr.io/nextcloud/continuous-integration-openldap:openldap-7 # zizmor: ignore[unpinned-images] + image: ghcr.io/nextcloud/continuous-integration-openldap:openldap-8 # zizmor: ignore[unpinned-images] ports: - 389:389 + - 636:636 env: SLAPD_DOMAIN: nextcloud.ci SLAPD_ORGANIZATION: Nextcloud diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 336179ac341..5538b0ac6b3 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -684,6 +684,22 @@ class Connection extends LDAPUtility { return false; } + if ($this->configuration->turnOffCertCheck) { + if ($this->ldap->setOption(null, LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER)) { + $this->logger->debug( + 'Turned off SSL certificate validation successfully.', + ['app' => 'user_ldap'] + ); + } else { + $this->logger->warning( + 'Could not turn off SSL certificate validation.', + ['app' => 'user_ldap'] + ); + } + } else { + $this->ldap->setOption(null, LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_DEMAND); + } + $this->ldapConnectionRes = $this->ldap->connect($host, $port) ?: null; if ($this->ldapConnectionRes === null) { @@ -703,20 +719,6 @@ class Connection extends LDAPUtility { } if ($this->configuration->ldapTLS) { - if ($this->configuration->turnOffCertCheck) { - if ($this->ldap->setOption($this->ldapConnectionRes, LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER)) { - $this->logger->debug( - 'Turned off SSL certificate validation successfully.', - ['app' => 'user_ldap'] - ); - } else { - $this->logger->warning( - 'Could not turn off SSL certificate validation.', - ['app' => 'user_ldap'] - ); - } - } - if (!$this->ldap->startTls($this->ldapConnectionRes)) { throw new ServerNotAvailableException('Start TLS failed, when connecting to LDAP host ' . $host . '.'); } diff --git a/apps/user_ldap/lib/ILDAPWrapper.php b/apps/user_ldap/lib/ILDAPWrapper.php index de2b9c50241..793eb2b73dc 100644 --- a/apps/user_ldap/lib/ILDAPWrapper.php +++ b/apps/user_ldap/lib/ILDAPWrapper.php @@ -151,7 +151,7 @@ interface ILDAPWrapper { /** * Sets the value of the specified option to be $value - * @param \LDAP\Connection $link LDAP link resource + * @param ?\LDAP\Connection $link LDAP link resource * @param int $option a defined LDAP Server option * @param mixed $value the new value for the option * @return bool true on success, false otherwise diff --git a/build/integration/ldap_features/ldap-openldap.feature b/build/integration/ldap_features/ldap-openldap.feature index 14fa3b63968..8767367071f 100644 --- a/build/integration/ldap_features/ldap-openldap.feature +++ b/build/integration/ldap_features/ldap-openldap.feature @@ -34,6 +34,22 @@ Feature: LDAP And Sending a "GET" to "/remote.php/webdav/welcome.txt" with requesttoken Then the HTTP status code should be "200" + Scenario: Test valid configuration with LDAPS protocol and port by logging in + Given modify LDAP configuration + | ldapHost | ldaps://openldap:636 | + | turnOffCertCheck | 1 | + And cookies are reset + And Logging in using web as "alice" + And Sending a "GET" to "/remote.php/webdav/welcome.txt" with requesttoken + Then the HTTP status code should be "200" + + Scenario: Test failing LDAPS connection through TLS verification + Given modify LDAP configuration + | ldapHost | ldaps://openldap:636 | + | turnOffCertCheck | 0 | + And cookies are reset + And Expect ServerException on failed web login as "alice" + Scenario: Look for a known LDAP user Given As an "admin" And sending "GET" to "/cloud/users?search=alice"