Add TLS/LDAPS utilities for certificate management#246
Draft
shridhargadekar wants to merge 2 commits into
Draft
Conversation
- Add TLSUtils class in sssd_test_framework/utils/tls.py - trust_ca_certificate(): Install CA cert to system trust store - trust_ca_certificate_file(): Install CA cert from file - configure_ldap_tls(): Configure LDAP client TLS settings - disable_certificate_verification(): Disable cert verification (testing only) - Add export_root_ca_certificate() method to ADHost - Exports AD root CA certificate in PEM format - Used for LDAPS connections on port 636 - Add tls utility to Client role - Accessible via client.tls in tests This enables LDAPS testing by allowing tests to trust AD's CA certificate: ca_cert = ad.host.export_root_ca_certificate() client.tls.trust_ca_certificate(ca_cert, 'ad-root-ca') client.adcli.join(domain=domain, args=['--use-ldaps'])" Signed-off-by: shridhargadekar <shridhar.always@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new TLSUtils utility for certificate management and LDAP TLS configuration, and adds a method to the AD host class for exporting root CA certificates. Review feedback suggests that the LDAP configuration logic is destructive and should avoid overwriting the entire configuration file. Additionally, the implementation is currently limited to Red Hat-based distributions and lacks proper shell quoting for file paths, which could lead to issues with special characters.
…destructive config Addresses review feedback: 1. Add multi-distribution support: - Detect RHEL/Fedora vs Debian/Ubuntu CA trust systems - RHEL: /etc/pki/ca-trust/source/anchors + update-ca-trust - Debian: /usr/local/share/ca-certificates + update-ca-certificates - Auto-detect /etc/openldap/ldap.conf vs /etc/ldap/ldap.conf 2. Add proper shell quoting: - Import shlex module - Use shlex.quote() for all file paths - Prevents issues with spaces/special characters in certificate names 3. Make LDAP config non-destructive: - Use grep + sed to update existing TLS_REQCERT lines - Append new lines only if they don't exist - Preserves all other LDAP configuration settings - Creates config file with touch if it doesn't exist Changes: - Added _detect_ca_trust_system() helper method - Updated trust_ca_certificate() to use detected paths and shlex.quote() - Rewrote configure_ldap_tls() to use sed for in-place updates - Updated docstrings to reflect multi-distro support
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add TLSUtils class in sssd_test_framework/utils/tls.py
Add export_root_ca_certificate() method to ADHost
Add tls utility to Client role
This enables LDAPS testing by allowing tests to trust AD's CA certificate:
ca_cert = ad.host.export_root_ca_certificate()
client.tls.trust_ca_certificate(ca_cert, 'ad-root-ca')
client.adcli.join(domain=domain, args=['--use-ldaps'])"