Nikolai, Since we last spoken, here is an update patch for review. Adding an optional ssl parameter to the DS class. When enabled, right now the tests fail because SSL is not configured properly on the client, but I'll get to that in my next patch?
Dan On Fri, Mar 11, 2016 at 1:16 PM, Nikolai Kondrashov < nikolai.kondras...@redhat.com> wrote: > Hi Dan, > > Thanks a lot for your work! Please see my comments below. > > On 03/11/2016 06:29 AM, Dan Lavu wrote: > >> Updated patch is attached, >> >> There were a few more packages I had to install to get CI running for >> Debian, should we had these to the makefile? >> >> root@sssd2:~# apt-get python-openssl >> >> dpkg -ihttp://ftp.us.debian.org/debian/pool/main/n/nss-wrapper/li >> bnss-wrapper_1.1.2-1_amd64.deb >> dpkg -ihttp://security.kali.org/pool/main/l/linux/linux-libc-dev_ >> 3.16.7-ckt20-1+deb8u4_amd64.deb >> > > I'm not sure if libnss-wrapper is in Debian Testing, but we certainly > shouldn't be downloading packages from a URL, especially non-debian > packages, > but use the repos configured on the machine. > > Below are test runs against Debian and Fedora >> > > root@sssd2:~/sssd/x86_64# cat /etc/debian_version >> 8.3 >> > > This is Debian Stable and we don't support that - we support Debian > Testing. > Ping me if you need help installing that. > > diff --git a/src/tests/intg/ca.py b/src/tests/intg/ca.py >> new file mode 100644 >> index 0000000000000000000000000000000000000000..c7adb7a2f0c2ea7f34 >> 2c9df1d6ad41dcb2fcc28e >> --- /dev/null >> +++ b/src/tests/intg/ca.py >> @@ -0,0 +1,163 @@ >> +# >> +# SSSD LOCAL domain tests >> +# >> +# Copyright (c) 2016 Red Hat, Inc. >> +# Author: Dan Lavu<d...@redhat.com> >> +# >> +# This is free software; you can redistribute it and/or modify it >> +# under the terms of the GNU General Public License as published by >> +# the Free Software Foundation; version 2 only >> +# >> +# This program is distributed in the hope that it will be useful, but >> +# WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> +# General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program. If not, see<http://www.gnu.org/licenses/>. >> +# >> + >> +from OpenSSL import crypto >> +from os.path import exists, join >> + >> +import socket >> +import os >> +import shutil >> +import config >> + >> + >> +class CA(): >> > > We're not basing CA on anything so there is no real need for parens and > they're better be removed to avoid confusion. > > + """CA Class""" >> > > This basically says what the line before it says, perhaps an expanded > explanation of the class function is in order. > > + >> + def __init__(self, subject=None, country='US', state='NC', >> + city='Raleigh', organization='Red Hat Inc.', >> unit='SSSD', config_dir=config.SSL_PATH): >> + if subject is None: >> + self.subject = socket.gethostname() >> + self.country = country >> + self.state = state >> + self.city = city >> + self.organization = organization >> + self.unit = unit >> + self.hostname = socket.gethostname() >> + if config.SSL_PATH is not '/tmp': >> + self.config_dir = config_dir >> + else: >> + self.config_dir = config_dir + '/ssl' >> > > This kind of logic can confuse the class user, and make it impossible to > actually put files into /tmp if necessary. > > I would replace this with just: > > self.config_dir = config_dir > > As a side note, it's better to not create a fixed-name files or directories > under /tmp or other world-writable directories, as the possibility of name > clashes is high there. I presume the "tempfile" module should be used > instead. > > + self.csr_dir = self.config_dir + '/newcerts' >> + self.key_dir = self.config_dir + '/private' >> + self.cert_dir = self.config_dir + '/certs' >> + >> + self.index = int(1000) >> + >> + >> + def setup(self): >> + """Setup CA using OpenSSL""" >> + cacert = self.hostname + '-ca.crt' >> + cakey = self.hostname + '-ca.key' >> + >> + if not exists(join(self.cert_dir, cacert)) or not >> exists(join(self.key_dir, cakey)): >> > > It's better to use the fully-qualified name of the "join" function here so > it's obvious this is not just some generic "join". I.e. please write > "os.path.join" instead. Same goes for "exists". > > To prevent further accidental uses like this don't import specific > functions > above, but just the module. > > Also, please put the generated paths into variables and use them instead, > here > and further. This will help against typos. > > + key = crypto.PKey() >> + key.generate_key(crypto.TYPE_RSA, 2048) >> + >> + ca = crypto.X509() >> > > crypto.X509() doesn't create a CA, but a certificate. Therefore the > variable > storing the result should be named accordingly (e.g. "cert", or "ca_cert", > not > "ca"). Plus having a "ca" variable inside a "CA" class is confusing. > > + ca.get_subject().C = self.country >> + ca.get_subject().ST = self.state >> + ca.get_subject().L = self.city >> + ca.get_subject().O = self.organization >> + ca.get_subject().OU = self.unit >> + ca.get_subject().CN = self.subject >> + ca.set_serial_number(self.index) >> + ca.gmtime_adj_notBefore(0) >> + ca.gmtime_adj_notAfter(10 * 365 * 24 * 60 * 60) >> + ca.set_issuer(ca.get_subject()) >> + ca.set_pubkey(key) >> + ca.sign(key, 'sha1') >> + >> + if not os.path.exists(self.csr_dir): >> + os.makedirs(self.csr_dir) >> + if not os.path.exists(self.key_dir): >> + os.makedirs(self.key_dir) >> + if not os.path.exists(self.cert_dir): >> + os.makedirs(self.cert_dir) >> + >> + open(os.path.join(self.cert_dir, cacert), 'wt').write \ >> + (crypto.dump_certificate(crypto.FILETYPE_PEM, ca)) >> + open(os.path.join(self.key_dir, cakey), 'wt').write \ >> + (crypto.dump_privatekey(crypto.FILETYPE_PEM, key)) >> > > Can we use "with" statement here, so that file is closed ASAP, instead of > waiting for GC? > > + >> + >> + def teardown(self): >> + """Teardown CA certificate files""" >> + if os.path.exists(self.config_dir) and not '/tmp': >> > > Hmm, this condition will always be false and the directory would never be > removed. I assume this is a piece of code trying to handle the special case > above and if that's to go then this one can simply be removed as well. > > + shutil.rmtree(self.config_dir) >> + >> + >> + def get_ca(self): >> + """Returns CA certificate in ASCII PEM encoding""" >> + ca = crypto.load_certificate(crypto.FILETYPE_PEM, open\ >> + (os.path.join(self.cert_dir, socket.gethostname() + >> '-ca.crt'), 'rt').read()) >> + ca_crt = crypto.dump_certificate(crypto.FILETYPE_PEM, ca) >> + >> + return ca_crt >> > > This function name is confusing for the same reason as the "ca" variable > above. Plus it would be better not to return the certificate as text, > unless > it's the main use. > > + def get_cert(self, csr, text=False): >> + """Retrieves certificate >> + required: csr -csr_type >> + optional: text >> + False - will return a pem object type (Default) >> + True - will return the certificate as ASCII >> + """ >> + cacert = self.hostname + '-ca.crt' >> + cakey = self.hostname + '-ca.key' >> > > Instead of generating these again, perhaps it is better to put them into > variables at initialization time? > > + ca_crt = crypto.load_certificate(crypto.FILETYPE_PEM, >> open(os.path.join(self.cert_dir, cacert), 'rt').read()) >> + ca_key = crypto.load_privatekey(crypto.FILETYPE_PEM, >> open(os.path.join(self.key_dir, cakey), 'rt').read()) >> > > Similarly with these paths. Besides, why not use the "get_ca" function here > (after renaming) and perhaps a similar one to retrieve the key? > > + cert = crypto.X509() >> + cert.set_subject(csr.get_subject()) >> + cert.set_serial_number(self.index+1) >> + cert.gmtime_adj_notBefore(0) >> + cert.gmtime_adj_notAfter(10 * 365 * 24 * 60 * 60) >> + cert.set_issuer(ca_crt.get_subject()) >> + cert.set_pubkey(csr.get_pubkey()) >> + cert.sign(ca_key, 'sha1') >> + >> + open(os.path.join(self.cert_dir, socket.gethostname() + >> '.crt'), 'wt')\ >> + .write(crypto.dump_certificate(crypto.FILETYPE_PEM, cert)) >> + server_crt = crypto.dump_certificate(crypto.FILETYPE_PEM, cert) >> + >> + if text is True: >> + return server_crt >> + else: >> + return cert >> > > It is better to keep a function doing only one thing, and i.e. not > implement > optional certificate conversion in this one. After all the caller can do it > themselves easily. So we can remove the "text" argument and not do this > conversion and branching. > > + >> + >> + def request_cert(self, fqdn=None, text=False): >> + """Generates CSR >> + optional: fqdn, socket.gethostname() (Default) >> + optional: text >> + False - will return a pem object type (Default) >> + True - will return the certificate as ASCII >> + """ >> + if fqdn is None: >> + fqdn = socket.getfqdn() >> + >> + hostname = socket.gethostname() >> + key = crypto.PKey() >> + key.generate_key(crypto.TYPE_RSA, 2048) >> + csr = crypto.X509Req() >> + csr.get_subject().CN = fqdn >> + csr.set_pubkey(key) >> + csr.sign(key, 'sha1') >> + >> + open(os.path.join(self.key_dir, hostname + '.key'), 'wt').\ >> + write(crypto.dump_privatekey(crypto.FILETYPE_PEM, key)) >> + open(os.path.join(self.csr_dir, hostname + '.csr'), 'wt').\ >> + write(crypto.dump_certificate_request(crypto.FILETYPE_PEM, >> csr)) >> > > The same notes about certificate/key retrieval here. > > + server_csr = crypto.dump_certificate_request(crypto.FILETYPE_PEM, >> csr) >> + >> + if text is True: >> + return server_csr >> + else: >> + return csr >> > > The same note about certificate conversion here. > > \ No newline at end of file >> diff --git a/src/tests/intg/config.py.m4 b/src/tests/intg/config.py.m4 >> index >> 563127c6ea895508308a4f94689cc4e26ca4cbde..141813fa7a7a5bc9c06e48dfc938885f4eb1d539 >> 100644 >> --- a/src/tests/intg/config.py.m4 >> +++ b/src/tests/intg/config.py.m4 >> @@ -11,3 +11,4 @@ PID_PATH = "pidpath" >> PIDFILE_PATH = PID_PATH + "/sssd.pid" >> LOG_PATH = "logpath" >> MCACHE_PATH = "mcpath" >> +SSL_PATH = "/tmp/ssl" >> > > config.py.m4 is for variables originating from build configuration. As > SSL_PATH is just a constant it shouldn't really be here. As such the proper > place for it could be ca.py. > > However, it is better to keep all the files tests create under the prefix > directory (to make cleanup easier and more reliable), in a place where > people > would expect them. I don't know where this specific configuration should > be, > but if it's e.g. /etc/ssl normally, then you can write in config.py.m4: > > SSL_PATH = SYSCONFDIR + "/ssl" > > > diff --git a/src/tests/intg/ds_openldap.py b/src/tests/intg/ds_openldap.py >> index >> fb230a081b58bd4e135585daa5a6ddf8f494861c..dbbbfea660d3c4e0f7c7546663cc8560522ffa37 >> 100644 >> --- a/src/tests/intg/ds_openldap.py >> +++ b/src/tests/intg/ds_openldap.py >> @@ -27,8 +27,10 @@ import errno >> import signal >> import shutil >> import sys >> +import socket >> from util import * >> from ds import DS >> +from ca import CA >> >> >> def hash_password(password): >> @@ -39,7 +41,7 @@ def hash_password(password): >> return "{SSHA}" + base64.standard_b64encode(hash.digest() + salt) >> >> >> -class DSOpenLDAP(DS): >> +class DSOpenLDAP(DS, CA): >> > > I don't think a DS should be derived from a CA. CA is a separate entity. > > """OpenLDAP directory server instance.""" >> >> def __init__(self, dir, port, base_dn, admin_rdn, admin_pw): >> @@ -60,6 +62,9 @@ class DSOpenLDAP(DS): >> self.conf_dir = self.dir + "/etc/ldap" >> self.conf_slapd_d_dir = self.conf_dir + "/slapd.d" >> self.data_dir = self.dir + "/var/lib/ldap" >> + self.ca_inst = CA() >> > > I don't think that's how inheritance is supposed to be used. Besides, it's > better to let the user create and supply a CA through an (optional) > argument > instead. Then the user can choose whether to use encryption or not. > > + >> + >> >> def _setup_config(self): >> """Setup the instance initial configuration.""" >> @@ -73,6 +78,13 @@ class DSOpenLDAP(DS): >> uid = os.geteuid() >> gid = os.getegid() >> >> + self.ca_inst.setup() >> > > If we won't be creating the CA instance ourselves, then we shouldn't be > calling "setup", the user should call it instead, so e.g. a single CA can > be > used by several LDAP servers. > > + self.ca_inst.get_cert(self.ca_inst.request_cert()) >> + fqdn = socket.gethostname() >> + cert = os.path.join(self.ca_inst.cert_dir, fqdn + '.crt') >> + ca_cert = os.path.join(self.ca_inst.cert_dir, fqdn + '-ca.crt') >> + key = os.path.join(self.ca_inst.key_dir, fqdn + '.key') >> > > Those three should really be variables in CA, the user shouldn't have to > know > how filenames inside CA are formed. Incapsulation! > > + >> # >> # Add configuration >> # >> @@ -82,6 +94,9 @@ class DSOpenLDAP(DS): >> cn: config >> olcPidFile: {self.pid_path} >> olcArgsFile: {args_file} >> + olcTLSCertificateKeyFile: {key} >> + olcTLSCertificateFile: {cert} >> + olcTLSCACertificatePath: {ca_cert} >> # Read slapd.conf(5) for possible values >> olcLogLevel: none >> >> @@ -282,3 +297,5 @@ class DSOpenLDAP(DS): >> >> for path in (self.conf_slapd_d_dir, self.run_dir, >> self.data_dir): >> shutil.rmtree(path, True) >> + >> + self.ca_inst.teardown() >> > > Similarly to "setup" note above. > > Aside from everything else above, please don't forget to run pylint on the > code before submitting! > > Nick > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/admin/lists/sssd-devel@lists. > fedorahosted.org >
diff --git src/tests/intg/ca.py src/tests/intg/ca.py new file mode 100644 index 0000000..691c030 --- /dev/null +++ src/tests/intg/ca.py @@ -0,0 +1,40 @@ +# +# Abstract directory server instance class +# +# Copyright (c) 2015 Red Hat, Inc. +# Author: Nikolai Kondrashov <nikolai.kondras...@redhat.com> +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + + +class CA: + """Abstract CA server instance""" + + def __init__(self, config_dir, subject, country, state, city, organization, unit): + """Initialize the instance""" + self.config_dir = config_dir + self.subject = subject + self.country = country + self.state = state + self.city = city + self.organization = organization + self.unit = unit + + def setup(self): + """Setup the instance""" + raise NotImplementedError() + + def teardown(self): + """Teardown the instance""" + raise NotImplementedError() diff --git src/tests/intg/ca_openssl.py src/tests/intg/ca_openssl.py new file mode 100644 index 0000000..b5c51c3 --- /dev/null +++ src/tests/intg/ca_openssl.py @@ -0,0 +1,180 @@ +# +# SSSD LOCAL domain tests +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Dan Lavu <d...@redhat.com> +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +import socket +import os.path + +from os.path import exists +from OpenSSL import crypto +from ca import CA + + +class CAOpenSSL(CA): + """OpenSSL CA instance""" + + def __init__(self, config_dir, subject, country, state, city, organization, unit): + """Initialize the instance""" + CA.__init__(self, config_dir, subject, country, state, city, organization, unit) + self.config_dir = config_dir + self.subject = subject + self.country = country + self.state = state + self.city = city + self.organization = organization + self.unit = unit + self.hostname = socket.gethostname() + + self.ca_cert_file = self.hostname + '-ca.crt' + self.ca_cert_key_file = self.hostname + '-ca.key' + + self.csr_dir = self.config_dir + '/newcerts' + self.key_dir = self.config_dir + '/private' + self.cert_dir = self.config_dir + '/certs' + + self.index = int(1000) + + def setup(self): + """Setup OpenSSL CA""" + if not exists(os.path.join(self.cert_dir, self.ca_cert_file)) or not \ + exists(os.path.join(self.key_dir, self.ca_cert_key_file)): + ca_key = crypto.PKey() + ca_key.generate_key(crypto.TYPE_RSA, 2048) + + ca_cert = crypto.X509() + ca_cert.get_subject().C = self.country + ca_cert.get_subject().ST = self.state + ca_cert.get_subject().L = self.city + ca_cert.get_subject().O = self.organization + ca_cert.get_subject().OU = self.unit + ca_cert.get_subject().CN = self.subject + ca_cert.set_serial_number(self.index) + ca_cert.gmtime_adj_notBefore(0) + ca_cert.gmtime_adj_notAfter(10 * 365 * 24 * 60 * 60) + ca_cert.set_issuer(ca_cert.get_subject()) + ca_cert.set_pubkey(ca_key) + ca_cert.sign(ca_key, 'sha1') + + if not os.path.exists(self.csr_dir): + os.makedirs(self.csr_dir) + if not os.path.exists(self.key_dir): + os.makedirs(self.key_dir) + if not os.path.exists(self.cert_dir): + os.makedirs(self.cert_dir) + + open(os.path.join(self.cert_dir, self.ca_cert_file), 'wt').\ + write(crypto.dump_certificate(crypto.FILETYPE_PEM, ca_cert)) + open(os.path.join(self.key_dir, self.ca_cert_key_file), 'wt').\ + write(crypto.dump_privatekey(crypto.FILETYPE_PEM, ca_key)) + + def teardown(self): + """Teardown""" + + def get_ca_cert(self): + """Retrieves CA certificate""" + ca_cert_file = os.path.join(self.cert_dir, self.ca_cert_file) + ca_cert = crypto.load_certificate(crypto.FILETYPE_PEM, + open(ca_cert_file, 'rt').read()) + return ca_cert + + def get_cacert_x509(self): + """Retrieves CA certificate in x509""" + ca_cert_file = os.path.join(self.cert_dir, self.ca_cert_file) + ca_cert = crypto.load_certificate(crypto.FILETYPE_PEM, + open(ca_cert_file, 'rt').read()) + ca_cert_x509 = crypto.dump_certificate(crypto.FILETYPE_PEM, ca_cert) + return ca_cert_x509 + + def get_ca_cert_path(self): + """Retrieves CA certificate file path""" + ca_cert_file = os.path.join(self.cert_dir, self.ca_cert_file) + + return ca_cert_file + + def get_ca_key(self): + """Retrieves CA certificate private key""" + ca_key_file = os.path.join(self.key_dir, self.ca_cert_key_file) + ca_key = crypto.load_privatekey(crypto.FILETYPE_PEM, + open(ca_key_file, 'rt').read()) + return ca_key + + def gen_cert(self, csr, hostname): + """Generates certificate from csr""" + ca_cert_file = os.path.join(self.cert_dir, self.ca_cert_file) + ca_key_file = os.path.join(self.key_dir, self.ca_cert_key_file) + + ca_cert = crypto.load_certificate(crypto.FILETYPE_PEM, + open(ca_cert_file, 'rt').read()) + ca_key = crypto.load_privatekey(crypto.FILETYPE_PEM, + open(ca_key_file, 'rt').read()) + + cert = crypto.X509() + cert.set_subject(csr.get_subject()) + cert.set_serial_number(self.index + 1) + cert.gmtime_adj_notBefore(0) + cert.gmtime_adj_notAfter(10 * 365 * 24 * 60 * 60) + cert.set_issuer(ca_cert.get_subject()) + cert.set_pubkey(csr.get_pubkey()) + cert.sign(ca_key, 'sha1') + + open(os.path.join(self.cert_dir, hostname + '.crt'), 'wt').\ + write(crypto.dump_certificate(crypto.FILETYPE_PEM, cert)) + return cert + + def get_cert(self, hostname): + """Retrieves server certificate in PEM""" + cert_file = os.path.join(self.cert_dir, hostname + '.crt') + cert = crypto.load_certificate(crypto.FILETYPE_PEM, open(cert_file, 'rt').read()) + return cert + + def get_cert_x509(self, hostname): + """Retrieves server certificate in x509""" + cert_file = os.path.join(self.cert_dir, hostname + '.crt') + cert_x509 = crypto.load_certificate(crypto.FILETYPE_PEM, open(cert_file, 'rt').read()) + return cert_x509 + + def get_cert_path(self, hostname): + """Retrieves server certificate file path""" + cert_file = os.path.join(self.cert_dir, hostname + '.crt') + return cert_file + + def get_cert_key(self, hostname): + """Retrieves server certificate private key""" + key_file = os.path.join(self.key_dir, hostname + '.key') + key = crypto.load_privatekey(crypto.FILETYPE_PEM, open(key_file, 'rt').read()) + return key + + def get_cert_key_path(self, hostname): + """Retrieves server certificate private key file path""" + key_file = os.path.join(self.key_dir, hostname + '.key') + return key_file + + def gen_csr(self, hostname): + """Generates certificate signing request""" + key = crypto.PKey() + key.generate_key(crypto.TYPE_RSA, 2048) + csr = crypto.X509Req() + csr.get_subject().CN = hostname + csr.set_pubkey(key) + csr.sign(key, 'sha1') + + open(os.path.join(self.key_dir, hostname + '.key'), 'a').\ + write(crypto.dump_privatekey(crypto.FILETYPE_PEM, key)) + open(os.path.join(self.csr_dir, hostname + '.csr'), 'a').\ + write(crypto.dump_certificate_request(crypto.FILETYPE_PEM, csr)) + return csr diff --git src/tests/intg/ds.py src/tests/intg/ds.py index 66cb887..1600f76 100644 --- src/tests/intg/ds.py +++ src/tests/intg/ds.py @@ -23,7 +23,7 @@ import ldap class DS: """Abstract directory server instance.""" - def __init__(self, dir, port, base_dn, admin_rdn, admin_pw): + def __init__(self, dir, port, base_dn, admin_rdn, admin_pw, ssl=None): """ Initialize the instance. @@ -37,11 +37,16 @@ class DS: """ self.dir = dir self.port = port - self.ldap_url = "ldap://localhost:"; + str(self.port) self.base_dn = base_dn self.admin_rdn = admin_rdn self.admin_dn = admin_rdn + "," + base_dn self.admin_pw = admin_pw + self.ssl = ssl + + if self.ssl: + self.ldap_url = "ldaps://localhost:" + str(self.port) + else: + self.ldap_url = "ldap://localhost:"; + str(self.port) def setup(self): """Setup the instance""" diff --git src/tests/intg/ds_openldap.py src/tests/intg/ds_openldap.py index 6a074c9..8192fa3 100644 --- src/tests/intg/ds_openldap.py +++ src/tests/intg/ds_openldap.py @@ -25,7 +25,10 @@ import os import errno import signal import shutil +import socket +import urllib import sys +import ca_openssl from util import * from ds import DS @@ -47,7 +50,7 @@ def hash_password(password): class DSOpenLDAP(DS): """OpenLDAP directory server instance.""" - def __init__(self, dir, port, base_dn, admin_rdn, admin_pw): + def __init__(self, dir, port, base_dn, admin_rdn, admin_pw, ssl=False): """ Initialize the instance. @@ -59,12 +62,13 @@ class DSOpenLDAP(DS): admin_rdn Administrator DN, relative to BASE_DN. admin_pw Administrator password. """ - DS.__init__(self, dir, port, base_dn, admin_rdn, admin_pw) + DS.__init__(self, dir, port, base_dn, admin_rdn, admin_pw, ssl) self.run_dir = self.dir + "/var/run/ldap" self.pid_path = self.run_dir + "/slapd.pid" self.conf_dir = self.dir + "/etc/ldap" self.conf_slapd_d_dir = self.conf_dir + "/slapd.d" self.data_dir = self.dir + "/var/lib/ldap" + self.ssl = ssl def _setup_config(self): """Setup the instance initial configuration.""" @@ -78,9 +82,6 @@ class DSOpenLDAP(DS): uid = os.geteuid() gid = os.getegid() - # - # Add configuration - # config = unindent(""" dn: cn=config objectClass: olcGlobal @@ -223,11 +224,23 @@ class DSOpenLDAP(DS): break except ldap.SERVER_DOWN: pass - attempt = attempt + 1 + attempt += 1 if attempt > 30: raise Exception("Failed to start slapd") time.sleep(1) + if self.ssl: + self.ca_hostname = socket.gethostname() + self.ca_inst = ca_openssl.CAOpenSSL(config.PREFIX, self.ca_hostname, + "US", + "NC", + "Raleigh", + "Red Hat Inc.", + "SSSD") + self.ca_inst.setup() + self.ca_inst.gen_cert(self.ca_inst.gen_csr(self.ca_hostname), self.ca_hostname) + self.enable_ssl(self.ca_inst.ca_cert_file, self.ca_inst.ca_cert_file, self.ca_inst.ca_cert_key_file) + # # Relax requirement of member attribute presence in groupOfNames # @@ -277,7 +290,7 @@ class DSOpenLDAP(DS): pid_file.close() attempt = 0 while os.path.isfile(self.pid_path): - attempt = attempt + 1 + attempt += 1 if attempt > 30: raise Exception("Failed to stop slapd") time.sleep(1) @@ -287,3 +300,29 @@ class DSOpenLDAP(DS): for path in (self.conf_slapd_d_dir, self.run_dir, self.data_dir): shutil.rmtree(path, True) + + if self.ssl: + self.ca_inst.teardown() + + def enable_ssl(self, ca_cert, cert, key): + """Enable SSL in OpenLDAP""" + + ldapi_socket = self.run_dir + "/ldapi" + ldapi_url = "ldapi://" + url_quote(ldapi_socket, "") + url_list = ldapi_url + " " + self.ldap_url + + modlist = [ + (ldap.MOD_ADD, "olcTLSCertificateKeyFile", [key]), + (ldap.MOD_ADD, "olcTLSCertificateFile", [cert]), + (ldap.MOD_ADD, "olcTLSCACertificatePath", [ca_cert]) + ] + ldap_conn = ldap.initialize(ldapi_url) + ldap_conn.simple_bind_s(self.admin_rdn + ",cn=config", self.admin_pw) + ldap_conn.modify_s("cn=config", modlist) + ldap_conn.unbind_s() + + if subprocess.call(['pkill', 'slapd']) != 0: + raise execfile("Failed to stop slapd") + + if subprocess.call(["slapd", "-F", self.conf_slapd_d_dir, "-h", url_list]) != 0: + raise Exception("Failed to start slapd") diff --git src/tests/intg/test_ldap.py src/tests/intg/test_ldap.py index 848cb41..ea2e05f 100644 --- src/tests/intg/test_ldap.py +++ src/tests/intg/test_ldap.py @@ -16,7 +16,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. # -import os +import os.path import stat import pwd import grp diff --git src/tests/intg/test_sssctl.py src/tests/intg/test_sssctl.py index 0df5d0b..3c2ba02 100644 --- src/tests/intg/test_sssctl.py +++ src/tests/intg/test_sssctl.py @@ -28,6 +28,7 @@ import signal import ds_openldap import ldap_ent import config +import socket from util import unindent import sssd_netgroup @@ -38,14 +39,14 @@ LDAP_BASE_DN = "dc=example,dc=com" def ds_inst(request): """LDAP server instance fixture""" ds_inst = ds_openldap.DSOpenLDAP( - config.PREFIX, 10389, LDAP_BASE_DN, - "cn=admin", "Secret123") + config.PREFIX, 10389, LDAP_BASE_DN, "cn=admin", "Secret123") try: ds_inst.setup() except: ds_inst.teardown() raise request.addfinalizer(lambda: ds_inst.teardown()) + return ds_inst
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
