create tablespace fails silently, or succeeds improperly

Hi Dave,

Excerpts from Dave Cramer's message of lun oct 18 12:23:40 -0300 2010:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

postgres@db01:~> less /opt/pg/data/jnj_indexes/PG_VERSION
8.4
postgres@db01:~> /opt/pg91/bin/psql -p 5433
psql (9.0.1)
Type "help" for help.

postgres=# select version();
version
-------------------------------------------------------------------------------------------------------------------
PostgreSQL 9.0.1 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 3.4.6 20060404 (Red Hat 3.4.6-11), 64-bit
(1 row)

postgres=# create TABLESPACE jnj_indexes location '/opt/pg/data/jnj_indexes';
CREATE TABLESPACE

IIRC the reason this works is that the tablespace code now creates a
version-specific subdirectory inside the specified directory. This was
done to help binary upgrades.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Dave Cramer <pg@fastcrypt.com> writes:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory. However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory. Bruce, exactly what is the rationale here?

regards, tom lane

On Mon, Oct 18, 2010 at 11:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dave Cramer <pg@fastcrypt.com> writes:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory.  However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory.  Bruce, exactly what is the rationale here?

OK, it appears there are a few loose ends here then as the
documentation http://www.postgresql.org/docs/9.0/interactive/sql-createtablespace.html
says the directory needs to be empty.

Dave

Dave Cramer wrote:

On Mon, Oct 18, 2010 at 11:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dave Cramer <pg@fastcrypt.com> writes:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory. ?However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory. ?Bruce, exactly what is the rationale here?

OK, it appears there are a few loose ends here then as the
documentation http://www.postgresql.org/docs/9.0/interactive/sql-createtablespace.html
says the directory needs to be empty.

Docs updated to say "should", not "must", be empty. Backpatched to
9.0.X. If we need more change, I can do those too.

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

+ It's impossible for everything to be true. +

On Mon, Oct 18, 2010 at 2:17 PM, Bruce Momjian <bruce@momjian.us> wrote:

Dave Cramer wrote:

On Mon, Oct 18, 2010 at 11:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dave Cramer <pg@fastcrypt.com> writes:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory. ?However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory. ?Bruce, exactly what is the rationale here?

OK, it appears there are a few loose ends here then as the
documentation http://www.postgresql.org/docs/9.0/interactive/sql-createtablespace.html
says the directory needs to be empty.

Docs updated to say "should", not "must", be empty.  Backpatched to
9.0.X.  If we need more change, I can do those too.

Perhaps we should fix the behavior rather than the documentation.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Alvaro Herrera wrote:

Hi Dave,

Excerpts from Dave Cramer's message of lun oct 18 12:23:40 -0300 2010:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

postgres@db01:~> less /opt/pg/data/jnj_indexes/PG_VERSION
8.4
postgres@db01:~> /opt/pg91/bin/psql -p 5433
psql (9.0.1)
Type "help" for help.

postgres=# select version();
version
-------------------------------------------------------------------------------------------------------------------
PostgreSQL 9.0.1 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 3.4.6 20060404 (Red Hat 3.4.6-11), 64-bit
(1 row)

postgres=# create TABLESPACE jnj_indexes location '/opt/pg/data/jnj_indexes';
CREATE TABLESPACE

IIRC the reason this works is that the tablespace code now creates a
version-specific subdirectory inside the specified directory. This was
done to help binary upgrades.

Right, the directory is catalog-version named, which was done to allow
for pg_upgrade to work for alpha/beta upgrades (pretty cool). The case
above happened because 8.4 still has data in that tablespace.
pg_upgrade does supply a script to delete old data files, but it was not
used in the case above.

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

+ It's impossible for everything to be true. +

Tom Lane wrote:

Dave Cramer <pg@fastcrypt.com> writes:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory. However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory. Bruce, exactly what is the rationale here?

Well, the symbolic link from data/pg_tblspc points to the top directory,
not to the catalog-version-named subdirectory. This was done for
several reasons, particularly so the directory pointed to by the symlink
would be exactly the same as that specified by CREATE TABLESPACE, for
code clarity.

Tom, is there a particular permission change you were wondering about?

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

+ It's impossible for everything to be true. +

Robert Haas wrote:

On Mon, Oct 18, 2010 at 2:17 PM, Bruce Momjian <bruce@momjian.us> wrote:

Dave Cramer wrote:

On Mon, Oct 18, 2010 at 11:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dave Cramer <pg@fastcrypt.com> writes:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory. ?However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory. ?Bruce, exactly what is the rationale here?

OK, it appears there are a few loose ends here then as the
documentation http://www.postgresql.org/docs/9.0/interactive/sql-createtablespace.html
says the directory needs to be empty.

Docs updated to say "should", not "must", be empty. ?Backpatched to
9.0.X. ?If we need more change, I can do those too.

Perhaps we should fix the behavior rather than the documentation.

We can't fix the behavior because we have to allow an old cluster to
keep its tablespace files during a pg_upgrade. They are given a script
to delete those files later, if they want.

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

+ It's impossible for everything to be true. +

On Mon, Oct 18, 2010 at 2:20 PM, Bruce Momjian <bruce@momjian.us> wrote:

Alvaro Herrera wrote:

Hi Dave,

Excerpts from Dave Cramer's message of lun oct 18 12:23:40 -0300 2010:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

postgres@db01:~> less /opt/pg/data/jnj_indexes/PG_VERSION
8.4
postgres@db01:~> /opt/pg91/bin/psql -p 5433
psql (9.0.1)
Type "help" for help.

postgres=# select version();
                                                      version
-------------------------------------------------------------------------------------------------------------------
 PostgreSQL 9.0.1 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 3.4.6 20060404 (Red Hat 3.4.6-11), 64-bit
(1 row)

postgres=# create TABLESPACE jnj_indexes location '/opt/pg/data/jnj_indexes';
CREATE TABLESPACE

IIRC the reason this works is that the tablespace code now creates a
version-specific subdirectory inside the specified directory.  This was
done to help binary upgrades.

Right, the directory is catalog-version named, which was done to allow
for pg_upgrade to work for alpha/beta upgrades (pretty cool).  The case
above happened because 8.4 still has data in that tablespace.
pg_upgrade does supply a script to delete old data files, but it was not
used in the case above.

right that's because I did not use pg_upgrade. I was manually running
create tablespace.

Dave

Dave Cramer wrote:

On Mon, Oct 18, 2010 at 2:20 PM, Bruce Momjian <bruce@momjian.us> wrote:

Alvaro Herrera wrote:

Hi Dave,

Excerpts from Dave Cramer's message of lun oct 18 12:23:40 -0300 2010:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

postgres@db01:~> less /opt/pg/data/jnj_indexes/PG_VERSION
8.4
postgres@db01:~> /opt/pg91/bin/psql -p 5433
psql (9.0.1)
Type "help" for help.

postgres=# select version();
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? version
-------------------------------------------------------------------------------------------------------------------
?PostgreSQL 9.0.1 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 3.4.6 20060404 (Red Hat 3.4.6-11), 64-bit
(1 row)

postgres=# create TABLESPACE jnj_indexes location '/opt/pg/data/jnj_indexes';
CREATE TABLESPACE

IIRC the reason this works is that the tablespace code now creates a
version-specific subdirectory inside the specified directory. ?This was
done to help binary upgrades.

Right, the directory is catalog-version named, which was done to allow
for pg_upgrade to work for alpha/beta upgrades (pretty cool). ?The case
above happened because 8.4 still has data in that tablespace.
pg_upgrade does supply a script to delete old data files, but it was not
used in the case above.

right that's because I did not use pg_upgrade. I was manually running
create tablespace.

OK, so you were sharing the tablespace with old and new clusters. You
are right that in the past that would not have been possible because
PG_VERSION would have conflicted, but it is now possible with all new
releases because of the catalog-version-named subdirectory. That seems
like I a feature, I guess.

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

+ It's impossible for everything to be true. +

On Mon, Oct 18, 2010 at 2:39 PM, Bruce Momjian <bruce@momjian.us> wrote:

Dave Cramer wrote:

On Mon, Oct 18, 2010 at 2:20 PM, Bruce Momjian <bruce@momjian.us> wrote:

Alvaro Herrera wrote:

Hi Dave,

Excerpts from Dave Cramer's message of lun oct 18 12:23:40 -0300 2010:

as seen below create tablespace does not throw an error or appear to
do anything other than register the tablespace.

postgres@db01:~> less /opt/pg/data/jnj_indexes/PG_VERSION
8.4
postgres@db01:~> /opt/pg91/bin/psql -p 5433
psql (9.0.1)
Type "help" for help.

postgres=# select version();
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? version
-------------------------------------------------------------------------------------------------------------------
?PostgreSQL 9.0.1 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 3.4.6 20060404 (Red Hat 3.4.6-11), 64-bit
(1 row)

postgres=# create TABLESPACE jnj_indexes location '/opt/pg/data/jnj_indexes';
CREATE TABLESPACE

IIRC the reason this works is that the tablespace code now creates a
version-specific subdirectory inside the specified directory. ?This was
done to help binary upgrades.

Right, the directory is catalog-version named, which was done to allow
for pg_upgrade to work for alpha/beta upgrades (pretty cool). ?The case
above happened because 8.4 still has data in that tablespace.
pg_upgrade does supply a script to delete old data files, but it was not
used in the case above.

right that's because I did not use pg_upgrade. I was manually running
create tablespace.

OK, so you were sharing the tablespace with old and new clusters.  You
are right that in the past that would not have been possible because
PG_VERSION would have conflicted, but it is now possible with all new
releases because of the catalog-version-named subdirectory.  That seems
like I a feature, I guess.

Sounds unintended. As it turns out I was expecting it to fail and was
surprised when it succeeded.

Dave

Dave Cramer wrote:

postgres=# select version();
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? version
-------------------------------------------------------------------------------------------------------------------
?PostgreSQL 9.0.1 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 3.4.6 20060404 (Red Hat 3.4.6-11), 64-bit
(1 row)

postgres=# create TABLESPACE jnj_indexes location '/opt/pg/data/jnj_indexes';
CREATE TABLESPACE

IIRC the reason this works is that the tablespace code now creates a
version-specific subdirectory inside the specified directory. ?This was
done to help binary upgrades.

Right, the directory is catalog-version named, which was done to allow
for pg_upgrade to work for alpha/beta upgrades (pretty cool). ?The case
above happened because 8.4 still has data in that tablespace.
pg_upgrade does supply a script to delete old data files, but it was not
used in the case above.

right that's because I did not use pg_upgrade. I was manually running
create tablespace.

OK, so you were sharing the tablespace with old and new clusters. ?You
are right that in the past that would not have been possible because
PG_VERSION would have conflicted, but it is now possible with all new
releases because of the catalog-version-named subdirectory. ?That seems
like I a feature, I guess.

Sounds unintended. As it turns out I was expecting it to fail and was
surprised when it succeeded.

Well, it was intended, or rather required by pg_upgrade.

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

+ It's impossible for everything to be true. +

Dave Cramer wrote:

IIRC the reason this works is that the tablespace code now creates a
version-specific subdirectory inside the specified directory. ?This was
done to help binary upgrades.

Right, the directory is catalog-version named, which was done to allow
for pg_upgrade to work for alpha/beta upgrades (pretty cool). ?The case
above happened because 8.4 still has data in that tablespace.
pg_upgrade does supply a script to delete old data files, but it was not
used in the case above.

right that's because I did not use pg_upgrade. I was manually running
create tablespace.

OK, so you were sharing the tablespace with old and new clusters. ?You
are right that in the past that would not have been possible because
PG_VERSION would have conflicted, but it is now possible with all new
releases because of the catalog-version-named subdirectory. ?That seems
like I a feature, I guess.

Sounds unintended. As it turns out I was expecting it to fail and was
surprised when it succeeded.

Well, it was intended, or rather required by pg_upgrade.

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

+ It's impossible for everything to be true. +

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory. However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory. Bruce, exactly what is the rationale here?

Tom, is there a particular permission change you were wondering about?

In testing it, I noticed that the permissions of the parent directory
(the one named in LOCATION) were changed to 0700, which is not where
I'd had them set before.  I'm not sure that that is still necessary
or reasonable.  We should make the subdirectory (eg PG_9.1_201010151)
mode 0700, but I am dubious that it's still sensible to require
ownership on the parent, much less to change its permissions.  The
argument for locking down the parent seems to be to prevent a bad guy
from renaming the subdirectory out of the way and substituting his own
--- but if we're trying to prevent that type of attack, then we have to
insist on restrictive permissions all the way up the path, not just on
the immediate parent.  And we do not try to prevent such attacks on the
$PGDATA directory itself, so why should we do it on a tablespace?

So basically I think this requires some re-thinking that it didn't get.
Perhaps we should just be satisfied if we are able to create the
subdirectory as owned by postgres, and leave it to the user as to
whether the parent directory is a secure place to put the subdirectory.

regards, tom lane

Bruce Momjian <bruce@momjian.us> writes:

Robert Haas wrote:

Perhaps we should fix the behavior rather than the documentation.

We can't fix the behavior because we have to allow an old cluster to
keep its tablespace files during a pg_upgrade. They are given a script
to delete those files later, if they want.

The new *layout* of the files may be forced by pg_upgrade
considerations, but I don't think that proves much of anything about
ownership and permissions settings --- especially not the fact that we
are now enforcing settings that were designed for the data directory
itself on what is effectively one level up from that.

regards, tom lane

On Mon, Oct 18, 2010 at 3:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

Robert Haas wrote:

Perhaps we should fix the behavior rather than the documentation.

We can't fix the behavior because we have to allow an old cluster to
keep its tablespace files during a pg_upgrade.  They are given a script
to delete those files later, if they want.

The new *layout* of the files may be forced by pg_upgrade
considerations, but I don't think that proves much of anything about
ownership and permissions settings --- especially not the fact that we
are now enforcing settings that were designed for the data directory
itself on what is effectively one level up from that.

I haven't yet been convinced we need or want to relax the rule about
the directory being empty.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

I suspect this behavior is partially intentional, because tablespace
creation now involves an extra level of subdirectory. However, it's
not clear to me why CREATE TABLESPACE is still changing the permissions
on the parent directory. Bruce, exactly what is the rationale here?

Tom, is there a particular permission change you were wondering about?

In testing it, I noticed that the permissions of the parent directory
(the one named in LOCATION) were changed to 0700, which is not where
I'd had them set before.  I'm not sure that that is still necessary
or reasonable.  We should make the subdirectory (eg PG_9.1_201010151)
mode 0700, but I am dubious that it's still sensible to require
ownership on the parent, much less to change its permissions.  The
argument for locking down the parent seems to be to prevent a bad guy
from renaming the subdirectory out of the way and substituting his own
--- but if we're trying to prevent that type of attack, then we have to
insist on restrictive permissions all the way up the path, not just on
the immediate parent.  And we do not try to prevent such attacks on the
$PGDATA directory itself, so why should we do it on a tablespace?

So basically I think this requires some re-thinking that it didn't get.
Perhaps we should just be satisfied if we are able to create the
subdirectory as owned by postgres, and leave it to the user as to
whether the parent directory is a secure place to put the subdirectory.

Good point. I did not think through the security restrictions of the
parent, but because we were symlinking to it, I thought we should lock
it down. I see no problem in relaxing the restrictions as you suggest.

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

+ It's impossible for everything to be true. +

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Robert Haas wrote:

Perhaps we should fix the behavior rather than the documentation.

We can't fix the behavior because we have to allow an old cluster to
keep its tablespace files during a pg_upgrade. They are given a script
to delete those files later, if they want.

The new *layout* of the files may be forced by pg_upgrade
considerations, but I don't think that proves much of anything about
ownership and permissions settings --- especially not the fact that we
are now enforcing settings that were designed for the data directory
itself on what is effectively one level up from that.

Agreed. I was speaking only about the ability to have something in that
top-level directory when you are creating the tablespace.

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

+ It's impossible for everything to be true. +

Robert Haas wrote:

On Mon, Oct 18, 2010 at 3:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

Robert Haas wrote:

Perhaps we should fix the behavior rather than the documentation.

We can't fix the behavior because we have to allow an old cluster to
keep its tablespace files during a pg_upgrade. ?They are given a script
to delete those files later, if they want.

The new *layout* of the files may be forced by pg_upgrade
considerations, but I don't think that proves much of anything about
ownership and permissions settings --- especially not the fact that we
are now enforcing settings that were designed for the data directory
itself on what is effectively one level up from that.

I haven't yet been convinced we need or want to relax the rule about
the directory being empty.

Uh, how would pg_upgrade work then? It would require renaming the
top-level tablespace directory, which might require root permissions.

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

+ It's impossible for everything to be true. +