On Tue, May 14, 2019 at 1:24 PM Daniel Gustafsson <dan...@yesql.se> wrote: > > On 14 May 2019, at 03:49, Thomas Munro <thomas.mu...@gmail.com> wrote: > > > I propose a new option $SUBJECT so that users can at least add a level of > > indirection and put the password in a file. > > +1, seems like a reasonable option to give.
Thanks for the review! > > Draft patch attached. > > I might be a bit thick, but this is somewhat hard to parse IMO: > > + File containing the password for user to bind to the directory with > to > + perform the search when doing search+bind authentication > > To add a little bit more security around this, does it make sense to check (on > unix filesystems) that the file isn’t world readable/editable? > > + fd = OpenTransientFile(path, O_RDONLY); > + if (fd < 0) > + return -1; Good point. However, I realised that this patch is nearly but not quite redundant. You can already write @somefile given a file somefile that contains ldapbindpasswd=secret. It'd be a bit nicer if you could also write ldapbindpasswd=@somefile to include just the value, and not have to include the option name in the file. Then you could use the same password file that you use for the ldapsearch command line tool, and in general that seems nicer. That syntax might have backwards compatibility problems though. You could probably resolve any problems by requiring quote marks around @ signs that are not acting as include directives, or something like that. If we do that, I'd also like to be able to write ldapbindpasswd=$SOME_ENV_VAR. Anyway, I hereby withdraw the earlier patch; it seems silly to do per-option ad hoc read-from-file variants. Perhaps we can do something much better and more general, or perhaps what we have is enough already. -- Thomas Munro https://enterprisedb.com
