#48816 (1.2.11 only) add a nsTLS1.0 on or off new configuration parameter to cn=encryption,cn=config in RHEL 6 389-ds-base
Closed: wontfix None Opened 7 years ago by nhosoi.

Description of problem:

389-ds-base got added support of TLS1.1 starting in
389-ds-base-1.2.11.15-55.el6 but there is no way to select the minimum TLS
version on the server side like in RHEL 7, which is TLS1.0 by default on RHEL
6.

this is a customer request to add support of sslversionmin and sslversionmax in
RHEL 6 389-ds-base, the goal is to disable TLS1.0


Version-Release number of selected component (if applicable):
389-ds-base-1.2.11.15-72.el6_7.x86_64


How reproducible:
always, default SSL/TLS configuration


Steps to Reproduce:

1. RHDS install with SSL/TLS config, and in dse.ldif:
dn: cn=encryption,cn=config
nsSSL2: off
nsSSL3: off
nsTLS1: on

2. trying to mimic a "legacy" LDAP client that cannot do TLS1.1 nor TLS1.2
echo "exit" |  openssl s_client -no_tls1_1 -no_tls1_2 -connect 10.14.5.15:636 |
grep Protocol


Actual results:
the LDAP server accepts TLS 1.0:

depth=1 CN = CAcert
DONE
    Protocol  : TLSv1

On my instance I can see:

{{{
dn: cn=encryption,cn=config
objectClass: top
objectClass: nsEncryptionConfig
cn: encryption
nsSSLSessionTimeout: 0
nsSSLClientAuth: allowed
sslVersionMin: TLS1.0
}}}

Why would we use

{{{
nsSSL2: off
nsSSL3: off
nsTLS1: on
nsTLS10: on
nsTLS11: on
}}}

Over sslVersionMin?

Also, it's confusing to see: nsTLS1 == tls1.2.

I think it should either bet:

{{{
sslVersionMin: TLS1.X
}}}

OR

{{{
nsTLS1: on
nsTLS10: on
nsTLS11: on
nsTLS12: on
}}}

Where nsTLS1 defaults to on, and you have to have "nsTLS1 | nsTLS12" to enable the proto. Or it could be "nsTLS1 & nsTLS12" but I feel that is confusing too ....

summary: We need to be explicit and clear about config meanings else it will confuse admins :)

Replying to [comment:2 firstyear]:

On my instance I can see:

{{{
dn: cn=encryption,cn=config
objectClass: top
objectClass: nsEncryptionConfig
cn: encryption
nsSSLSessionTimeout: 0
nsSSLClientAuth: allowed
sslVersionMin: TLS1.0
}}}
Because sslVersionMin is not backported to 1.2.11 and no plan to do so now.

Why would we use

{{{
nsSSL2: off
nsSSL3: off
nsTLS1: on
nsTLS10: on
nsTLS11: on
}}}

Over sslVersionMin?
Not "over". sslVersionMin does not exist in 1.2.11.

Also, it's confusing to see: nsTLS1 == tls1.2.
nsTLS1 existed before this patch which meant nsTLS1.0 through the highest that the "current" NSS library supports. That said, nsTLS1 != TLS1.2. It could be nsTLS1 == [TLS1.0 .. TLS1.1] or [TLS1.0 .. TLS1.2] or [TLS1.0 .. TLS1.3] depending upon the version of NSS. And there is no plan to change the existing behaviour of nsTLS1.

On top of it, I meant to introduce nsTLS10 and nsTLS11 to disable TLS1.0 and TLS1.1.

The code may be confusing, but the default value of nsTLS1, nsTLS10 and nsTLS11 are all on. So, unless you explicitly disable nsTLS1, [TLS1.0 .. highest] is enabled.

I think it should either bet:

{{{
sslVersionMin: TLS1.X
}}}
No plan to do this.

OR

{{{
nsTLS1: on
nsTLS10: on
nsTLS11: on
nsTLS12: on
}}}

Where nsTLS1 defaults to on, and you have to have "nsTLS1 | nsTLS12" to enable the proto. Or it could be "nsTLS1 & nsTLS12" but I feel that is confusing too ....
It may be confusing. So, we could clearly state the above.
nsTLS1: on ==> [TLS1.0 .. highest]
If you add "nsTLS10: off" to the above ==> [TLS1.1 .. highest]
If you add "nsTLS11: off" to the above ==> [TLS1.2 .. highest]

summary: We need to be explicit and clear about config meanings else it will confuse admins :)
Again, nsTLS10 and nsTLS11 is for disabling the older versions not to add. Hope it is not that confusing.

Thank you for your comments, William. Could you please take a look at my response (#comment:3)? Thanks!!

Replying to [comment:3 nhosoi]:

Replying to [comment:2 firstyear]:

On my instance I can see:

{{{
dn: cn=encryption,cn=config
objectClass: top
objectClass: nsEncryptionConfig
cn: encryption
nsSSLSessionTimeout: 0
nsSSLClientAuth: allowed
sslVersionMin: TLS1.0
}}}
Because sslVersionMin is not backported to 1.2.11 and no plan to do so now.

Okay, that makes sense,

Also, it's confusing to see: nsTLS1 == tls1.2.
nsTLS1 existed before this patch which meant nsTLS1.0 through the highest that the "current" NSS library supports. That said, nsTLS1 != TLS1.2. It could be nsTLS1 == [TLS1.0 .. TLS1.1] or [TLS1.0 .. TLS1.2] or [TLS1.0 .. TLS1.3] depending upon the version of NSS. And there is no plan to change the existing behaviour of nsTLS1.

On top of it, I meant to introduce nsTLS10 and nsTLS11 to disable TLS1.0 and TLS1.1.

The code may be confusing, but the default value of nsTLS1, nsTLS10 and nsTLS11 are all on. So, unless you explicitly disable nsTLS1, [TLS1.0 .. highest] is enabled.

OR

{{{
nsTLS1: on
nsTLS10: on
nsTLS11: on
nsTLS12: on
}}}

Where nsTLS1 defaults to on, and you have to have "nsTLS1 | nsTLS12" to enable the proto. Or it could be "nsTLS1 & nsTLS12" but I feel that is confusing too ....
It may be confusing. So, we could clearly state the above.
nsTLS1: on ==> [TLS1.0 .. highest]
If you add "nsTLS10: off" to the above ==> [TLS1.1 .. highest]
If you add "nsTLS11: off" to the above ==> [TLS1.2 .. highest]

summary: We need to be explicit and clear about config meanings else it will confuse admins :)
Again, nsTLS10 and nsTLS11 is for disabling the older versions not to add. Hope it is not that confusing.

It's confusing me :S

Anyway, here is the code you have:

{{{
if (enableTLS10) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_0;
} else if (enableTLS11) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_1;
} else if (enableTLS1) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_2;
}}}

So 'nsTLS1: on' only enables tls1.2 as the minimum, NOT tls1.0 through tls1.2. This is what is confusing. Where as you said:

I think that this will cause some misunderstanding. I would write this as:

{{{
if (enableTLS1 || enableTLS10) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_0;
} else if (enableTLS1 || enableTLS11) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_1;
} else if (enableTLS1 || enableTLS12) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_2;
}}}

Then, if you have nsTLS1: on you mean "all versions of TLS". If you want a specific one, you turn that to off, and set on for the version min you want.

Alternately:

{{{
if (enableTLS1) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_0;
} else if ( enableTLS11) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_1;
} else if (enableTLS12) {
NSSVersionMin = SSL_LIBRARY_VERSION_TLS_1_2;
}}}

Make nsTLS1: represent 1.0 and all higher versions, and add nsTLS12 as the option for on.

Note this is just for discussion, and doesn't mean it will be the accepted one.

As per the patch description:

{{{
Example:
cn=encryption,cn=config
nsTLS1: off
nsTLS11: on ## Default
==> sslVersionMin: TLS1.1

cn=encryption,cn=config
nsTLS10: off
nsTLS11: off
nsTLS12: on ## Default
==> sslVersionMin: TLS1.2

cn=encryption,cn=config
nsTLS1: on
nsTLS11: off
==> sslVersionMin: TLS1.0

}}}

This makes it so that nsTLS1: means "Tls 1.0 and higher". nsTLS11: means "Tls 1.1 and higher".

This way, if there is a nsTLS13 one day, it's easier to add.

Additionally, no config change is needed to current servers, and we get a clearer meaning for the settings (I think).

I know you did not have a time to test it, but the code has some more tricks... (e.g, if nsTLS1 and nsSSL3 are both off, nsTLS1 is forced to enabled type of things.) So, unless test the code, it's not ready to push, unfortunately.

If you are hoping to take this ticket over, I don't mind?

It's not really my intent to take over the ticket .... More just asking the question about if there is a better way to express our meaning in this configuration.

Well, given that I haven't changed the meaning of nsTLS1, those things will still work. And may actually work "better", as nsTLS1 now covers TLS1.0 and above rather than TLS1.2 and higher. In the case you said "if nsTLS1 and nsSSL3 are both off, nsTLS1 is forced to enabled type of things." where you turn off NSTLS and SSL3, in this code you submitted, this would mean we only support TLS1.2, which could potentially break many clients .... Where my version, it will enable TLS1.0 and higher.

I posted this patch so that it could provoke thoughts about if there is a better way to express the config that is less "confusing".

Perhaps think about it, and we'll see :)

git patch file (master) -- revised based upon the comments by William and Ludwig (Thanks!)
0001-Ticket-48816-1.2.11-only-add-a-nsTLS1.0-on-or-off-ne.3.patch

Yep I'm happy with this. It's a good compromise between both of our ideas I think.

Thanks so much for the ideas, comments and discussions, William, Ludwig, and Mark!!
Final review was made by wibrown@redhat.com.

Pushed to 389-ds-base-1.2.11 branch:
2550047..6111400 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 6111400

Metadata Update from @firstyear:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.2.11.33

7 years ago

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 issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1876

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata