389-ds-base

#50430 Issue 50426 - nsSSL3Ciphers is limited to 1024 characters
Closed by spichugi. Opened by mreynolds.
mreynolds/389-ds-base ticket50426  into  master

Download 50430.patch

Bug Description:

There was a hardcoded buffer for processing TLS ciphers. Anything over 1024 characters was truncated and was not applied.

Fix Description:

Don't use a fixed size buffer and just use the entire string. When printing errors about invalid format then we must use a fixed sized buffer, but we will truncate that log value as to not exceed the ssl logging function's buffer, and still output a useful message.

ASAN approved

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

Maybe I'm missing it, but how is ciphers allocated / reallocated to ensure it's large enough?

My understanding is that ciphers is allocated reading the config (slapd_ssl_init), checked in _conf_setciphers (slapd_ssl_init2) and then freed. Surprised that it is not used later.

Anyway the fix address the 1024 limit and looks good regarding the previous behavior.
LGTM

Maybe I'm missing it, but how is ciphers allocated / reallocated to ensure it's large enough?

"ciphers" is a static variable, and is set in slapi_ssl_init():

ciphers = slapi_entry_attr_get_charptr(entry, "nsssl3ciphers");

Then in slapd_ssl_init2() we parse it in _conf_setciphers() which copies each cipher substring into the global cipher list struct.

Linter complains here because we don't use the object. Generaly, I think it is better to use log functions from the instances. Like topo.ms["master1"].log.

Linter complains here because we don't use the object. Generaly, I think it is better to use log functions from the instances. Like topo.ms["master1"].log.

This is what create_test.py is doing. Maybe we should change it in there via a different ticket?

Meanwhile I will remove that code from the test script...

rebased onto d690114380bc592ede783826f45ea88262a4db23

Changes applied to CI test please review...

Linter complains here because we don't use the object. Generaly, I think it is better to use log functions from the instances. Like topo.ms["master1"].log.

This is what create_test.py is doing. Maybe we should change it in there via a different ticket?
Meanwhile I will remove that code from the test script...

Yeah, I think we should.

Could you also, please, remove from lib389._constants import * line? Ideally, mass imports with * should be avoided.

So yeah, we should have some create_test.py clean up...

It is really minor. The rest looks good to me!

rebased onto 6b2fd1bd7ac2aa6f9f6c90fc4683614ffb25afdb

rebased onto 22f2f9a1502e63bb169b7d599b5a3b35ddb31b8a

Pull-Request has been merged by mreynolds

All good for me too (sorry for late comment) and thanks for explaining about ciphers allocation!

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/3488

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.

Pull-Request has been closed by spichugi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.