Issue 6256 - nsslapd-numlisteners limit is not enforced #6257
Conversation
ldap/servers/slapd/configdse.c
Outdated
| @@ -287,7 +287,8 @@ load_config_dse(Slapi_PBlock *pb __attribute__((unused)), | |||
| if (attr_name) { | |||
| retval = config_set(attr_name, values, returntext, 1 /* force apply */); | |||
| if ((strcasecmp(attr_name, CONFIG_MAXDESCRIPTORS_ATTRIBUTE) == 0) || | |||
| (strcasecmp(attr_name, CONFIG_RESERVEDESCRIPTORS_ATTRIBUTE) == 0)) { | |||
| (strcasecmp(attr_name, CONFIG_RESERVEDESCRIPTORS_ATTRIBUTE) == 0) || | |||
| (strcasecmp(attr_name, CONFIG_NUM_LISTENERS_ATTRIBUTE) == 0)) { | |||
There was a problem hiding this comment.
I wonder why to put the attribute to this check if we don't benefit from that?
We should either remove it from here or we should add this (or something like this with LDAP_UNWILLING_TO_PERFORM) to ldap/servers/slapd/libglobs.c::config_set_num_listeners:
if (nValue > maxVal) {
nValue = maxVal;
retVal = LDAP_UNWILLING_TO_PERFORM;
} else {
retVal = LDAP_OPERATIONS_ERROR;
}Or do I miss something?
There was a problem hiding this comment.
The check is here to cover the corner case where a user has changed version, if we don't add the check and the "nsslapd-numlisteners" attr is present in dse.ldif the instance will fail to start.
IIUC we need to return LDAP_OPERATIONS_ERROR when an invalid value has been used for this attr, otherwise the invalid value gets written to dse.ldif.
There was a problem hiding this comment.
I had a look over this again and yes, it is best to return LDAP_UNWILLING_TO_PERFORM when an invalid value for the nsslapd-numlisteners is used.
There was a problem hiding this comment.
Okay, IIUC, you changed the behaviour now to reject the value instead of normalizing it (as it was before).
Could you please elaborate on why you think it's the current thing to do for nsslapd-numlisteners?
You probably need to change the commit message and test the docstring with the explanation.
There was a problem hiding this comment.
When the dse backend callback is called there are two passes done for each mod, the first pass validates the value and the second pass apply's the change. With the existing behaviour, when an invalid value is input the config code normalises the value during the first pass, then during the second pass the invalid value is written to dse.ldif, which is why this issue was created.
To avoid this we need to reject an invalid value so the second pass doesn't apply the change. To reject the invalid value we need to return either LDAP_OPERATIONS_ERROR like almost all of the other config_set functions or LDAP_UNWILLING_TO_PERFORM.
Yes the plan was to update the commit message during merge, I need to update the CI test so I will update the doc string also.
|
Sorry, I was waiting for GitHub CI to be working again, but maybe we had some fix recently? Could you please rebase onto It will also give us the test you and @ssidhaye added. |
Description: When a invalid value for the attribute nsslapd-numlisteners is used, config normalises the value but the invalid value is written to dse.ldif. Fix description: Modify config to return operations error if an invalid value is used. Modify configdse to handle an operations error if an invalid value is used. Fixes: #6256 Reviewed by:
Description: When a invalid value for the attribute nsslapd-numlisteners is used, config normalises the value but the invalid value is written to dse.ldif.
Fix description: Modify config to return operations error if an invalid value is used. Modify configdse to handle an operations error if an invalid value is used.
Fixes: #6256
Reviewed by: