Discussion in the #49109 is still going. I've used original option ssl->ldaps and tls->starttls for now.
It is a draft of the patch, we can change it later if we'll agree so.

I'll add a test also a bit later.

Tiny nitpick but I think this should be LDAP+StartTLS instead of StartTLS. But that's just my opinion :)

There are tests in replication with TLS we could use as an inspiration for this?

I've already tested the patch for replica agreement (I used suites/replication/tls_client_auth_repl_test.py for this).
I will create parametrization there for both options and update the pull-request.
I want to be 100% that I don't break anything here. :D

And regarding LDAP+StartTLS. I am okay with it, though I think StartTLS is also pretty understandable enough (we can't have LDAPS+StartTLS, it is against the concept).

Let's see what others think and I can change it accordingly. :)
@mreynolds @lkrispen @tbordaz @mhonek

I think it is a good idea. LDAP_StartTLS is also an option.
Should we accept StartTLS+LDAP and LDAP+StartTLS (or StartTLS_LDAP and LDAP_StartTLS)

As William and Matus proposed, I've changed nsDS5ReplicaTransportInfo: TLS to LDAP+StartTLS.
Also, I've changed existed TLS test suite so it covers the change.

We are still in the decision process. So if we will agree with Theirry's option, I'll change the logic accordingly.

Tiny nitpick is DNA_PROT_LDAP_STARTTLS should be the name given the previous scheme used of the macros.

Beside my tiny nitpick, I think this looks good. Ack from me. (Note I haven't built this or run the test).

@spichugi ping, this is pretty easy to follow up and finish :)

I have a problem with LDAP+StartTLS. First, this implies the order does not matter: It could also be StartTLS+LDAP. This requires more logic to process the setting. But the main problem I have is that StartTLS implies LDAP. For example, you would never use LDAPS+StartTLS. If you can set "LDAP+StartTLS", then as a end user, who is inexperienced, I would think that LDAPS+StartTLS is also a valid option. It's confusing because it's redundant. I think for clarity & simplicity it should just be "StartTLS" (case insensitive).

LGTM as long as firstyear's comment about DNA_PROT_LDAP_STARTTLS was addressed (since the PR was rebased I don't know what he was talking about or if you addressed it).

LGTM as long as firstyear's comment about DNA_PROT_LDAP_STARTTLS was addressed (since the PR was rebased I don't know what he was talking about or if you addressed it).

I changed it back too because of the LDAP+StartTLS -> StartTLS change.

Before his comment it was:

+#define DNA_PROT_LDAPS "LDAPS"
+#define DNA_PROT_STARTTLS "LDAP+StartTLS"

William asked for:

+#define DNA_PROT_LDAPS "LDAPS"
+#define DNA_PROT_LDAP_STARTTLS "LDAP+StartTLS"

But we changed LDAP+StartTLS so I changed it back again (it is now like this):

+#define DNA_PROT_LDAPS "LDAPS"
+#define DNA_PROT_STARTTLS "StartTLS"

I can change it back to this, no problem:

+#define DNA_PROT_LDAPS "LDAPS"
+#define DNA_PROT_LDAP_STARTTLS "StartTLS"

But it seems a bit redundant to me.

I see, yes I like it the way you have it now. I also see why William did it the other way (to be consistent with the "LDAP+StartTLS" naming), but I agree it is redundant and I like what you have now.

Fine by me :) thanks for taking it into consideration. Ack.

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2672

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.