#84 Improve topologies.py module
Opened 6 years ago by spichugi. Modified 6 years ago

First, we need to have an SSL enabled for our topologies. We have two options:

  1. Make it optional. And for this, I can create a function (or fix existing one) and we will have option enable it with one line.
  2. Still, create a function, but enable SSL by default in every instance created with topologies.py module

I vote for the second option because I think, it is the most frequent scenario among our customers (to have SSL enabled by default). And with it, we will have it enabled even for the cases that don't involve direct SSL operations.

And for the second point for the topologies.py improvement, we need to replace ReplicaLegacy object with a new Replicas DSLdapObjects.


I agree with this. I was about to work on the python instance creation over the duration of pycon and one of the goals was to make DS deploy with TLS OOB. I'm happy to work on this, because I think it makes tests simpler for us.

Metadata Update from @firstyear:
- Custom field Origin adjusted to None
- Custom field Review Status adjusted to None

6 years ago

Metadata Update from @firstyear:
- Issue assigned to firstyear

6 years ago

Currently, while working on #83 I am rewriting topologies.py module. I am replacing old replicas with DSLdapObjects, but the SSL enabling is still on you, if you want. :)

Yep, I'll take this on.

While developing tests with MMR SSL setup, we face an issue.
create_rsa_ca function creates a CA cert in the instance directory.
When we set up the NSS for another instance, we don't have (or I haven't found) a tool in lib389 to point the CA cert.
We need to have one CA cert, because while creating a connection we need to point OPT_X_TLS_CACERTDIR to that place.

What we can do is to create CA cert and then distribute it over all instances. Please, share your thoughts on the preferred way to implement it.
I can take the task if @firstyear wouldn't mind because it is a blocker for our test automation.

P.S. there is a small test draft (it creates CA every time, so this part we need to fix).
mmr_ssl_test.py

Ideally, you create just one CA Cert and use that to sign all the server certs for every instance. That's the easiest way, otherwise you have to import each instances' ca cert into the other instances' cert db's.

So lib389 definitely needs some work in this area. We need a way to pass in an existing CA cert to the SSL/TLS setup. We just need more tools/functions to work with certs than what exists today - although none of it should be hard.

I think we should also simplify the SSL setup in lib389. Right now you have to always run the same 4 or 5 commands. It would be nice if it had a single simple "setupTLS()" function that did everything (it could take an existing CA cert as a param, etc). We could add an option to pass in a properties dictionary to setupTLS(), etc. Just throwing out ideas

Ideally, you create just one CA Cert and use that to sign all the server certs for every instance. That's the easiest way, otherwise you have to import each instances' ca cert into the other instances' cert db's.
So lib389 definitely needs some work in this area. We need a way to pass in an existing CA cert to the SSL/TLS setup. We just need more tools/functions to work with certs than what exists today - although none of it should be hard.
I think we should also simplify the SSL setup in lib389. Right now you have to always run the same 4 or 5 commands. It would be nice if it had a single simple "setupTLS()" function that did everything (it could take an existing CA cert as a param, etc). We could add an option to pass in a properties dictionary to setupTLS(), etc. Just throwing out ideas

Thanks! I completely agree with all points. The issue was created to simplify things. :)
So I'll create a method to enable TLS (that will unite the procedure) and I will add it to the existing topology creation function.

@spichugi one question here. After reading "Still, create a function, but enable SSL by default in every instance created with topologies.py module" and "create a method to enable TLS", I am just curious to know that will there be a function to disable SSL/TLS too? As in some scenario ( like in some bug verification etc), we may need an instance without SSL setup too.
Thanks.

@amsharma Yes, I think we can do this.

@spichugi I'll implement this today, but there are some fixes in https://pagure.io/lib389/issue/95 that are needed to make it "stable, and I don't want to just duplicate them. I'm not 100% sure what I should do, maybe extract the fixes I need from 95 to this? And then make 95 just to support the new certmap? Or to just let it happen, but I @mreynolds has been delayed reviewing the certmap patch. What do you think?

@amsharma Can you give me an example of a test where we don't want TLS? I can't really think of any. And even if we install TLS you can still access the plaintext. So perhaps the path-of-least-resistance is TLS everywhere, and then you can make the test itself call ldap:// in the small cases you needed.

0002-Ticket-84-install-TLS-in-test-instances.patch

Let's start the discussion with this: At the moment, we can't merge it because we'll break EVERY test that use nss_ssl today, so we need to clean up after this, and it probably means a patch along with 389-ds-base as well.

The main point is that all tests now create an external SSLca, and that is used to sign each instance of ds. IE replication over ldaps will "just work".

We also create this for each instance - in cases where we need tests without this, we can just do "config.set('nsslapd-security', 'off')" and "instance.sslport = None" (to trick the LDAPURL function.

Let me know what you think of this, I think it solves the problem we have. It's also a nice step to "TLS by default" for new installs too, due to the way that I added this to the python installer.

Let me know what you think @spichugi and @amsharma

Saying this, the bulk of tests should be unaffected by this change, I was able to run a number of the test suites without issue or modification from dirsrvtests.

ds/dirsrvtests/tests/suites/password/pwd_root_extop.py:32:    assert(standalone.nss_ssl.create_rsa_key_and_cert() is True)
ds/dirsrvtests/tests/suites/sasl/plain.py:70:    assert(standalone.nss_ssl.reinit() is True)
ds/dirsrvtests/tests/tickets/ticket49039_test.py:35:    topo.standalone.nss_ssl.reinit()
ds/dirsrvtests/tests/tickets/ticket48784_test.py:50:    server.nss_ssl.reinit()
ds/dirsrvtests/tests/tickets/ticket47838_test.py:65:    topology_st.standalone.nss_ssl.reinit()
ds/dirsrvtests/tests/tickets/ticket48194_test.py:50:    topology_st.standalone.nss_ssl.reinit()
ds/dirsrvtests/tests/tickets/ticket48798_test.py:48:    assert (topology_st.standalone.nss_ssl._db_exists() is False)
ds/dirsrvtests/tests/tickets/ticket49303_test.py:58:    server.nss_ssl.reinit()
lib389/lib389/tests/tls_external_test.py:63:    assert(standalone.nss_ssl.reinit() is True)

These tests will need modification to support the new TLS-by-default in topologies,

@amsharma Can you give me an example of a test where we don't want TLS? I can't really think of any. And even if we install TLS you can still access the plaintext. So perhaps the path-of-least-resistance is TLS everywhere, and then you can make the test itself call ldap:// in the small cases you needed.

Well, pretty much every test that doesn't require TLS for communication, like clu or ds_tools. My concern is that keys generation for each test module will deplete the entropy pool and create a big overhead time wise.

As I've already said in IRC, great patch! Thank you!

257 +        except FileExistsError:
258 +            pass

There is no FileExistsError in Python 2. You can do something like (import errno):

except OSError as e:
    if e.errno == errno.EEXIST:
        pass
    else:
        raise

Also, could you please document the nss_ssl while you are there?
A format example: https://pagure.io/lib389/blob/master/f/lib389/aci.py#_32

Other tests that fail with the patch (and not fails without it) - 389-ds-base: 1.3.7.5-4.el7. I've left only the errors, other code removed:

ERROR at teardown of dirsrvtests/tests/suites/replication/acceptance_test.py::test_new_suffix
    >           topo_m4.ms["master{}".format(num)].backend.delete(NEW_SUFFIX)
    E         NOT_ALLOWED_ON_NONLEAF: {'desc': 'Operation not allowed on non-leaf'}

dirsrvtests/tests/suites/setup_ds/setup_ds_test.py:62:
    log.info('set SER_INST_SCRIPTS_ENABLED to {}'.format(config_attr))
    >   standalone = create_instance(config_attr)
    E         OPERATIONS_ERROR: {'info': 'nsslapd-securePort: "None" is invalid, ports must range from 1 to 65535', 'desc': 'Operations error'}

dirsrvtests/tests/tickets/ticket47536_test.py:383:
    >       create_keys_certs(topology_m2)

dirsrvtests/tests/tickets/ticket47966_test.py:79: 
    >       M2.backend.delete(DEFAULT_SUFFIX, beDn, beName)

dirsrvtests/tests/tickets/ticket48109_test.py:89: AssertionError

dirsrvtests/tests/tickets/ticket48961_test.py:137:
    >       topology_st.standalone.restart()
    E               SERVER_DOWN: {'desc': "Can't contact LDAP server"} # During DirSrv.open()

Login to comment on this ticket.

Metadata
Attachments 2