PR#51061: Issue 51060 - unable to set sslVersionMin to TLS1.0 - 389-ds-base

389-ds-base

#51061 Issue 51060 - unable to set sslVersionMin to TLS1.0

Closed 3 years ago by spichugi. Opened 3 years ago by mreynolds.

mreynolds/389-ds-base sslminver into master

Issue 51060 - unable to set sslVersionMin to TLS1.0

Mark Reynolds • 3 years ago

3548738

dirsrvtests/tests/suites/tls/ssl_version_test.py

file modified

+12

		`@@ -19,10 +19,12 @@`
		`1. Get current default range`
		`2. Set sslVersionMin and verify it is applied after a restart`
		`3. Set sslVersionMax and verify it is applied after a restart`
		`+ 4. Sanity test all the min/max versions`
		`:expectedresults:`
		`1. Success`
		`2. Success`
		`3. Success`
		`+ 4. Success`
		`"""`

		`topo.standalone.enable_tls()`
		`@@ -47,6 +49,16 @@`
		`max = enc.get_attr_val_utf8('sslVersionMax')`
		`assert max == default_min`

		`+ # Sanity test all the min/max versions`
		`+ for attr, versions in [('sslVersionMin', ['TLS1.0', 'TLS1.1', 'TLS1.2', 'TLS1.0']),`
		`+ ('sslVersionMax', ['TLS1.0', 'TLS1.1', 'TLS1.2'])]:`
		`+ for version in versions:`
		`+ # Test that the setting is correctly applied after a restart`
		`+ enc.replace(attr, version)`
		`+ topo.standalone.restart()`
		`+ current_val = enc.get_attr_val_utf8(attr)`
		`+ assert current_val == version`
		`+`

		`if __name__ == '__main__':`
		`# Run isolated`

ldap/servers/slapd/ssl.c

file modified

+2 -2

		`@@ -1373,7 +1373,7 @@`
		`val, emin);`
		`(*rval) = enabledNSSVersions.min;`
		`} else {`
		`- (*rval) = CURRENT_DEFAULT_SSL_VERSION;`
		`+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;`
		`}`
		`} else {`
		`if (enabledNSSVersions.max < CURRENT_DEFAULT_SSL_VERSION) {`
		`@@ -1384,7 +1384,7 @@`
		`val, emax);`
		`(*rval) = enabledNSSVersions.max;`
		`} else {`
		`- (*rval) = CURRENT_DEFAULT_SSL_VERSION;`
		`+ (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;`
		`}`
		`}`
		`} else if (tlsv < 1.2f) { /* TLS1.1 */`

mreynolds commented 3 years ago

Description:

When processing the "sslVersionMin" attribute we were incorrectly setting it to TLS1.2 (current default level)

https://pagure.io/389-ds-base/issue/51060

firstyear commented 3 years ago

Does this leave the max default to 1.2?

mreynolds commented 3 years ago

Does this leave the max default to 1.2?

Yes, this change only impacts the ssl version range if it is explicitly set. If you don't set it, it defaults to the system crypto policy. So for me on Fedora 31 it defaults to 1.2 for both the min and the max. This bug was that if I set the min to 1.0, the server overwrote it when it should not have.

firstyear commented 3 years ago

Cool, in that case ack from me!

rebased onto 3548738

3 years ago

Pull-Request has been merged by mreynolds

3 years ago