Re: BUG #10680: LDAP bind password leaks to log on failed authentication

Attached is the patch (against master) for the approach we discussed:
sanitizing the log message by removing the sensitive information out of the
hba raw line.

Three things to note:

- I really only saw need to sanitize out the ldapbindpasswd and the
radiussecret (although I agree that can be argued). Is there anything else
that should be redacted?
- I substitute the sensitive phrase with the string "[sanitized]" -- this
is based on domain language of my customer...might be better named
something else?
- I couldn't for the life of me find a generic, simplified, replace_str
function either already imported from an existing dependency or within
postgresql code itself. Of course, some related functions in varlena.c and
regex, but I didn't think this routine warranted the overhead. Instead, I
settled for adding a simple single-purpose sanitizeLine function...but
being new here, I don't want to impose my design decisions over big picture
maintenance....especially since, as I noted, there is a potential (very
unlikely IMO) chance there could be a buffer overflow if someone makes a
single hba line over 4kb. I leave it in the experts hands...if it's
preferred an alternate way and it's simpler for you to just tell me what to
change in the patch, I can fix and resubmit.

Code successfully builds and existing regression tests pass. I did not add
any new tests here, because it seemed like a fairly large undertaking or
such a small change, since I couldn't find an existing set of tests to
piggy back on. I would be happy to add another todo request and work on
another path to add tests for auth.c as a whole if there is interest in it.

Thanks,

S

On Sat, Oct 11, 2014 at 8:44 PM, Steven Siebert <smsiebe(at)gmail(dot)com> wrote:

> Dropped off my radar I'm afraid, but the customer is still quite
> interested in getting this fixed. What we finally worked out should be
> quick work, I'll throw up a patch tonight. Thanks for the ping!
>
> Thanks,
>
> S
>
> On Sat, Oct 11, 2014 at 2:35 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
>>
>> Was any progress made on this, the reporting of LDAP/RADIUS passwords in
>> our server logs?
>>
>>
>> ---------------------------------------------------------------------------
>>
>> On Mon, Jun 23, 2014 at 04:42:24PM -0400, Steven Siebert wrote:
>> > Thanks Magnus =) I'll move forward with this guidance.
>> >
>> >
>> > On Mon, Jun 23, 2014 at 4:35 PM, Magnus Hagander <magnus(at)hagander(dot)net>
>> wrote:
>> > > On Mon, Jun 23, 2014 at 10:26 PM, Steven Siebert <smsiebe(at)gmail(dot)com>
>> wrote:
>> > >>
>> > >> Thanks for the continued discussion on this issue.
>> > >>
>> > >> It seems like, generally, fixing this vulnerability is getting a
>> green
>> > >> light.
>> > >>
>> > >> I wouldn't mind re-working the patch for this bug if I knew the
>> > >> consensus on the preferred implementation. As I mentioned
>> previously,
>> > >> I'm new here, so how do I go about soliciting "votes" (or otherwise)
>> > >> the preferred approach so that I may move forward.
>> > >
>> > >
>> > > I think the current summary is that "option c" is the one that people
>> would
>> > > accept if you submit it (provided the regular caveats about it being
>> > > correctly implemented etc, of course). It should of course cover other
>> > > potentially sensitive fields as well (such as the radius encryption
>> key).
>> > >
>> > > If you implement a patch for that option, I will be happy to review
>> and
>> > > apply it.
>> > >
>> > > --
>> > > Magnus Hagander
>> > > Me: http://www.hagander.net/
>> > > Work: http://www.redpill-linpro.com/
>> >
>> >
>> > --
>> > Sent via pgsql-bugs mailing list (pgsql-bugs(at)postgresql(dot)org)
>> > To make changes to your subscription:
>> > http://www.postgresql.org/mailpref/pgsql-bugs
>>
>> --
>> Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
>> EnterpriseDB http://enterprisedb.com
>>
>> + Everyone has their own god. +
>>
>
>