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

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

From
smsiebe@gmail.com
Date:
The following bug has been logged on the website:

Bug reference:      10680
Logged by:          Steven Siebert
Email address:      smsiebe@gmail.com
PostgreSQL version: 9.3.4
Operating system:   Linux
Description:

When a user fails to login when the LDAP method is used, the ldapbindpasswd
(in plain text) is leaked to the log, even when the log level is set to
warning.

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

From
Stephen Frost
Date:
Greetings,

* smsiebe@gmail.com (smsiebe@gmail.com) wrote:
> When a user fails to login when the LDAP method is used, the ldapbindpasswd
> (in plain text) is leaked to the log, even when the log level is set to
> warning.

If you don't want the server to see the user's password, don't use LDAP
authentication.  A much better approach is Kerberos or client-side SSL
certificates.

There may be something which is done to improve the specific case
mentioned here (or perhaps not..), but if LDAP is used then the PG
server will see the user's password because that's how that
authentication system works.

    Thanks,

        Stephen

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

From
Steven Siebert
Date:
Thanks for the reply.

>
> If you don't want the server to see the user's password, don't use LDAP
> authentication.  A much better approach is Kerberos or client-side SSL
> certificates.

Sadly, all other authentication options will not work for us.

I'm not seeing the user password in the log, I'm seeing the bind
password (ldapbindpasswd) that in the pg_hba.conf file.  There is a
line in auth.c that, on every failed attempt, prints the full (raw)
configuration line to the log at all log levels.  So, this isn't just
a problem with LDAP (with ldapbindpasswd) but also the RADIUS method
(radiussecret).

I've submitted a patch and we're discussing the problem further on the
pgsql-hackers distro.  Really, I think it all comes down to finding
the right balance of security and convenience of the administrator.
I'm hopeful we'll come up with the right answer soon and I can submit
a new patch.

S

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

From
Stephen Frost
Date:
Steven,

* Steven Siebert (smsiebe@gmail.com) wrote:
> > If you don't want the server to see the user's password, don't use LDAP
> > authentication.  A much better approach is Kerberos or client-side SSL
> > certificates.
>=20
> Sadly, all other authentication options will not work for us.
>=20
> I'm not seeing the user password in the log, I'm seeing the bind
> password (ldapbindpasswd) that in the pg_hba.conf file.  There is a
> line in auth.c that, on every failed attempt, prints the full (raw)
> configuration line to the log at all log levels.  So, this isn't just
> a problem with LDAP (with ldapbindpasswd) but also the RADIUS method
> (radiussecret).

Ah, ok.  Kerberos and SSL certs aren't immune to that problem, though
the secrets don't ever end up in the logs- but they still must be
visible to the server process in order.  Of course, if you already
have access to the server process, there shouldn't be much to gain
=66rom the Kerberos secret, the RADIUS secret, the SSL private key, or
the LDAP bind password..

> I've submitted a patch and we're discussing the problem further on the
> pgsql-hackers distro.  Really, I think it all comes down to finding
> the right balance of security and convenience of the administrator.
> I'm hopeful we'll come up with the right answer soon and I can submit
> a new patch.

Oh, yeah, I saw that discussion but hadn't quite put it together with
this bug report (somehow I saw the bug report after the hackers
discussion...).

    Thanks,

        Stephen

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

From
Steven Siebert
Date:
> Ah, ok.  Kerberos and SSL certs aren't immune to that problem, though
> the secrets don't ever end up in the logs- but they still must be
> visible to the server process in order.  Of course, if you already
> have access to the server process, there shouldn't be much to gain
> from the Kerberos secret, the RADIUS secret, the SSL private key, or
> the LDAP bind password..

Agreed.  In our situation (government), though, we must export out
logs to enterprise logging services where auditors (that wouldn't
otherwise have access to the server/process) would be able to see it.

Despite the arguments of it being in another file...generally, having
clear-text secrets copied around to multiple places is a bad thing.  I
think it should be easy to come to compromise...and we're willing to
put in the work once we do figure out the best course of action =)

Thanks!

S
Steven Siebert <smsiebe@gmail.com> writes:
> Agreed.  In our situation (government), though, we must export out
> logs to enterprise logging services where auditors (that wouldn't
> otherwise have access to the server/process) would be able to see it.

The thing is that the postmaster logs will certainly contain all manner
of sensitive information.  A few examples:

* Occasionally, people mess up and enter their username as their password
and vice versa.  Logging of connection failures, or indeed mere logging of
error messages, will therefore expose their password --- admittedly, not
identified as such, but if you see a subsequent successful connection you
know whose it was.

* Logging of queries is likely to expose sensitive user data in the form
of constants in the queries, eg "INSERT INTO customers (name, address,
credit_card_number) VALUES (...)".  Even if you're not logging all
queries, failed queries could still expose such data.

* An example pretty directly connected to yours is that people have
complained about how statement logging will capture "ALTER USER joe
WITH PASSWORD 'joes-new-password'".

So basically, making the logs safe to show to untrusted auditors is a
fool's errand.  You need to deal with this problem in some other,
nontechnical, way.  IOW, why exactly don't you trust the auditors,
and how will you fix that?

            regards, tom lane

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

From
Steven Siebert
Date:
> * Occasionally, people mess up and enter their username as their password
> and vice versa.  Logging of connection failures, or indeed mere logging of
> error messages, will therefore expose their password --- admittedly, not
> identified as such, but if you see a subsequent successful connection you
> know whose it was.
>
> * Logging of queries is likely to expose sensitive user data in the form
> of constants in the queries, eg "INSERT INTO customers (name, address,
> credit_card_number) VALUES (...)".  Even if you're not logging all
> queries, failed queries could still expose such data.
>
> * An example pretty directly connected to yours is that people have
> complained about how statement logging will capture "ALTER USER joe
> WITH PASSWORD 'joes-new-password'".
>

Sadly, we (devs/administrators) realize all these things.  Big picture
logic plays no role in the way individuals develop the security
checklists or those accreditors that interpret those checklists.  It's
very black-and-white...either the database supports xyz (in this case,
no clear-text passwords in the log) or it fails that check.

I can give you specific reasons why the points you made above (which
are quite good points) are not applicable to us (ie users don't
directly log into the database, we use service accounts and handle
user auth/authz with PKI at the application level....and we don't log
individual statements because auditing changes are being done in a
different, approved, manner)...but we're digging down to such a
specific corner case it gets silly to argue scenarios.

> So basically, making the logs safe to show to untrusted auditors is a
> fool's errand.  You need to deal with this problem in some other,
> nontechnical, way.  IOW, why exactly don't you trust the auditors,
> and how will you fix that?

Right.  We (my team) agree.  We think it's stupid.  It doesn't matter
what we think. (Welcome to my world).  I'm sorry if I seem frustrated,
it's not with you...it's purely with the situation we're in having to
deal with this ourselves.

Believe me, I really hate to look at it like this, but it comes down
to: is there anything we can do within postgres / the postgres
community to eliminate this one specific 'vulnerability'?  Yes, we're
focusing on just the one vulnerability right now - where clear text
passwords are, arguably, *intentionally* sent into the log.  It's
something that can be fixed...and you have a developer (me) willing to
fix it if given direction on how he should proceed.

There are currently three suggestions on a fix put forth already:
 1) remove the raw line from the log entirely, just keeping the line number
 2) log that one specific event containing the raw log at a lower log
level (ie debug)
 3) parse out the password and continue to log the sanitized line at
the same "level" (all)

I'm OK with the fact that the patch I provided using the first
approach seems to be denied.  Can we consider either approach 2, 3, or
perhaps a combination or 2/3?

I do have alternative means at my disposal (ie use flume, or something
similar, to filter out just the log events I'm interested in and
forward off)...but we wanted to be able to help those behind us that
had similar concerns by fixing it at the source of the 'problem'.  I
want postgres to be unequivocally be approved software for the
government - not conditionally based on complex usages of 3rd party
applications to get it into an approved state.

S

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

From
Stephen Frost
Date:
* Steven Siebert (smsiebe@gmail.com) wrote:
> There are currently three suggestions on a fix put forth already:
>  1) remove the raw line from the log entirely, just keeping the line numb=
er
>  2) log that one specific event containing the raw log at a lower log
> level (ie debug)
>  3) parse out the password and continue to log the sanitized line at
> the same "level" (all)
>=20
> I'm OK with the fact that the patch I provided using the first
> approach seems to be denied.  Can we consider either approach 2, 3, or
> perhaps a combination or 2/3?

I actually don't really see a huge problem with 1, but I need to go
review the thread in more detail...

> I do have alternative means at my disposal (ie use flume, or something
> similar, to filter out just the log events I'm interested in and
> forward off)...but we wanted to be able to help those behind us that
> had similar concerns by fixing it at the source of the 'problem'.  I
> want postgres to be unequivocally be approved software for the
> government - not conditionally based on complex usages of 3rd party
> applications to get it into an approved state.

Yeah, I tend to agree- mistakes and errors are different considerations
when it comes to auditing, etc.

    Thanks,

        Stephen

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

From
Magnus Hagander
Date:
On Thu, Jun 19, 2014 at 5:37 PM, Stephen Frost <sfrost@snowman.net> wrote:

> * Steven Siebert (smsiebe@gmail.com) wrote:
> > There are currently three suggestions on a fix put forth already:
> >  1) remove the raw line from the log entirely, just keeping the line
> number
> >  2) log that one specific event containing the raw log at a lower log
> > level (ie debug)
> >  3) parse out the password and continue to log the sanitized line at
> > the same "level" (all)
> >
> > I'm OK with the fact that the patch I provided using the first
> > approach seems to be denied.  Can we consider either approach 2, 3, or
> > perhaps a combination or 2/3?
>
> I actually don't really see a huge problem with 1, but I need to go
> review the thread in more detail...
>

The reason the raw line was added in the first place was debugging cases
where the running pg_hba.conf might not be the same as the one in the
filesystem - either because of a reload not being done, or a reload of a
broken file.

I think 3 is a good option of these, assuming we can do it in a reasonably
good way.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

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

From
Stephen Frost
Date:
* Magnus Hagander (magnus@hagander.net) wrote:
> On Thu, Jun 19, 2014 at 5:37 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > I actually don't really see a huge problem with 1, but I need to go
> > review the thread in more detail...
>=20
> The reason the raw line was added in the first place was debugging cases
> where the running pg_hba.conf might not be the same as the one in the
> filesystem - either because of a reload not being done, or a reload of a
> broken file.

erm, not entirely convinced that's a great reason to log the whole line,
but..

> I think 3 is a good option of these, assuming we can do it in a reasonably
> good way.

I'd be fine with this approach.  I'd definitely like to see this
addressed in some manner because it's, clearly, not going to go away as
a request (I remember dealing with similar issues quite a few years ago
and all the arguments about how it "should" be ok to log passwords
didn't fly and we ended up having to address it also).

    Thanks,

        Stephen

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

From
Steven Siebert
Date:
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.

Thanks!

Steve



On Thu, Jun 19, 2014 at 12:09 PM, Stephen Frost <sfrost@snowman.net> wrote:
> * Magnus Hagander (magnus@hagander.net) wrote:
>> On Thu, Jun 19, 2014 at 5:37 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> > I actually don't really see a huge problem with 1, but I need to go
>> > review the thread in more detail...
>>
>> The reason the raw line was added in the first place was debugging cases
>> where the running pg_hba.conf might not be the same as the one in the
>> filesystem - either because of a reload not being done, or a reload of a
>> broken file.
>
> erm, not entirely convinced that's a great reason to log the whole line,
> but..
>
>> I think 3 is a good option of these, assuming we can do it in a reasonably
>> good way.
>
> I'd be fine with this approach.  I'd definitely like to see this
> addressed in some manner because it's, clearly, not going to go away as
> a request (I remember dealing with similar issues quite a few years ago
> and all the arguments about how it "should" be ok to log passwords
> didn't fly and we ended up having to address it also).
>
>         Thanks,
>
>                 Stephen

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

From
Magnus Hagander
Date:
On Mon, Jun 23, 2014 at 10:26 PM, Steven Siebert <smsiebe@gmail.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/

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

From
Steven Siebert
Date:
Thanks Magnus =)  I'll move forward with this guidance.


On Mon, Jun 23, 2014 at 4:35 PM, Magnus Hagander <magnus@hagander.net> wrote:
> On Mon, Jun 23, 2014 at 10:26 PM, Steven Siebert <smsiebe@gmail.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/

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

From
Bruce Momjian
Date:
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@hagander.net> wrote:
> > On Mon, Jun 23, 2014 at 10:26 PM, Steven Siebert <smsiebe@gmail.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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

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

From
Steven Siebert
Date:
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@momjian.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@hagander.net>
> wrote:
> > > On Mon, Jun 23, 2014 at 10:26 PM, Steven Siebert <smsiebe@gmail.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@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-bugs
>
> --
>   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>   EnterpriseDB                             http://enterprisedb.com
>
>   + Everyone has their own god. +
>

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

From
Steven Siebert
Date:
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@gmail.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@momjian.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@hagander.net> wrote:
> > On Mon, Jun 23, 2014 at 10:26 PM, Steven Siebert <smsiebe@gmail.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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +


Attachment

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

From
Tom Lane
Date:
Steven Siebert <smsiebe@gmail.com> writes:
> 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.

I still say that this is an ill-considered, unmaintainable, and
fundamentally insecure approach to solving the wrong problem.

As a single example of what's wrong with it, suppose that you
fat-finger some syntax detail of an LDAP line in pg_hba.conf.
When you issue "pg_ctl reload", will the postmaster log the broken
line in the postmaster log?  I sure hope so, because not doing so
would be a major usability fail.  Will it obscure the RADIUS secret?
No, because the syntax error will prevent it from correctly
identifying which part of the line is the secret, if indeed it
even realizes that the line might contain a secret.

If we go down this path, we'll be battling "oh, but what about that
other scenario?" cases till kingdom come; there will never be any
reason to think we've covered them all.

The right problem to be solving, to my mind, is that you feel a need
to give access to the postmaster log to untrusted people.  Now maybe
that's just a problem of wrong administrative procedures, but let's
consider what we might do in PG to improve your ability to do that
safely.  Perhaps what we should be entertaining is a proposal to have
multiple log channels, some containing more security-relevant messages
and others less so.  Then you could give people the ability to read only
the non-security-relevant messages.  If we arranged for *all* messages
relevant to pg_hba.conf to go into a secure log, it'd be a lot easier to
convince ourselves that we would not leak any security-critical info
than if we take the approach this patch proposes.

            regards, tom lane

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

From
Stephen Frost
Date:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Steven Siebert <smsiebe@gmail.com> writes:
> > 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.
>=20
> I still say that this is an ill-considered, unmaintainable, and
> fundamentally insecure approach to solving the wrong problem.

I agree that this approach is pretty grotty.

> As a single example of what's wrong with it, suppose that you
> fat-finger some syntax detail of an LDAP line in pg_hba.conf.

I've never liked that we tell users to put passwords in pg_hba.conf, be
they LDAP passwords or RADIUS secrets, it's just not the right place.

> Perhaps what we should be entertaining is a proposal to have
> multiple log channels, some containing more security-relevant messages
> and others less so.  Then you could give people the ability to read only
> the non-security-relevant messages.  If we arranged for *all* messages
> relevant to pg_hba.conf to go into a secure log, it'd be a lot easier to
> convince ourselves that we would not leak any security-critical info
> than if we take the approach this patch proposes.

I definitely like the idea of having multiple log channels with a way to
control what kinds of messages go to which- but I've also got a simpler
proposal:

Let's stop having passwords and secrets in a complex configuration file
which can have parsing and other failures.

How about allowing users to put that information in an independent file,
as do for SSL (admittedly, probably more because it's easier for us to
deal with OpenSSL that way, but still)?

ldapbindpwfile=3D/etc/whatever
radiussecretfile=3D/etc/whatever

as examples.  The files would contain *only* the password or secret
(ignoring any newline) and must be readable by the PG user.  We could
happily log any issues with pg_hba, etc, as long as we don't log what
came from those files.  Keeping the passwords out of pg_hba.conf also
makes it easier for admins to manage those files across multiple systems
(eg: using puppet, chef, etc) while having local PWs for each box too.
Might not be perfect for LDAP (unless we also provide a
ldapbinduserfile), though the bind user could be handled through puppet
or similar and it isn't as secure a concern, and it'd work directly for
RADIUS which directly support different secrets for different hosts.

    Thanks,

        Stephen

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

From
Tom Lane
Date:
Stephen Frost <sfrost@snowman.net> writes:
> Let's stop having passwords and secrets in a complex configuration file
> which can have parsing and other failures.

> How about allowing users to put that information in an independent file,
> as do for SSL (admittedly, probably more because it's easier for us to
> deal with OpenSSL that way, but still)?

And then what?  This other file can't possibly be so simple that
it's immune to having syntax errors, for example.  You're just moving
the same problems from point A to point B.  More, this would complicate
configuration and thereby create a whole new set of possible config
errors, which we'd then feel pressure to ameliorate by adding more
logging showing what the postmaster is doing.  And that logging would
have this same issue of maybe it's exposing information that person A
doesn't want logged ... even though person B needs that very same info
to help him figure out his configuration mistake.  A likely example of
that is feeding the wrong password/secret to some auth infrastructure
service, because you referenced the wrong item in this secondary file.

The core problem here is that we *need* to put security-relevant info
into the postmaster log in order to help people resolve problems.  Moving
around configuration details isn't going to fix that; indeed, the more
complicated the configuration files, the worse the problem will be.

            regards, tom lane

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

From
Stephen Frost
Date:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > Let's stop having passwords and secrets in a complex configuration file
> > which can have parsing and other failures.
>=20
> > How about allowing users to put that information in an independent file,
> > as do for SSL (admittedly, probably more because it's easier for us to
> > deal with OpenSSL that way, but still)?
>=20
> And then what?  This other file can't possibly be so simple that
> it's immune to having syntax errors, for example. =20

Uh, no, that's exactly the point of the independent file.

It's *only* the password.

fopen() - fails, then log that you can't open the file
fgets() - fails, then log that you can't read the file

(check for newline and remove it, if it's there)

If it grows to be a complex configuration file which can have syntax
errors, then it loses the point, I agree, but I don't hear anyone
complaining about SSL keys or SSH keys or Kerberos keytabs being leaked
in log files- and I'm pretty darn sure they would be complaining if it
was happening, regardless of any "keep your log files secure and only
let trusted people look at them" requirement.

> You're just moving
> the same problems from point A to point B.  More, this would complicate
> configuration and thereby create a whole new set of possible config
> errors, which we'd then feel pressure to ameliorate by adding more
> logging showing what the postmaster is doing.  And that logging would
> have this same issue of maybe it's exposing information that person A
> doesn't want logged ... even though person B needs that very same info
> to help him figure out his configuration mistake.  A likely example of
> that is feeding the wrong password/secret to some auth infrastructure
> service, because you referenced the wrong item in this secondary file.

This is done in other systems and has worked well from my experience.

> The core problem here is that we *need* to put security-relevant info
> into the postmaster log in order to help people resolve problems.  Moving
> around configuration details isn't going to fix that; indeed, the more
> complicated the configuration files, the worse the problem will be.

Having pg_hba be complicated but not having passwords in secrets in it
isn't making this problem worse- we can log all we want about pg_hba.
The point is to remove the sensetive information from the complicated
config file, even if that makes the complicated config file a bit more
complicated.

    Thanks,

        Stephen

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

From
Steven Siebert
Date:
Tom,

Your response is truly insightful - and completely valid.  Please, allow me
to explain our perspective...


> I still say that this is an ill-considered, unmaintainable, and
> fundamentally insecure approach to solving the wrong problem.
>

As you mentioned, the issue at hand isn't a complete solution, I certainly
agree.  The objections and scenarios you have raised are solid based on the
way the log is now, where items of interest to security auditors (ie
connection attempts (success and fail)) are interleaved with messages that
may reveal sensitive information about the data or the vulnerability of the
server.  As you suggested, the underlying problem we (US Government) have
is that our data must follow access control policies of lease
privileged/need to know.  Like I mentioned before, our use case is that
specific logging data identified in the NIST database STIG must make it to
a centralized repository (ie splunk), where the data must not contain
information not required to do the auditors job. But, with this particular
issue, even though I implement filtering at the log record level, filtering
out all the other messages containing possible data leakages or just simply
stuff that auditors wouldn't care about -- this is a specific log record we
need to track -- and it contains sensitive information...and it can be
fixed.

Could there be a spillage with other audit data?  Sure...there are
procedures in place to sanitize and ensure that problem doesn't happen
again - application bugs happen...and at the end of the day, all customers
must accept risk.  When this does happen...they call the devs (cue
superhero music) and we prevent it from happening again. It's just that, in
this case, one of my developers identified a risk before we deployed
production and I'm trying to fix this "bug" prior to accreditation...before
it bites...because we know about it....and I have fingers that can type
code.

Then there is another, simpler, view...should it be "OK" to routinely print
out passwords in log events...especially those log events that would want
to be seen by people other than the database admnistrators?   Consolidating
logging and making use of tools like splunk, apache flume, logstash is
common place now in enterprise and government...and now an emergence of
cloud logging services like loggly or logentries makes spillages like this
even scarier -- and can even make people decide against postgresql...


> As a single example of what's wrong with it, suppose that you
> fat-finger some syntax detail of an LDAP line in pg_hba.conf.
> When you issue "pg_ctl reload", will the postmaster log the broken
> line in the postmaster log?  I sure hope so, because not doing so
> would be a major usability fail.
>
Will it obscure the RADIUS secret?
> No, because the syntax error will prevent it from correctly
> identifying which part of the line is the secret, if indeed it
> even realizes that the line might contain a secret.
>

Without going down the rabbit hole discussing each particular scenario, I
think this is a good example how designing with security as a requirement
and not as an aspect/afterthought, could help in the design decision of a
component.  There is an alternative here that can satisfy both camps,
following well-known administrative precedent of fstab (don't confidently
update your fstab and then restart your server before you validate with
mount -a or mount -vf...another story for another day).  Rather than
logging the entire hba raw line to the log when there is such errors -
which has an inherent inconvenience of requiring the administrator to
restart (multiple times if there is an error?) the (production?)
application and have a RUNTIME failure to diagnose, why not provide a
utility making use of the postgresql codebase (even finding additional use
in unit testing code) to validate the hba file format/structure of the
file, and even validating the connection itself, providing a more verbose
error at the command line (where the admin obviously has permission).  With
this approach, you don't need to change your hba file...you also don't need
to log raw lines that knowingly contain sensitive information to a audit
message, you can log an line number only and even tell them to use the
utility to validate and get more information.  Bonus points for having a
tool that allows an admin to verify settings before restarting a server.


> The right problem to be solving, to my mind, is that you feel a need
> to give access to the postmaster log to untrusted people.  Now maybe
> that's just a problem of wrong administrative procedures, but let's
> consider what we might do in PG to improve your ability to do that
> safely.  Perhaps what we should be entertaining is a proposal to have
> multiple log channels, some containing more security-relevant messages
> and others less so.  Then you could give people the ability to read only
> the non-security-relevant messages.  If we arranged for *all* messages
> relevant to pg_hba.conf to go into a secure log, it'd be a lot easier to
> convince ourselves that we would not leak any security-critical info
> than if we take the approach this patch proposes.
>
>
I appreciate you humoring our use case rather than disregarding it as a
invalid administrative procedure =)

I think log channels is a great idea!  But, in the case, it wouldn't help,
right?  This is the sole message that gets logged when auth_failed is
called -- something we specifically must whitelist through our filter...as
required by NIST.  This message, specifically, is a problem, thus my focus
on the individual issue.  We solve the log channels problem essentially the
same way suggested by postgres.org...parsing the log (
http://wiki.postgresql.org/images/9/9d/Logging_pgopen_withnotes.pdf).  In
our opinion, the way postgresql does logging is adequate, because we can
filter and we can route and we can do whatever we need with 1st-3rd party
tools.  In fact, I don't meant to prevent any awesome new features that
could come out of this discussion -- but the logging we have now works,
even at cloud scale (tools have been developed to work around it).  The
problem we have is simply how this one log message gets formed. I don't
need a new logging system to be created to solve this one issue =)

Thanks,

S

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

From
Bruce Momjian
Date:
On Sun, Oct 12, 2014 at 03:42:10PM -0400, Tom Lane wrote:
> The right problem to be solving, to my mind, is that you feel a need
> to give access to the postmaster log to untrusted people.  Now maybe
> that's just a problem of wrong administrative procedures, but let's
> consider what we might do in PG to improve your ability to do that
> safely.  Perhaps what we should be entertaining is a proposal to have
> multiple log channels, some containing more security-relevant messages
> and others less so.  Then you could give people the ability to read only
> the non-security-relevant messages.  If we arranged for *all* messages
> relevant to pg_hba.conf to go into a secure log, it'd be a lot easier to
> convince ourselves that we would not leak any security-critical info
> than if we take the approach this patch proposes.

Uh, are we ready to output pg_hba.conf syntax errors (that might contain
passwords) to the that security channel?  That seems confusing too.  :-(

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

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

From
Alvaro Herrera
Date:
Bruce Momjian wrote:
> On Sun, Oct 12, 2014 at 03:42:10PM -0400, Tom Lane wrote:
> > The right problem to be solving, to my mind, is that you feel a need
> > to give access to the postmaster log to untrusted people.  Now maybe
> > that's just a problem of wrong administrative procedures, but let's
> > consider what we might do in PG to improve your ability to do that
> > safely.  Perhaps what we should be entertaining is a proposal to have
> > multiple log channels, some containing more security-relevant messages
> > and others less so.  Then you could give people the ability to read only
> > the non-security-relevant messages.  If we arranged for *all* messages
> > relevant to pg_hba.conf to go into a secure log, it'd be a lot easier to
> > convince ourselves that we would not leak any security-critical info
> > than if we take the approach this patch proposes.
>
> Uh, are we ready to output pg_hba.conf syntax errors (that might contain
> passwords) to the that security channel?  That seems confusing too.  :-(

I don't see why it would be confusing.  The rule would be along the
lines of "if there's a problem parsing pg_hba.conf, log to the security
channel".  It doesn't matter that the actual contents being logged turn
out not to be security sensitive; it's enough that they *could be*.

If this seems confusing, an idea is to log a generic "parse errors in
pg_hba.conf, see security.log for details" to the standard log channel.
But this doesn't seem necessary to me.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

From
Steven Siebert
Date:
I apologize if this is a double-post...I'm not sure if my message made it
to the distro, it's not in the mail list archive.  I don't mean to seem
like I've gone silent...I might have received a bounce and I didn't catch
it.

V/R,

Steve



---------- Forwarded message ----------
From: Steven Siebert <smsiebe@gmail.com>
Date: Sun, Oct 12, 2014 at 8:31 PM
Subject: Re: [BUGS] BUG #10680: LDAP bind password leaks to log on failed
authentication
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Bruce Momjian <bruce@momjian.us>, Magnus Hagander <magnus@hagander.net>,
Stephen Frost <sfrost@snowman.net>, pgsql-bugs <pgsql-bugs@postgresql.org>


Tom,

Your response is truly insightful - and completely valid.  Please, allow me
to explain our perspective...


> I still say that this is an ill-considered, unmaintainable, and
> fundamentally insecure approach to solving the wrong problem.
>

As you mentioned, the issue at hand isn't a complete solution, I certainly
agree.  The objections and scenarios you have raised are solid based on the
way the log is now, where items of interest to security auditors (ie
connection attempts (success and fail)) are interleaved with messages that
may reveal sensitive information about the data or the vulnerability of the
server.  As you suggested, the underlying problem we (US Government) have
is that our data must follow access control policies of lease
privileged/need to know.  Like I mentioned before, our use case is that
specific logging data identified in the NIST database STIG must make it to
a centralized repository (ie splunk), where the data must not contain
information not required to do the auditors job. But, with this particular
issue, even though I implement filtering at the log record level, filtering
out all the other messages containing possible data leakages or just simply
stuff that auditors wouldn't care about -- this is a specific log record we
need to track -- and it contains sensitive information...and it can be
fixed.

Could there be a spillage with other audit data?  Sure...there are
procedures in place to sanitize and ensure that problem doesn't happen
again - application bugs happen...and at the end of the day, all customers
must accept risk.  When this does happen...they call the devs (cue
superhero music) and we prevent it from happening again. It's just that, in
this case, one of my developers identified a risk before we deployed
production and I'm trying to fix this "bug" prior to accreditation...before
it bites...because we know about it....and I have fingers that can type
code.

Then there is another, simpler, view...should it be "OK" to routinely print
out passwords in log events...especially those log events that would want
to be seen by people other than the database admnistrators?   Consolidating
logging and making use of tools like splunk, apache flume, logstash is
common place now in enterprise and government...and now an emergence of
cloud logging services like loggly or logentries makes spillages like this
even scarier -- and can even make people decide against postgresql...


> As a single example of what's wrong with it, suppose that you
> fat-finger some syntax detail of an LDAP line in pg_hba.conf.
> When you issue "pg_ctl reload", will the postmaster log the broken
> line in the postmaster log?  I sure hope so, because not doing so
> would be a major usability fail.
>
Will it obscure the RADIUS secret?
> No, because the syntax error will prevent it from correctly
> identifying which part of the line is the secret, if indeed it
> even realizes that the line might contain a secret.
>

Without going down the rabbit hole discussing each particular scenario, I
think this is a good example how designing with security as a requirement
and not as an aspect/afterthought, could help in the design decision of a
component.  There is an alternative here that can satisfy both camps,
following well-known administrative precedent of fstab (don't confidently
update your fstab and then restart your server before you validate with
mount -a or mount -vf...another story for another day).  Rather than
logging the entire hba raw line to the log when there is such errors -
which has an inherent inconvenience of requiring the administrator to
restart (multiple times if there is an error?) the (production?)
application and have a RUNTIME failure to diagnose, why not provide a
utility making use of the postgresql codebase (even finding additional use
in unit testing code) to validate the hba file format/structure of the
file, and even validating the connection itself, providing a more verbose
error at the command line (where the admin obviously has permission).  With
this approach, you don't need to change your hba file...you also don't need
to log raw lines that knowingly contain sensitive information to a audit
message, you can log an line number only and even tell them to use the
utility to validate and get more information.  Bonus points for having a
tool that allows an admin to verify settings before restarting a server.


> The right problem to be solving, to my mind, is that you feel a need
> to give access to the postmaster log to untrusted people.  Now maybe
> that's just a problem of wrong administrative procedures, but let's
> consider what we might do in PG to improve your ability to do that
> safely.  Perhaps what we should be entertaining is a proposal to have
> multiple log channels, some containing more security-relevant messages
> and others less so.  Then you could give people the ability to read only
> the non-security-relevant messages.  If we arranged for *all* messages
> relevant to pg_hba.conf to go into a secure log, it'd be a lot easier to
> convince ourselves that we would not leak any security-critical info
> than if we take the approach this patch proposes.
>
>
I appreciate you humoring our use case rather than disregarding it as a
invalid administrative procedure =)

I think log channels is a great idea!  But, in the case, it wouldn't help,
right?  This is the sole message that gets logged when auth_failed is
called -- something we specifically must whitelist through our filter...as
required by NIST.  This message, specifically, is a problem, thus my focus
on the individual issue.  We solve the log channels problem essentially the
same way suggested by postgres.org...parsing the log (
http://wiki.postgresql.org/images/9/9d/Logging_pgopen_withnotes.pdf).  In
our opinion, the way postgresql does logging is adequate, because we can
filter and we can route and we can do whatever we need with 1st-3rd party
tools.  In fact, I don't meant to prevent any awesome new features that
could come out of this discussion -- but the logging we have now works,
even at cloud scale (tools have been developed to work around it).  The
problem we have is simply how this one log message gets formed. I don't
need a new logging system to be created to solve this one issue =)

Thanks,

S

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

From
Steven Siebert
Date:
I haven't heard a anything after my response to Toms email on 12Oct, is
there anything I can do to move the forward on this?  It's actually fine
with me if it's preferred to close the issue as won't fixed if the group
wants to go another direction...it at least gets me out of limbo and onto a
path of fixing either the right way (with a different patch) or as an
additional filter outside of postgres (I can re-implement what I did here
just as easily outside postgres).  Ideally, though, if there is anything I
can do to modify the path to better meet the requirements of the group, I'm
ready to take action.

Thanks,

S

On Sun, Oct 12, 2014 at 8:31 PM, Steven Siebert <smsiebe@gmail.com> wrote:

> Tom,
>
> Your response is truly insightful - and completely valid.  Please, allow
> me to explain our perspective...
>
>
>> I still say that this is an ill-considered, unmaintainable, and
>> fundamentally insecure approach to solving the wrong problem.
>>
>
> As you mentioned, the issue at hand isn't a complete solution, I certainly
> agree.  The objections and scenarios you have raised are solid based on the
> way the log is now, where items of interest to security auditors (ie
> connection attempts (success and fail)) are interleaved with messages that
> may reveal sensitive information about the data or the vulnerability of the
> server.  As you suggested, the underlying problem we (US Government) have
> is that our data must follow access control policies of lease
> privileged/need to know.  Like I mentioned before, our use case is that
> specific logging data identified in the NIST database STIG must make it to
> a centralized repository (ie splunk), where the data must not contain
> information not required to do the auditors job. But, with this particular
> issue, even though I implement filtering at the log record level, filtering
> out all the other messages containing possible data leakages or just simply
> stuff that auditors wouldn't care about -- this is a specific log record we
> need to track -- and it contains sensitive information...and it can be
> fixed.
>
> Could there be a spillage with other audit data?  Sure...there are
> procedures in place to sanitize and ensure that problem doesn't happen
> again - application bugs happen...and at the end of the day, all customers
> must accept risk.  When this does happen...they call the devs (cue
> superhero music) and we prevent it from happening again. It's just that, in
> this case, one of my developers identified a risk before we deployed
> production and I'm trying to fix this "bug" prior to accreditation...before
> it bites...because we know about it....and I have fingers that can type
> code.
>
> Then there is another, simpler, view...should it be "OK" to routinely
> print out passwords in log events...especially those log events that would
> want to be seen by people other than the database admnistrators?
> Consolidating logging and making use of tools like splunk, apache flume,
> logstash is common place now in enterprise and government...and now an
> emergence of cloud logging services like loggly or logentries makes
> spillages like this even scarier -- and can even make people decide against
> postgresql...
>
>
>> As a single example of what's wrong with it, suppose that you
>> fat-finger some syntax detail of an LDAP line in pg_hba.conf.
>> When you issue "pg_ctl reload", will the postmaster log the broken
>> line in the postmaster log?  I sure hope so, because not doing so
>> would be a major usability fail.
>>
> Will it obscure the RADIUS secret?
>> No, because the syntax error will prevent it from correctly
>> identifying which part of the line is the secret, if indeed it
>> even realizes that the line might contain a secret.
>>
>
> Without going down the rabbit hole discussing each particular scenario, I
> think this is a good example how designing with security as a requirement
> and not as an aspect/afterthought, could help in the design decision of a
> component.  There is an alternative here that can satisfy both camps,
> following well-known administrative precedent of fstab (don't confidently
> update your fstab and then restart your server before you validate with
> mount -a or mount -vf...another story for another day).  Rather than
> logging the entire hba raw line to the log when there is such errors -
> which has an inherent inconvenience of requiring the administrator to
> restart (multiple times if there is an error?) the (production?)
> application and have a RUNTIME failure to diagnose, why not provide a
> utility making use of the postgresql codebase (even finding additional use
> in unit testing code) to validate the hba file format/structure of the
> file, and even validating the connection itself, providing a more verbose
> error at the command line (where the admin obviously has permission).  With
> this approach, you don't need to change your hba file...you also don't need
> to log raw lines that knowingly contain sensitive information to a audit
> message, you can log an line number only and even tell them to use the
> utility to validate and get more information.  Bonus points for having a
> tool that allows an admin to verify settings before restarting a server.
>
>
>> The right problem to be solving, to my mind, is that you feel a need
>> to give access to the postmaster log to untrusted people.  Now maybe
>> that's just a problem of wrong administrative procedures, but let's
>> consider what we might do in PG to improve your ability to do that
>> safely.  Perhaps what we should be entertaining is a proposal to have
>> multiple log channels, some containing more security-relevant messages
>> and others less so.  Then you could give people the ability to read only
>> the non-security-relevant messages.  If we arranged for *all* messages
>> relevant to pg_hba.conf to go into a secure log, it'd be a lot easier to
>> convince ourselves that we would not leak any security-critical info
>> than if we take the approach this patch proposes.
>>
>>
> I appreciate you humoring our use case rather than disregarding it as a
> invalid administrative procedure =)
>
> I think log channels is a great idea!  But, in the case, it wouldn't help,
> right?  This is the sole message that gets logged when auth_failed is
> called -- something we specifically must whitelist through our filter...as
> required by NIST.  This message, specifically, is a problem, thus my focus
> on the individual issue.  We solve the log channels problem essentially the
> same way suggested by postgres.org...parsing the log (
> http://wiki.postgresql.org/images/9/9d/Logging_pgopen_withnotes.pdf).  In
> our opinion, the way postgresql does logging is adequate, because we can
> filter and we can route and we can do whatever we need with 1st-3rd party
> tools.  In fact, I don't meant to prevent any awesome new features that
> could come out of this discussion -- but the logging we have now works,
> even at cloud scale (tools have been developed to work around it).  The
> problem we have is simply how this one log message gets formed. I don't
> need a new logging system to be created to solve this one issue =)
>
> Thanks,
>
> S
>
>

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

From
Tom Lane
Date:
Steven Siebert <smsiebe@gmail.com> writes:
> I haven't heard a anything after my response to Toms email on 12Oct, is
> there anything I can do to move the forward on this?  It's actually fine
> with me if it's preferred to close the issue as won't fixed if the group
> wants to go another direction...

Well, Stephen was proposing that we make it possible to push all secrets
into separate files (one file per secret), which would allow DBAs to
eliminate the problem as long as they were willing to set up such files.
I'm not entirely sold on that reasoning, because I'm doubtful that people
would bother; but if you care to look into what that would involve, it
would help move the discussion forward.

            regards, tom lane

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

From
Steven Siebert
Date:
Thanks Tom,

Moving the secrets to another file will indeed fix this problem, and I
really don't think it'll take very much to implement this.  As you
mentioned, this increases the complexity of the configuration process,
and I certainly don't want to impose our backwards incompatible change
(on upgrade, they would need to create those new files) on everyone
else in the community.

At risk of adding more work for me, perhaps we can find a compromise
that would not create this backwards incompatible change while
addressing this and another issue that is related to gov requirements:
storing clear text passwords in configuration files.  Similar to the
DBA/auditor role separation, there is a requirement to separate the
roles of the DBA/administrator, and what they have access to - in this
case access to the secret (radiussecret/ldapbindpasswd) the postgres
dba is using to authenticate.  We've been able to mitigate this
because our dba is our admin -- but in reality this is an impediment
that each agency would need to work around as well.

My counter proposal is to optionally allow dbas to encrypt the secrets
used in the configuration file by substituting the actual password
with a variable.  This would allow the configuration to stay the same
for those that do not wish to make use of the feature, but for those
administrators that want to produce logs without clear text passwords
and remove clear text passwords from config files completely, they can
take the following steps:
1) replace the secrets with a variable: 'myPassword' changes to
${myPasswordRef}.
2) use pgadmin GUI or a new CLI command ('secphrase' or whatever) to
create a new encrypted phrase
3) while printing the hba line, the raw line will contain the variable
string but when the actual secret is requested the encrypted value
will be retrieved

Normally, storing anything in reversible encryption is really just the
same as storing it as clear text, but what I propose is to use the
same asymmetric keys used for secure SSL comms
(http://www.postgresql.org/docs/9.3/static/ssl-tcp.html).  PostgreSQL
already has precedence for this (SSL comms) and allows the user to
decide what degree they wish to protect their system: using a
passphrase on the private key forces the user to enter the passphrase
on boot, if no passphrase, it just works.  In the case of using the
same keys for the secret phrases, typing the passphrase once will also
allow for the decryption of the secret phrases.

The encrypted phrases that were created by this process wouldl be
stored in a special file with the key of the reference name and the
value being an b64 encoded encrypted secret.


Thoughts?

S



On Sun, Oct 19, 2014 at 4:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Steven Siebert <smsiebe@gmail.com> writes:
>> I haven't heard a anything after my response to Toms email on 12Oct, is
>> there anything I can do to move the forward on this?  It's actually fine
>> with me if it's preferred to close the issue as won't fixed if the group
>> wants to go another direction...
>
> Well, Stephen was proposing that we make it possible to push all secrets
> into separate files (one file per secret), which would allow DBAs to
> eliminate the problem as long as they were willing to set up such files.
> I'm not entirely sold on that reasoning, because I'm doubtful that people
> would bother; but if you care to look into what that would involve, it
> would help move the discussion forward.
>
>                         regards, tom lane

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

From
Tom Lane
Date:
Steven Siebert <smsiebe@gmail.com> writes:
> Moving the secrets to another file will indeed fix this problem, and I
> really don't think it'll take very much to implement this.  As you
> mentioned, this increases the complexity of the configuration process,
> and I certainly don't want to impose our backwards incompatible change
> (on upgrade, they would need to create those new files) on everyone
> else in the community.

I'm not following what's backwards incompatible about it?  As I see it,
this would require some new syntax ("@filename" perhaps?) that you
could optionally use in place of a secret.  If you had in mind to *force*
people to use out-of-line secret storage, I agree that ain't happening.

> My counter proposal is to optionally allow dbas to encrypt the secrets
> used in the configuration file by substituting the actual password
> with a variable.

This sounds like more complication with no real benefit.  Also, we've
generally avoided putting any strong encryption capability into core
Postgres because of worries about US export regulations.  Perhaps that
worry is obsolete, but I'm not sure.

            regards, tom lane

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

From
Steven Siebert
Date:
Tom, thanks for the quick response =)

> If you had in mind to *force*
> people to use out-of-line secret storage, I agree that ain't happening.

That's certainly something I don't want to do at all, which is why I
expressed concern.  I didn't see the idea of using a token/variable to
identify that the password lives in another file in the previous
emails, sorry if I missed it.  Since this is the same idea I had
presented for the variables, this obviously works for me =)

> This sounds like more complication with no real benefit.

Depends on the perspective again, I guess.  To those attempting to use
PostgreSQL in a secure environment following NIST best practices -- it
is a benefit =).  But, like I mentioned, I've successfully mitigated
that in our situation, so it's just a thought for those coming behind
me.

Honestly, it is more complicated...and not really necessary in my
situation.  But, IMO, that goes the same for moving the passwords out
of the one config file and to other file(s), since I've already
provided a patch that solves the problem of having clear text
passwords in the log file by filtering which also retains the
debugging benefit.  Honestly, I can't really see how this new approach
does anything more for security than what I already provided....but
I'm more than happy to oblige.  If anyone has the patients to explain
it to me, I would appreciate it =).

I really just wanted to offer this solution to the community, which
does provide additional security (removing clear text passwords from
config files, optionally) using existing security mechanism available
(SSL support baked in already).  I can provide a patch for any
approach there consensus for.

>Also, we've generally avoided putting any strong encryption capability into core
> Postgres because of worries about US export regulations.  Perhaps that
> worry is obsolete, but I'm not sure.

Most of the US export restrictions were removed about 15ish years ago
- unless you're using encryption specifically designed for the
military, you're good.

Further, Postgres already provides native SSL support
(http://www.postgresql.org/docs/9.3/static/ssl-tcp.html)...and my
proposal simply piggy backs on this.  I wasn't suggesting to
incorporate any new encryption technologies.

Like I said, I'm quite OK with not implementing this more complicated
solution -- just wanted to offer it up since it popped into my head.

S

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

From
Stephen Frost
Date:
Steven,

* Steven Siebert (smsiebe@gmail.com) wrote:
> At risk of adding more work for me, perhaps we can find a compromise
> that would not create this backwards incompatible change while
> addressing this and another issue that is related to gov requirements:
> storing clear text passwords in configuration files.  Similar to the
> DBA/auditor role separation, there is a requirement to separate the
> roles of the DBA/administrator, and what they have access to - in this
> case access to the secret (radiussecret/ldapbindpasswd) the postgres
> dba is using to authenticate.  We've been able to mitigate this
> because our dba is our admin -- but in reality this is an impediment
> that each agency would need to work around as well.

I agree that there is a requirement to not store passwords in config
files, but having secrets stored in independent files actually solves
that- those files are then *not* config files.  Consider that Kerberos
has a keytab file which certainly has secrets in it.  As for trying to
control what the DBA can see vs. the administrator- that's certainly
good too, but not possible with PG if the DBA is a superuser.  It
doesn't matter if it's encrypted or not, PG must know the secret at some
point and a superuser in PG can get access to pretty much anything that
the postgres unix user can.

This is why there is ongoing work to reduce the need for a superuser
role, to make it possible to have an actual DBA role which is *not* a
superuser but is still able to get their job done, while superuser would
be reserved for system administrators.

> 1) replace the secrets with a variable: 'myPassword' changes to
> ${myPasswordRef}.

We can simply have a new option for the radius auth method that is
'pwfile' or similar- there's no need for real variables.

> 2) use pgadmin GUI or a new CLI command ('secphrase' or whatever) to
> create a new encrypted phrase

If you can demonstrate that this would actually be useful then perhaps
it can be considered, but I don't see how this does anything for you-
how would PG get the password to use with the radius server?  You'd need
a key somewhere for that which PG can read.

> 3) while printing the hba line, the raw line will contain the variable
> string but when the actual secret is requested the encrypted value
> will be retrieved

Sure, we can print the hba line w/ this new option and not have the
secret be leaked to the logfile, but store the actual password for use
by the authentication method.

> Normally, storing anything in reversible encryption is really just the
> same as storing it as clear text, but what I propose is to use the
> same asymmetric keys used for secure SSL comms
> (http://www.postgresql.org/docs/9.3/static/ssl-tcp.html). =20

Ok- and then what?  You encrypt it with a different key than you use for
decryption, sure, but PG still has to know the decryption key to be able
to actually use the secret..  Now, if you fix radius to not use a shared
secret structure...

    Thanks,

        Stephen

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

From
Steven Siebert
Date:
Stephen,

Thanks for the response.  I was actually about to write the patch after
having let the thread bake a bit =)


> This is why there is ongoing work to reduce the need for a superuser
> role, to make it possible to have an actual DBA role which is *not* a
> superuser but is still able to get their job done, while superuser would
> be reserved for system administrators.
>

Ah!  That makes a lot of sense.  Thanks for the context.


>
>
> We can simply have a new option for the radius auth method that is
> 'pwfile' or similar- there's no need for real variables.
>
>
Easy enough - I can do it this way...much cleaner.


> > 2) use pgadmin GUI or a new CLI command ('secphrase' or whatever) to
> > create a new encrypted phrase
>
> If you can demonstrate that this would actually be useful then perhaps
> it can be considered, but I don't see how this does anything for you-
> how would PG get the password to use with the radius server?  You'd need
> a key somewhere for that which PG can read.
>
> That was really in the context of taking the approach of encrypting the
password...if it's a simple file...no biggie....don't really think this is
necessary.


> > 3) while printing the hba line, the raw line will contain the variable
> > string but when the actual secret is requested the encrypted value
> > will be retrieved
>
> Sure, we can print the hba line w/ this new option and not have the
> secret be leaked to the logfile, but store the actual password for use
> by the authentication method.
>

perfect!


>
> > Normally, storing anything in reversible encryption is really just the
> > same as storing it as clear text, but what I propose is to use the
> > same asymmetric keys used for secure SSL comms
> > (http://www.postgresql.org/docs/9.3/static/ssl-tcp.html).
>
> Ok- and then what?  You encrypt it with a different key than you use for
> decryption, sure, but PG still has to know the decryption key to be able
> to actually use the secret..  Now, if you fix radius to not use a shared
> secret structure...
>

lol.  well, the idea of piggybacking on the way SSL comms works is that
this has already been addressed - at the DBA makes the choice (based on
their requirements) if they want to have to type it in each time they
restart the service or put it in a file...I wouldn't be introducing a
foreign concept to postgres.  Of course, it's moot...I'm more than happy a
patch based around your suggestions - and it'll meet our needs for the
STIGs.

I'll move forward with writing the patch now, I'm grateful for the
additional feedback.

Thanks!

S

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

From
Stephen Frost
Date:
* Steven Siebert (smsiebe@gmail.com) wrote:
> Thanks for the response.  I was actually about to write the patch after
> having let the thread bake a bit =3D)

Great- would love to have a patch to review. :)

> > > Normally, storing anything in reversible encryption is really just the
> > > same as storing it as clear text, but what I propose is to use the
> > > same asymmetric keys used for secure SSL comms
> > > (http://www.postgresql.org/docs/9.3/static/ssl-tcp.html).
> >
> > Ok- and then what?  You encrypt it with a different key than you use for
> > decryption, sure, but PG still has to know the decryption key to be able
> > to actually use the secret..  Now, if you fix radius to not use a shared
> > secret structure...
>=20
> lol.  well, the idea of piggybacking on the way SSL comms works is that
> this has already been addressed - at the DBA makes the choice (based on
> their requirements) if they want to have to type it in each time they
> restart the service or put it in a file...I wouldn't be introducing a
> foreign concept to postgres.  Of course, it's moot...I'm more than happy a
> patch based around your suggestions - and it'll meet our needs for the
> STIGs.

Having a password have to be provided at startup is one option, I agree
with that.  I'm not sure it makes sense in this case, but perhaps..
We did do that with our KDC when I was operating a NIST Moderate level
system, but we didn't use that approach for the shared secrets on each
server as it just wouldn't have been workable.  Storing the files on a
encrypted filesystem which is mounted over loopback and requires a
password to mount would also work though and would remove the need for
PG to do any particular magic in this case.

Let's at least get the 'pwfile' option handled first and then we can
consider alternatives.  There's no need to introduce public key
cryptography for the "type-in-the-password-at-startup" solution though;
we'd only be doing that because we could piggy-back on OpenSSL.  I
suspect we could do better than that.

> I'll move forward with writing the patch now, I'm grateful for the
> additional feedback.

Sounds great!

If you aren't familiar with it already, you might want to check out our
Developer FAQ and http://commitfest.postgresql.org in particular, as
that's how we track patches which are submitted to the community to try
and avoid patches getting forgotten. :)

    Thanks!

        Stephen