Re: new heapcheck contrib module

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amul Sul <sulamul(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: new heapcheck contrib module
Date: 2020-09-22 17:55:35
Message-ID: 107F3C9D-2598-4E77-A4D0-0084BD3CBA30@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Sep 21, 2020, at 2:09 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> I think there should be a call to pg_class_aclcheck() here, just like
> the one in pg_prewarm, so that if the superuser does choose to grant
> access, users given access can check tables they anyway have
> permission to access, but not others. Maybe put that in
> check_relation_relkind_and_relam() and rename it. Might want to look
> at the pg_surgery precedent, too.

In the presence of corruption, verify_heapam() reports to the user (in other words, leaks) metadata about the corrupted rows. Reasoning about the attack vectors this creates is hard, but a conservative approach is to assume that an attacker can cause corruption in order to benefit from the leakage, and make sure the leakage does not violate any reasonable security expectations.

Basing the security decision on whether the user has access to read the table seems insufficient, as it ignores row level security. Perhaps that is ok if row level security is not enabled for the table or if the user has been granted BYPASSRLS. There is another problem, though. There is no grantable privilege to read dead rows. In the case of corruption, verify_heapam() may well report metadata about dead rows.

pg_surgery also appears to leak information about dead rows. Owners of tables can probe whether supplied TIDs refer to dead rows. If a table containing sensitive information has rows deleted prior to ownership being transferred, the new owner of the table could probe each page of deleted data to determine something of the content that was there. Information about the number of deleted rows is already available through the pg_stat_* views, but those views don't give such a fine-grained approach to figuring out how large each deleted row was. For a table with fixed content options, the content can sometimes be completely inferred from the length of the row. (Consider a table with a single text column containing either "approved" or "denied".)

But pg_surgery is understood to be a collection of sharp tools only to be used under fairly exceptional conditions. amcheck, on the other hand, is something that feels safer and more reasonable to use on a regular basis, perhaps from a cron job executed by a less trusted user. Forcing the user to be superuser makes it clearer that this feeling of safety is not justified.

I am inclined to just restrict verify_heapam() to superusers and be done. What do you think?


Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2020-09-22 17:58:21 Re: new heapcheck contrib module
Previous Message Etsuro Fujita 2020-09-22 17:20:46 Re: Asynchronous Append on postgres_fdw nodes.