I think we should improve the message, maybe "Unauthenticated binds are enabled - this is insecure".

Second, looking at this, I can see that you change from hard tab to spaces - perhaps your editor did this?

Anyway, I prefer 4 space, but please make sure that you apply it to the whole function (line 6516 through 6536 with the patch).

If you use vim, you can see this with:

set list listchars=tab:·\ ,trail:·,precedes:<,extends:>

About the message: as we discussed in the mail thread unauth binds will become anonymous and so no more info will be provided than by a direct anonymous bind. SO the main problem is on the client assuming to bind with a dn and getting results for anonymous. I would try to reflect this in the log message, eg "Enabling unauthenticated binds can send unexpected results to clients and should be avoided"

And I think a message at startup is easily overlooked or does not help much in tracking clients doing aunautheticated binds.

How about to extend the messsage in the access log:

conn=4 op=0 RESULT err=0 tag=97 nentries=0 etime=0.000000 dn="" (unauthenticated bind)

Second, looking at this, I can see that you change from hard tab to spaces - perhaps your editor did this?

Anyway, I prefer 4 space, but please make sure that you apply it to the whole function (line 6516 through 6536 with the patch).

Hmm, I'm not sure what you mean here. This function uses hard tabs already so that's I used as well to indent the code. But then I used spaces to indent the slapi_log_err arguments, because if we use tabs in this case it might end up displaying the result differently in different editors (meaning, the arguments will not be aligned properly). Does this make sense? Or do you want me to change it to hard tabs or spaces for the whole function?

@ilias95 Your editor made them "soft" tabs, so it messed up the patch somehow. I couldn't apply it without ignore white space options .... >.<

I find it easier to just convert places with hard tabs to soft if it's a small enough function, but @mreynolds plan to clang format the whole code base soon to fix this anyway.

I think that @lkrispen's comments are really valid. How about you update the message based on his input.

@lkrispen It's true it's easy for admins to overlook. It's also something we assume most people don't use. I think adding the unauth message to the log is a good idea, but we can do that as a follow up patch? Another option is that in the new lib389 tools we have a healthcheck function which can "report" bad configs, so perhaps we could add this to taht?

Okay, I replaced all hard tabs in the function by spaces and I updated the log message.

I think this message is a bit scary when we say "potential security issues":

"Unauthenticated binds are enabled - this can lead to unexpected results with clients and potential security issues\n"

I can see admins saying: What security issues?! Oh my, lets open a support case, etc.

The potential security issue is actually on the client side and not the DS side. I think we should use Ludwig's original suggestion saying it should just "be avoided", and not use words like "security". Thoughts?

If we don't use the word security, people will ignore it. We need a scary warning like this to make people notice it I think. Perhaps we could say client security issues to be specific?

If we don't use the word security, people will ignore it.

In my experience many customers do watch the errors log, and when a new message appears they do take notice and inquire about it to support or a mailing list.

We need a scary warning like this to make people notice it I think. Perhaps we could say client security issues to be specific?

That's better, but it's only a client security issue if the client uses LDAP binds for its own authentication purposes against some other application. It still has no impact on DS and it's data. It's a corner case IMO, and to use vague/scary messages is inappropriate - especially since it's all on the client side.

Perhaps we just need a very verbose warning message describing the above scenario in more detail. But saying "potential security issues", or "potential client security issues", is just way too vague, and not good practice in my opinion. If we are going to state it's a security issue then we need to say why, and in detail.

Either way we need to fully document this somewhere, either in a tech doc or in the Admin guide.

Yeah, it needs to go to the admin guide. I think we could bikeshed the message a lot, but "this could cause client security issues, please contact support / docs for more" at a general outline.

@mreynolds Did we come up with a solution to the msg? @ilias95 what do you think?

@firstyear, this could also be a job for healthcheck tool. Don't you think it would be more useful there ?

@tbordaz For certain - at the time, the healthcheck tool wasn't conceived yet. I think this is a good candidate for a healtcheck item :)

@firstyear thanks for your feedback. I am changing the title of the ticket.

Heathcheck tool will improve from version to version I do not think there is a need to backport each improvement in previous releases. Moving this ticket to 1.4.3 but if someone feels it is required in 1.4.2 please switch it back to that milestone.

Agreed, this is okay for 1.4.3

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

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.