Re: FPW compression leaks information

On 07/07/2015 04:31 PM, Stephen Frost wrote:
> * Heikki Linnakangas (hlinnaka@iki.fi) wrote:
>> On 07/07/2015 04:15 PM, Stephen Frost wrote:
>>> * Fujii Masao (masao.fujii@gmail.com) wrote:
>>>> On Thu, Apr 16, 2015 at 4:26 PM, Michael Paquier
>>>> <michael.paquier@gmail.com> wrote:
>>>>> +         the compression ratio of a full page image gives a hint of what is
>>>>> +         the existing data of this page.  Tables that contain sensitive
>>>>> +         information like <structname>pg_authid</structname> with password
>>>>> +         data could be potential targets to such attacks. Note that as a
>>>>> +         prerequisite a user needs to be able to insert data on the same page
>>>>> +         as the data targeted and need to be able to detect checkpoint
>>>>> +         presence to find out if a compressed full page write is included in
>>>>> +         WAL to calculate the compression ratio of a page using WAL positions
>>>>> +         before and after inserting data on the page with data targeted.
>>>>> +        </para>
>>>>> +       </warning>
>>>>
>>>> I think that, in addition to the description of the risk, it's better to
>>>> say that this vulnerability is cumbersome to exploit in practice.
>>>>
>>>> Attached is the updated version of the patch. Comments?
>>>
>>> Personally, I don't like simply documenting this issue.  I'd much rather
>>> we restrict the WAL info as it's really got no business being available
>>> to the general public.  I'd be fine with restricting that information to
>>> superusers when compression is enabled, or always, for 9.5 and then
>>> fixing it properly in 9.6 by allowing it to be visible to a
>>> "pg_monitoring" default role which admins can then grant to users who
>>> should be able to see it.
>>
>> Well, then you could still launch the same attack if you have the
>> pg_monitoring privileges. So it would be more like a "monitoring and
>> grab everyone's passwords" privilege ;-). Ok, in seriousness the
>> attack isn't that easy to perform, but I still wouldn't feel
>> comfortable giving that privilege to anyone who isn't a superuser
>> anyway.
>
> The alternative is to have monitoring tools which are running as
> superuser, which, in my view at least, is far worse.

Or don't enable fpw_compression for tables where the information leak is 
a problem.

>>> Yes, we'll get flak from people who are running with non-superuser
>>> monitoring tools today, but we already have a bunch of superuser-only
>>> things that monioring tools want, so this doesn't move the needle
>>> particularly far, in my view.
>>
>> That would be a major drawback IMHO, and a step in the wrong direction.
>
> I'm not following.  If we don't want the information to be available to
> everyone then we need to limit it, and right now the only way to do that
> is to restrict it to superuser because we haven't got anything more
> granular.
>
> In other words, it seems like your above response about not wanting this
> to be available to anyone except superusers is counter to this last
> sentence where you seem to be saying we should continue to have the
> information available to everyone and simply document that there's a
> risk there as in the proposed patch.

I don't think we can or should try to hide the current WAL location. At 
least not until we have a monitoring role separate from superuserness.

- Heikki