ldaps: add LDAPS support - #264
Conversation
shridhargadekar
commented
Jul 31, 2026
- Add export_root_ca_certificate() to ADHost, SambaHost, and IPAHost
- Add CertUtils (client.cert) utility for system-level CA cert install into /etc/openldap/ldap.conf — works with adcli, realmd, ldapsearch
- Add SSSDCommonConfiguration helpers: ad_use_ldaps(), samba_use_ldaps(), ipa_set_tls_cacert() for SSSD-specific LDAPS configuration
74dde9f to
3721fa4
Compare
|
|
||
| return self.__naming_context | ||
|
|
||
| def export_root_ca_certificate(self) -> str: |
There was a problem hiding this comment.
These functions doesn't belong in hosts, it should be in roles and actually already have this method for this.
You should also add it to the generic provider because realm join works for AD, Samba, and IPA, allowing you to write a single test for multiple topologies using TopologyGroup.AnyDC or AnyAD
| truncate --size 0 /var/log/krb5kdc.log | ||
| """) | ||
|
|
||
| def export_root_ca_certificate(self) -> str: |
There was a problem hiding this comment.
Like the previous comment, this doesn't belong in hosts, it should be in roles, there is an IPACertifcateAuthority.get() which may work for the CA as well. This CertificateAuthority class should be extended.
| self.adminpw: str = self.config.get("adminpw", self.bindpw) | ||
| """Password of the admin user, defaults to value of ``bindpw``.""" | ||
|
|
||
| self.ca_cert_path: str = self.config.get("ca_cert_path", "/var/data/certs/ca.crt") |
There was a problem hiding this comment.
Same as the previous comment, but this has no CertificateAuthority class. Please make each role a separate commit.
There was a problem hiding this comment.
Still needs to move to the ad role.
| .. code-block:: python | ||
| :caption: Example usage with adcli | ||
|
|
||
| @pytest.mark.topology(KnownTopology.Samba) |
There was a problem hiding this comment.
I don't think this test scenario is really valid, since the KnownTopology.Samba is already joined when the test starts. Have you tried running this test?
|
|
||
|
|
||
| class _HasCACert(Protocol): | ||
| """Any host that can export its root CA certificate.""" |
There was a problem hiding this comment.
Moving to roles, this shouldn't be necessary.
| def install_ca( | ||
| self, | ||
| host: _HasCACert, | ||
| name: str = "test-ldap-ca.crt", |
There was a problem hiding this comment.
Nitpick, the ca.crt doesn't have to be used for ldap, I'd omit ldap from the name.
| self.sssd.fs.mkdir_p(parent) | ||
| self.sssd.fs.write(cacert_path, ca_cert) | ||
|
|
||
| def ad_use_ldaps(self, ad: AD, cacert_path: str = "/etc/sssd/pki/ad-ca.crt") -> None: |
There was a problem hiding this comment.
We already have a method to configure ldap, SSSDCommonConfiugration.ldap_provider, instead of adding three new methods, please extend this method to support ldaps.
def ldap_provider(
self,
server: str,
naming_context: str,
bind_user_dn: str,
bind_password: str,
subids: bool = False,
cacert: str = "/etc/ipa/ca.crt",
tls_reqcert: str = "demand",
ssl: bool = False,
config: dict[str, str] | None = None,
) -> None:
a94e0cc to
41ed743
Compare
spoore1
left a comment
There was a problem hiding this comment.
Just a couple of quick notes until I can test this out.
| """ | ||
| return self._password_policy | ||
|
|
||
| def export_root_ca_certificate(self) -> str: |
There was a problem hiding this comment.
This won't be needed when we re-enable the AD CA class right?
I already have an approved PR to re-enable it but, I need to first get the sssd-ci-containers dependency merged. I'm planning to do that early next week unless it's urgent to get this in place sooner?
There was a problem hiding this comment.
Once dependent PR is merged, this can modify
There was a problem hiding this comment.
Both of those PRs are now merged. You can rebase and pick up the updates to use here.
There was a problem hiding this comment.
I may have misread this initially. Is this needed because not all providers have a CA object with an export method?
41ed743 to
0cc8a13
Compare
|
Depends upon #255 |
|
|
||
| def _configure_tls_cacert(self) -> None: | ||
| """Point ``/etc/openldap/ldap.conf`` at the system CA bundle.""" | ||
| system_bundle = "/etc/pki/tls/certs/ca-bundle.crt" |
There was a problem hiding this comment.
Hi,
with Fedora 44 this changed, see https://fedoraproject.org/wiki/Changes/droppingOfCertPemFile for details.
Since you add the CA certificates to the system store, which sould be used by OpenLDAP by default, maybe the better strategy would be to make sure that all occurrences of TLS_CACERT and TLS_CACERTDIR are commented out in /etc/openldap/ldap.conf to force libldap to use the defaults?
bye,
Sumit
| :param provider: Provider role to fetch the root CA certificate from. | ||
| :type provider: GenericProvider | ||
| :param cacert_path: Path on the client where the CA cert is written, | ||
| defaults to ``/etc/sssd/pki/ca.crt`` |
There was a problem hiding this comment.
Hi,
would it be better to let use SSSD the system's CA certificate store by default and only use a dedicated file if one is provided?
bye,
Sumit
|
|
||
| Connects to ``hostname:port`` with ``openssl s_client``, captures the | ||
| certificate the server presents, writes it to the system trust anchor | ||
| directory, runs ``update-ca-trust``, and points ``TLS_CACERT`` in |
There was a problem hiding this comment.
See comments below about the system certificates and Fedora 44.
|
|
||
| def export_root_ca_certificate(self) -> str: | ||
| """ | ||
| Export the AD root CA certificate in PEM format. |
There was a problem hiding this comment.
Hi,
you can get the CA certificate directly with certutil.exe -ca.cert filename.
bye,
Sumit
There was a problem hiding this comment.
This doesn't seem to work for me (at least not in the VM we have in sssd-ci-containers):
PS C:\Users\Administrator\AppData\Local\Temp> certutil.exe -ca.cert C:\temp\ca.crt
Expected no more than 1 args, received 2
CertUtil: Too many arguments
The old PowerShell code though did appear to work better but, I'm not sure it actually exported the correct certificate. It seems to have exported the AD Certificate for CN=dc.ad.test not the CN=ad-RootCA, DC=ad, DC=test one. I'm guessing the latter is needed?
|
|
||
| from __future__ import annotations | ||
|
|
||
| import posixpath |
There was a problem hiding this comment.
Everywhere else, we just use os.path I suggest keeping it consistent.
| self.sssd.fs.mkdir_p(parent) | ||
| self.sssd.fs.write(cacert_path, ca_cert) | ||
|
|
||
| def use_ldaps(self, provider: GenericProvider, cacert_path: str = "/etc/sssd/pki/ca.crt") -> None: |
There was a problem hiding this comment.
Please extend the existing method.
There was a problem hiding this comment.
Are you referring to client.install_ca() from client.py? Or any other file.
…vider Add abstract method that each AD/IPA/Samba role must implement. Returns the root CA certificate in PEM format for use with LDAPS connections. Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
Add ca_cert_path to SambaHost (defaults to /var/data/certs/ca.crt) and implement export_root_ca_certificate() on the Samba role to read the CA certificate from that path for LDAPS connections. Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
Implement export_root_ca_certificate() on the IPA role by fetching the CA certificate via IPA's HTTP endpoint for LDAPS connections. Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
Implement export_root_ca_certificate() on the AD role by walking the cert chain from the LDAPS server certificate in LocalMachine\My to its issuer in LocalMachine\Root, falling back to the server cert for self-signed DC certs (no AD CS required). Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
0cc8a13 to
e2b0d8a
Compare
Add use_ldaps() to SSSDCommonConfiguration that fetches the root CA certificate from the provider, writes it to the client, and sets ldap_tls_cacert in the SSSD domain section. Sets ad_use_ldaps=True for AD/Samba providers. Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
e2b0d8a to
7b80a98
Compare
spoore1
left a comment
There was a problem hiding this comment.
I'm not sure the AD export method is working as expected. My testing showed the certutil fail and the old commit worked better but, I'm not sure it's pulling the expected certificate.
|
|
||
| def export_root_ca_certificate(self) -> str: | ||
| """ | ||
| Export the AD root CA certificate in PEM format. |
There was a problem hiding this comment.
This doesn't seem to work for me (at least not in the VM we have in sssd-ci-containers):
PS C:\Users\Administrator\AppData\Local\Temp> certutil.exe -ca.cert C:\temp\ca.crt
Expected no more than 1 args, received 2
CertUtil: Too many arguments
The old PowerShell code though did appear to work better but, I'm not sure it actually exported the correct certificate. It seems to have exported the AD Certificate for CN=dc.ad.test not the CN=ad-RootCA, DC=ad, DC=test one. I'm guessing the latter is needed?
| """ | ||
| return self._password_policy | ||
|
|
||
| def export_root_ca_certificate(self) -> str: |
There was a problem hiding this comment.
I may have misread this initially. Is this needed because not all providers have a CA object with an export method?
| result = self.host.conn.run( | ||
| """ | ||
| $tmp = [System.IO.Path]::GetTempFileName() + ".crt" | ||
| certutil -ca.cert $tmp | Out-Null |
There was a problem hiding this comment.
As I mentioned above, this didn't work for me when I was testing the changes. The old code in a previous commit came closer but, it exported a certificate for "cn=dc.ad.test" not the "CN=ad-RootCA, DC=ad, DC=test" Root CA certificate. The get_ca_cert() method code grabs the ad-RootCA one so maybe just use that for the AD method?
|
|
||
| return self.local.sudorule(name) | ||
|
|
||
| def install_ca( |
There was a problem hiding this comment.
Maybe name this one install_ca_cert or install_ca_certificate? My first thought with install_ca was that this was for setting up an openssl local CA.
There was a problem hiding this comment.
Hi,
about certutil, you might have to use quotes in PowerShell like e.g. certutil.exe -"ca.cert" C:\temp\ca.crt to avoid some shell magic.
HTH
bye,
Sumit
7b80a98 to
e7f0513
Compare
Add install_ca() to install a provider's root CA certificate into the system trust store and configure /etc/openldap/ldap.conf with TLS_CACERT and SASL_CBINDING tls-server-end-point for adcli/realmd/ ldapsearch LDAPS use. Add install_ca_from_server() as an alternative that fetches the CA directly from a TLS port via openssl s_client. Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
Replace the PowerShell cert-chain-walk with certutil.exe -ca.cert which reads the CA certificate directly from AD Certificate Services, converts from DER to PEM in Python. Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
e7f0513 to
effb2c3
Compare