>
>> + 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