From ea033b6baa9bdd58a0cc667bf5d4512b73e914d4 Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: Apr 18 2018 14:42:38 +0000 Subject: Issue 49109 - nsDS5ReplicaTransportInfo should accept StartTLS as an option Bug Description: nsDS5ReplicaTransportInfo SSL vs TLS is not really clear, given that most libraries now support TLS as the default "SSL". We should make this clear in nsDS5ReplicaTransportInfo by allowing: ldaps -> SSL StartTLS -> TLS options. So that it's really clear what you are asking for when you configure it. Fix Description: Add additional options for nsDS5ReplicaTransportInfo - LDAPS and StartTLS. Legacy options will stay for some time and will deprecated in later version. Also, change the DNA plugin values in the same way. And rename replica flags through the code base as followed: TRANSPORT_FLAG_SSL -> TRANSPORT_FLAG_LDAPS TRANSPORT_FLAG_TLS -> TRANSPORT_FLAG_STARTTLS Also, check the change in the existing TLS replication test suite. https://pagure.io/389-ds-base/issue/49109 Reviewed by: wibrown, mreynolds (Thanks!) --- diff --git a/dirsrvtests/tests/suites/replication/tls_client_auth_repl_test.py b/dirsrvtests/tests/suites/replication/tls_client_auth_repl_test.py index 07a9948..34a0b3a 100644 --- a/dirsrvtests/tests/suites/replication/tls_client_auth_repl_test.py +++ b/dirsrvtests/tests/suites/replication/tls_client_auth_repl_test.py @@ -33,6 +33,11 @@ def tls_client_auth(topo_m2): m1 = topo_m2.ms['master1'] m2 = topo_m2.ms['master2'] + if ds_is_older('1.4.0.6'): + transport = 'SSL' + else: + transport = 'LDAPS' + # Create the certmap before we restart for enable_tls cm_m1 = CertmapLegacy(m1) cm_m2 = CertmapLegacy(m2) @@ -65,8 +70,8 @@ def tls_client_auth(topo_m2): agmt_m1.replace_many( ('nsDS5ReplicaBindMethod', 'SSLCLIENTAUTH'), - ('nsDS5ReplicaTransportInfo', 'SSL'), - ('nsDS5ReplicaPort', '%s' % m2.sslport), + ('nsDS5ReplicaTransportInfo', transport), + ('nsDS5ReplicaPort', str(m2.sslport)), ) agmt_m1.remove_all('nsDS5ReplicaBindDN') @@ -75,8 +80,8 @@ def tls_client_auth(topo_m2): agmt_m2.replace_many( ('nsDS5ReplicaBindMethod', 'SSLCLIENTAUTH'), - ('nsDS5ReplicaTransportInfo', 'SSL'), - ('nsDS5ReplicaPort', '%s' % m1.sslport), + ('nsDS5ReplicaTransportInfo', transport), + ('nsDS5ReplicaPort', str(m1.sslport)), ) agmt_m2.remove_all('nsDS5ReplicaBindDN') @@ -85,6 +90,56 @@ def tls_client_auth(topo_m2): return topo_m2 +def test_ssl_transport(tls_client_auth): + """Test different combinations for nsDS5ReplicaTransportInfo values + + :id: 922d16f8-662a-4915-a39e-0aecd7c8e6e2 + :setup: Two master replication, enabled TLS client auth + :steps: + 1. Set nsDS5ReplicaTransportInfoCheck: SSL or StartTLS or TLS + 2. Restart the instance + 3. Check that replication works + 4. Set nsDS5ReplicaTransportInfoCheck: LDAPS back + :expectedresults: + 1. Success + 2. Success + 3. Replication works + 4. Success + """ + + m1 = tls_client_auth.ms['master1'] + m2 = tls_client_auth.ms['master2'] + repl = ReplicationManager(DEFAULT_SUFFIX) + replica_m1 = Replicas(m1).get(DEFAULT_SUFFIX) + replica_m2 = Replicas(m2).get(DEFAULT_SUFFIX) + agmt_m1 = replica_m1.get_agreements().list()[0] + agmt_m2 = replica_m2.get_agreements().list()[0] + + if ds_is_older('1.4.0.6'): + check_list = (('TLS', False),) + else: + check_list = (('SSL', True), ('StartTLS', False), ('TLS', False)) + + for transport, secure_port in check_list: + agmt_m1.replace_many(('nsDS5ReplicaTransportInfo', transport), + ('nsDS5ReplicaPort', '{}'.format(m2.port if not secure_port else m2.sslport))) + agmt_m2.replace_many(('nsDS5ReplicaTransportInfo', transport), + ('nsDS5ReplicaPort', '{}'.format(m1.port if not secure_port else m1.sslport))) + repl.test_replication_topology(tls_client_auth) + + if ds_is_older('1.4.0.6'): + agmt_m1.replace_many(('nsDS5ReplicaTransportInfo', 'SSL'), + ('nsDS5ReplicaPort', str(m2.sslport))) + agmt_m2.replace_many(('nsDS5ReplicaTransportInfo', 'SSL'), + ('nsDS5ReplicaPort', str(m1.sslport))) + else: + agmt_m1.replace_many(('nsDS5ReplicaTransportInfo', 'LDAPS'), + ('nsDS5ReplicaPort', str(m2.sslport))) + agmt_m2.replace_many(('nsDS5ReplicaTransportInfo', 'LDAPS'), + ('nsDS5ReplicaPort', str(m1.sslport))) + repl.test_replication_topology(tls_client_auth) + + def test_extract_pemfiles(tls_client_auth): """Test TLS client authentication between two masters operates as expected with 'on' and 'off' options of nsslapd-extract-pemfiles diff --git a/ldap/servers/plugins/dna/dna.c b/ldap/servers/plugins/dna/dna.c index 9041788..b9a6f4c 100644 --- a/ldap/servers/plugins/dna/dna.c +++ b/ldap/servers/plugins/dna/dna.c @@ -74,6 +74,8 @@ #define DNA_PROT_LDAP "LDAP" #define DNA_PROT_TLS "TLS" #define DNA_PROT_SSL "SSL" +#define DNA_PROT_LDAPS "LDAPS" +#define DNA_PROT_STARTTLS "StartTLS" /* For transferred ranges */ #define DNA_NEXT_RANGE "dnaNextRange" @@ -1901,8 +1903,10 @@ dna_get_shared_servers(struct configEntry *config_entry, PRCList **servers, int } if (strcasecmp(server->remote_bind_method, DNA_METHOD_SSL) == 0) { /* requires a bind DN */ - if (strcasecmp(server->remote_conn_prot, DNA_PROT_SSL) != 0 && - strcasecmp(server->remote_conn_prot, DNA_PROT_TLS) != 0) { + if ((strcasecmp(server->remote_conn_prot, DNA_PROT_SSL) != 0 || + strcasecmp(server->remote_conn_prot, DNA_PROT_LDAPS) != 0) && + (strcasecmp(server->remote_conn_prot, DNA_PROT_TLS) != 0 || + strcasecmp(server->remote_conn_prot, DNA_PROT_STARTTLS) != 0)) { reason = "bind method (SSL) requires either SSL or TLS connection " "protocol."; err = 1; @@ -3065,9 +3069,9 @@ dna_get_replica_bind_creds(char *range_dn, struct dnaServer *server, char **bind *port = slapi_entry_attr_get_int(entries[0], DNA_REPL_PORT); /* Check if we should use SSL */ - if (transport && (strcasecmp(transport, "SSL") == 0)) { + if (transport && (strcasecmp(transport, DNA_PROT_SSL) == 0 || strcasecmp(transport, DNA_PROT_LDAPS) == 0)) { *is_ssl = 1; - } else if (transport && (strcasecmp(transport, "TLS") == 0)) { + } else if (transport && (strcasecmp(transport, DNA_PROT_TLS) == 0 || strcasecmp(transport, DNA_PROT_STARTTLS) == 0)) { *is_ssl = 2; } else { *is_ssl = 0; @@ -3156,9 +3160,11 @@ dna_get_remote_config_info(struct dnaServer *server, char **bind_dn, char **bind ; /* just use it directly */ } - if (server->remote_conn_prot && strcasecmp(server->remote_conn_prot, DNA_PROT_SSL) == 0) { + if (server->remote_conn_prot && (strcasecmp(server->remote_conn_prot, DNA_PROT_SSL) == 0 || + strcasecmp(server->remote_conn_prot, DNA_PROT_LDAPS) == 0 )) { *is_ssl = 1; - } else if (server->remote_conn_prot && strcasecmp(server->remote_conn_prot, DNA_PROT_TLS) == 0) { + } else if (server->remote_conn_prot && (strcasecmp(server->remote_conn_prot, DNA_PROT_TLS) == 0 || + strcasecmp(server->remote_conn_prot, DNA_PROT_STARTTLS) == 0 )) { *is_ssl = 2; } else { *is_ssl = 0; diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h index e08fec7..d271b87 100644 --- a/ldap/servers/plugins/replication/repl5.h +++ b/ldap/servers/plugins/replication/repl5.h @@ -360,8 +360,8 @@ typedef struct repl_bos Repl_Bos; /* In repl5_agmt.c */ typedef struct repl5agmt Repl_Agmt; -#define TRANSPORT_FLAG_SSL 1 -#define TRANSPORT_FLAG_TLS 2 +#define TRANSPORT_FLAG_LDAPS 1 +#define TRANSPORT_FLAG_STARTTLS 2 #define BINDMETHOD_SIMPLE_AUTH 1 #define BINDMETHOD_SSL_CLIENTAUTH 2 #define BINDMETHOD_SASL_GSSAPI 3 diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c index 9c88d58..20a0ca9 100644 --- a/ldap/servers/plugins/replication/repl5_agmt.c +++ b/ldap/servers/plugins/replication/repl5_agmt.c @@ -73,7 +73,7 @@ typedef struct repl5agmt { char *hostname; /* remote hostname */ int64_t port; /* port of remote server */ - uint32_t transport_flags; /* SSL, TLS, etc. */ + uint32_t transport_flags; /* LDAPS, StartTLS, etc. */ char *binddn; /* DN to bind as */ struct berval *creds; /* Password, or certificate */ int64_t bindmethod; /* Bind method - simple, SSL */ @@ -148,7 +148,7 @@ Schema for replication agreement: cn nsds5ReplicaHost - hostname nsds5ReplicaPort - port number -nsds5ReplicaTransportInfo - "SSL", "startTLS", or may be absent; +nsds5ReplicaTransportInfo - "LDAPS", "StartTLS", or may be absent ("SSL" and "TLS" values will be deprecated later) nsds5ReplicaBindDN nsds5ReplicaCredentials nsds5ReplicaBindMethod - "SIMPLE" or "SSLCLIENTAUTH". @@ -207,7 +207,7 @@ agmt_is_valid(Repl_Agmt *ra) if ((0 == ra->transport_flags) && (BINDMETHOD_SSL_CLIENTAUTH == ra->bindmethod)) { slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "agmt_is_valid - Replication agreement \"%s\" " " is malformed: cannot use SSLCLIENTAUTH if using plain LDAP - please " - "change %s to SSL or TLS before changing %s to use SSLCLIENTAUTH\n", + "change %s to LDAPS or StartTLS before changing %s to use SSLCLIENTAUTH\n", slapi_sdn_get_dn(ra->dn), type_nsds5TransportInfo, type_nsds5ReplicaBindMethod); return_value = 0; } @@ -298,7 +298,7 @@ agmt_new_from_entry(Slapi_Entry *e) ra->port = port; } - /* SSL, TLS, or other transport stuff */ + /* LDAPS, StartTLS, or other transport stuff */ ra->transport_flags = 0; (void)agmt_set_transportinfo_no_lock(ra, e); (void)agmt_set_WaitForAsyncResults(ra, e); @@ -1755,10 +1755,10 @@ agmt_set_transportinfo_no_lock(Repl_Agmt *ra, const Slapi_Entry *e) tmpstr = slapi_entry_attr_get_charptr(e, type_nsds5TransportInfo); if (!tmpstr || !strcasecmp(tmpstr, "LDAP")) { ra->transport_flags = 0; - } else if (strcasecmp(tmpstr, "SSL") == 0) { - ra->transport_flags = TRANSPORT_FLAG_SSL; - } else if (strcasecmp(tmpstr, "TLS") == 0) { - ra->transport_flags = TRANSPORT_FLAG_TLS; + } else if (strcasecmp(tmpstr, "SSL") == 0 || strcasecmp(tmpstr, "LDAPS") == 0) { + ra->transport_flags = TRANSPORT_FLAG_LDAPS; + } else if (strcasecmp(tmpstr, "TLS") == 0 || strcasecmp(tmpstr, "StartTLS") == 0) { + ra->transport_flags = TRANSPORT_FLAG_STARTTLS; } /* else do nothing - invalid value is a no-op */ diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c index b44c5c7..c31b3df 100644 --- a/ldap/servers/plugins/replication/repl5_connection.c +++ b/ldap/servers/plugins/replication/repl5_connection.c @@ -1177,9 +1177,9 @@ conn_connect(Repl_Connection *conn) /* ugaston: if SSL has been selected in the replication agreement, SSL client * initialisation should be done before ever trying to open any connection at all. */ - if (conn->transport_flags == TRANSPORT_FLAG_TLS) { + if (conn->transport_flags == TRANSPORT_FLAG_STARTTLS) { secure = SLAPI_LDAP_INIT_FLAG_startTLS; - } else if (conn->transport_flags == TRANSPORT_FLAG_SSL) { + } else if (conn->transport_flags == TRANSPORT_FLAG_LDAPS) { secure = SLAPI_LDAP_INIT_FLAG_SSL; } diff --git a/ldap/servers/plugins/replication/windows_connection.c b/ldap/servers/plugins/replication/windows_connection.c index 1dcd8b7..8efa23b 100644 --- a/ldap/servers/plugins/replication/windows_connection.c +++ b/ldap/servers/plugins/replication/windows_connection.c @@ -1212,9 +1212,9 @@ windows_conn_connect(Repl_Connection *conn) /* ugaston: if SSL has been selected in the replication agreement, SSL client * initialisation should be done before ever trying to open any connection at all. */ - if (conn->transport_flags == TRANSPORT_FLAG_TLS) { + if (conn->transport_flags == TRANSPORT_FLAG_STARTTLS) { secure = SLAPI_LDAP_INIT_FLAG_startTLS; - } else if (conn->transport_flags == TRANSPORT_FLAG_SSL) { + } else if (conn->transport_flags == TRANSPORT_FLAG_LDAPS) { secure = SLAPI_LDAP_INIT_FLAG_SSL; }