pg_auto_failover
pg_auto_failover copied to clipboard
Prevent unauthorized access from localhost
Right now the only way to run pg_auto_ctl is to completely trust the node it runs on, because any connection made from "localhost" is allowed to authenticate using "trust". Even when usig --skip-pg-hba. This is because we do not change the default --auth argument when we run initdb. https://www.postgresql.org/docs/12/app-initdb.html
https://github.com/citusdata/pg_auto_failover/blob/7986d6a9e07b6dc925f7bffacba52b0a1eab27a2/src/bin/pg_autoctl/pgctl.c#L755
I tried changing it to --auth=reject --auth-local=trust, but this causes connection errors on the pg_auto_ctl side.
It would be nice to use UNIX socket connections from pg_auto_ctl to postgres instead of TCP connections over localhost. This way we can make sure that only the postgres user (or whatever user used pg_auto_ctl create can connect to postgres without auth).
One idea to fix this is to allow specifying --pghost=/tmp. This currently does not work for multiple reasons:
--pghostis not implemented oncreate monitor- I think we should set
unix_socket_directories. - BONUS: We should change the
initdbcommand to not create the openpg_hba.conf
I'm slowly ramping up on this, so please let me know if the following makes sense/correct or not:
1)
Right now the only way to run pg_auto_ctl is to completely trust the node it runs on, because any connection made from "localhost" is allowed to authenticate using "trust"
I cannot understand this very well. When I debug/read-code, what I see is both on the monitor or Postgres data nodes, pg_auto_ctl connects to local server over unix domain socket, not through localhost
See pg_setup_get_local_connection_string (): https://github.com/citusdata/pg_auto_failover/blob/7986d6a9e07b6dc925f7bffacba52b0a1eab27a2/src/bin/pg_autoctl/pgsetup.c#L715-L797
And, when I print the connection strings to the logs, I see the following, which are all unix domain sockets:
connectionString: port=5432 dbname=template1 host=/tmp
connectionString: port=5432 dbname=pg_auto_failover host=/tmp user=autoctl_node
connectionString: port=9700 dbname=postgres host=/tmp
connectionString: port=9701 dbname=postgres host=/tmp
2)
I tried changing it to --auth=reject --auth-local=trust, but this causes connection errors on the pg_auto_ctl side.
I also cannot understand the reason for adding --auth=reject. Aren't we already updating pg_hba.conf for remote accesses? Did you mean --auth-host which is to control connections through localhost?
3)
My high level question: Is this the only security problem if an unauthorized user connects to the machine via ssh which runs pg_auto_ctl?
For example, I couldn't see anything special for protecting the file with KEEPER_STATE_FILENAME name. What if someone deletes the file?
Overall, do we need anything further to protect the databases and the HA system?
My Proposal
I think we should do 3 things:
-
My understanding is that Postgres'
unix_socket_permissionssetting actually aims to improve this situation. We want to prevent unauthorized OS users connecting via unix domain socket. We should probably set it to0770(only user and group) or maybe even0700(only the user that initiated the database can connect). With that, we prevent unauthorized users connecting via unix domain sockets. I verified that locally. -
We should add a rule to pg_hba.conf, to prevent unauthorized TCP connections over
localhost, as we do for remote accesses. -
We should go over the code, and make sure that all local connections are always done over unix domain socket, not through tcp
localhost
To answer your questions:
- When I run the following command:
make install -j9 && pg_autoctl create monitor --pgdata monitor --ssl-self-signed --run --auth trust -vvv
There are messages like this (with host=localhost):
Connecting to "port=5432 dbname=pg_auto_failover host=localhost user=autoctl_node"
- Yes,
--auth-host=reject --auth-local=trustresult in the same inpg_hba.conffile. I agree that's more as expected.
Woops, pressed send too fast:
3. Maybe not, but it's one an operator cannot fix. File system permissions an operator can change. This is not the case when Postgres is listening with trust on localhost and we require this. Then an operator cannot tighten this security down and effectively any user has postgres user permisions.
Regarding your specific point about KEEPER_STATE_FILENAME. This is stored in XDG_DATA_HOME, which by default is ~/.local/share. Assuming the operator runs pg_autoctl as the postgres user. This would be in /home/postgres/.local/share, which is probably at least not writable by anyone.
About your proposal:
- I agree about
unix_socket_permissions, I think0700 - This can be done by passing
--auth-host=reject --auth-local=trusttoinitdb. - I agree this should be the default, but possibly some people will still want to connect over the network. So, I think it's good to keep the
--pghostflag (and add it tocreate monitor).
There are messages like this (with host=localhost):
For future reference: My installation/configuration might be different, but I couldn't see localhost in the output on my Mac:
MacBook-Pro-Onders:pg_auto_failover onderkalaci$ pg_autoctl create monitor --pgdata /Users/onderkalaci/Documents/pg_auto_failover/monitor --ssl-self-signed --run --auth trust -vvv
12:27:54 TRACE pgsetup.c:1459: pgsetup_validate_ssl_settings
12:27:54 INFO pgsetup.c:1561: Using default --ssl-mode "require"
12:27:54 INFO pgsetup.c:1566: Using --ssl-self-signed: pg_autoctl will create self-signed certificates, allowing for encrypted network traffic
12:27:54 WARN pgsetup.c:1569: Self-signed certificates provide protection against eavesdropping; this setup does NOT protect against Man-In-The-Middle attacks nor Impersonation attacks.
12:27:54 WARN pgsetup.c:1571: See https://www.postgresql.org/docs/current/libpq-ssl.html for details
12:27:54 TRACE config.c:165: SetConfigFilePath: "/Users/onderkalaci/.config/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.cfg"
12:27:54 TRACE config.c:193: SetStateFilePath: "/Users/onderkalaci/.local/share/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.state"
12:27:54 TRACE config.c:208: SetKeeperStateFilePath: "/Users/onderkalaci/.local/share/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.init"
12:27:54 TRACE config.c:234: SetPidFilePath: "/tmp/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.pid"
12:27:54 DEBUG monitor_config.c:252: Reading configuration from /Users/onderkalaci/.config/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.cfg
12:27:54 TRACE ini_file.c:109: pg_autoctl.role = monitor
12:27:54 TRACE ini_file.c:109: pg_autoctl.nodename = 192.168.2.6
12:27:54 TRACE ini_file.c:109: postgresql.pgdata = /Users/onderkalaci/Documents/pg_auto_failover/monitor
12:27:54 TRACE ini_file.c:109: postgresql.pg_ctl = /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_ctl
12:27:54 TRACE ini_file.c:109: postgresql.username = autoctl_node
12:27:54 TRACE ini_file.c:109: postgresql.dbname = pg_auto_failover
12:27:54 TRACE ini_file.c:109: postgresql.host = /tmp
12:27:54 TRACE ini_file.c:109: postgresql.port = 5432
12:27:54 TRACE ini_file.c:109: postgresql.listen_addresses = *
12:27:54 TRACE ini_file.c:109: postgresql.auth_method = trust
12:27:54 TRACE ini_file.c:109: ssl.sslmode = require
12:27:54 TRACE ini_file.c:109: ssl.active = 1
12:27:54 TRACE ini_file.c:109: ssl.cert_file = /Users/onderkalaci/Documents/pg_auto_failover/monitor/server.crt
12:27:54 TRACE ini_file.c:109: ssl.key_file = /Users/onderkalaci/Documents/pg_auto_failover/monitor/server.key
12:27:54 DEBUG pgctl.c:118: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_controldata /Users/onderkalaci/Documents/pg_auto_failover/monitor
12:27:54 DEBUG pgctl.c:739: pg_controldata: fatal: could not open file "/Users/onderkalaci/Documents/pg_auto_failover/monitor/global/pg_control" for reading: No such file or directory
12:27:54 DEBUG pgctl.c:155: Failed to run "/Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_controldata" on "/Users/onderkalaci/Documents/pg_auto_failover/monitor", see above for details
12:27:54 DEBUG pgctl.c:118: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_controldata /Users/onderkalaci/Documents/pg_auto_failover/monitor
12:27:54 DEBUG pgctl.c:739: pg_controldata: fatal: could not open file "/Users/onderkalaci/Documents/pg_auto_failover/monitor/global/pg_control" for reading: No such file or directory
12:27:54 DEBUG pgctl.c:155: Failed to run "/Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_controldata" on "/Users/onderkalaci/Documents/pg_auto_failover/monitor", see above for details
12:27:54 TRACE monitor_config.c:306: monitor_config_write_file "/Users/onderkalaci/.config/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.cfg"
12:27:54 ERROR pgsetup.c:795: connectionString: port=5432 dbname=pg_auto_failover host=/tmp user=autoctl_node
12:27:54 TRACE monitor.c:96: monitor_init: port=5432 dbname=pg_auto_failover host=/tmp user=autoctl_node
12:27:58 INFO pgctl.c:758: Initialising a PostgreSQL cluster at "/Users/onderkalaci/Documents/pg_auto_failover/monitor"
12:27:58 DEBUG pgctl.c:759: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_ctl initdb -s -D /Users/onderkalaci/Documents/pg_auto_failover/monitor [0]
12:27:58 TRACE monitor_config.c:306: monitor_config_write_file "/Users/onderkalaci/.config/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.cfg"
12:27:58 INFO pgctl.c:1746: /usr/local/opt/[email protected]/bin/openssl req -new -x509 -days 365 -nodes -text -out /Users/onderkalaci/Documents/pg_auto_failover/monitor/server.crt -keyout /Users/onderkalaci/Documents/pg_auto_failover/monitor/server.key -subj "/CN=192.168.2.6"
12:27:58 DEBUG pgctl.c:739: Generating a RSA private key
12:27:58 DEBUG pgctl.c:739: ..................................+++++
12:27:58 DEBUG pgctl.c:739: .................+++++
12:27:58 DEBUG pgctl.c:739: writing new private key to '/Users/onderkalaci/Documents/pg_auto_failover/monitor/server.key'
12:27:58 DEBUG pgctl.c:739: -----
12:27:58 TRACE monitor_config.c:306: monitor_config_write_file "/Users/onderkalaci/.config/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.cfg"
12:27:58 DEBUG pgctl.c:389: Configuration file "/Users/onderkalaci/Documents/pg_auto_failover/monitor/postgresql-auto-failover.conf" doesn't exists yet, creating with content:
# Settings by pg_auto_failover
shared_preload_libraries = 'pgautofailover'
listen_addresses = '*'
port = 5432
log_destination = stderr
logging_collector = on
log_directory = log
log_min_messages = info
log_connections = off
log_disconnections = off
log_lock_waits = on
ssl = on
ssl_cert_file = '/Users/onderkalaci/Documents/pg_auto_failover/monitor/server.crt'
ssl_key_file = '/Users/onderkalaci/Documents/pg_auto_failover/monitor/server.key'
ssl_ciphers = 'ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384'
12:27:58 DEBUG pgctl.c:300: Adding include 'postgresql-auto-failover.conf' to "/Users/onderkalaci/Documents/pg_auto_failover/monitor/postgresql.conf"
12:27:58 INFO pgctl.c:847: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_ctl --pgdata /Users/onderkalaci/Documents/pg_auto_failover/monitor --options "-p 5432" --options "-h *" --wait start
12:27:58 DEBUG pgctl.c:118: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_controldata /Users/onderkalaci/Documents/pg_auto_failover/monitor
12:27:58 DEBUG pgsetup.c:173: Found PostgreSQL system 6830726616286946078 at "/Users/onderkalaci/Documents/pg_auto_failover/monitor", version 1201, catalog version 201909212
12:27:58 TRACE pgsetup.c:641: read_pg_pidfile: pid 86824, port 5432, host /tmp, status "ready"
12:27:58 TRACE pgsetup.c:932: pg_setup_is_ready: ready
12:27:58 ERROR pgsetup.c:795: connectionString: port=5432 dbname=postgres host=/tmp
12:27:58 DEBUG pgsql.c:151: Connecting to "port=5432 dbname=postgres host=/tmp"
12:27:58 DEBUG pgsql.c:1461: Running command on Postgres: CREATE USER "autoctl" WITH LOGIN;
12:27:58 DEBUG pgsql.c:1280: Running command on Postgres: CREATE DATABASE "pg_auto_failover" WITH OWNER "autoctl";
12:27:59 DEBUG pgsql.c:128: Disconnecting from "port=5432 dbname=postgres host=/tmp"
12:27:59 ERROR pgsetup.c:795: connectionString: port=5432 dbname=pg_auto_failover host=/tmp
12:27:59 DEBUG pgsql.c:151: Connecting to "port=5432 dbname=pg_auto_failover host=/tmp"
12:27:59 DEBUG pgsql.c:1350: Running command on Postgres: CREATE EXTENSION "pgautofailover";
12:27:59 DEBUG pghba.c:327: HBA: adding CIDR from hostname "192.168.2.6"
12:27:59 DEBUG pghba.c:328: HBA: local ip address: 192.168.2.6
12:27:59 DEBUG pghba.c:329: HBA: CIDR address to open: 192.168.2.0/24
12:27:59 INFO pghba.c:330: Granting connection privileges on 192.168.2.0/24
12:27:59 DEBUG pgsql.c:385: SELECT current_setting($1);
12:27:59 DEBUG pgsql.c:409: 'hba_file'
12:27:59 DEBUG pghba.c:112: Ensuring the HBA file "/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_hba.conf" contains the line: hostssl "pg_auto_failover" "autoctl_node" 192.168.2.0/24 trust
12:27:59 DEBUG pghba.c:169: Wrote new /Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_hba.conf
12:27:59 DEBUG pgsql.c:385: SELECT pg_reload_conf();
12:27:59 DEBUG pgsql.c:128: Disconnecting from "port=5432 dbname=pg_auto_failover host=/tmp"
12:27:59 INFO monitor_pg_init.c:251: Your pg_auto_failover monitor instance is now ready on port 5432.
12:27:59 INFO cli_create_drop_node.c:766: Monitor has been succesfully initialized.
12:27:59 TRACE service.c:37: monitor_service_init
12:27:59 TRACE service.c:151: create_pidfile(86801): "/tmp/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.pid"
12:27:59 DEBUG pgctl.c:1161: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_ctl status -D /Users/onderkalaci/Documents/pg_auto_failover/monitor [0]
12:27:59 TRACE monitor_config.c:306: monitor_config_write_file "/Users/onderkalaci/.config/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.cfg"
12:27:59 INFO pgctl.c:1746: /usr/local/opt/[email protected]/bin/openssl req -new -x509 -days 365 -nodes -text -out /Users/onderkalaci/Documents/pg_auto_failover/monitor/server.crt -keyout /Users/onderkalaci/Documents/pg_auto_failover/monitor/server.key -subj "/CN=192.168.2.6"
12:27:59 DEBUG pgctl.c:739: Generating a RSA private key
12:27:59 DEBUG pgctl.c:739: ............+++++
12:27:59 DEBUG pgctl.c:739: ................+++++
12:27:59 DEBUG pgctl.c:739: writing new private key to '/Users/onderkalaci/Documents/pg_auto_failover/monitor/server.key'
12:27:59 DEBUG pgctl.c:739: -----
12:27:59 TRACE monitor_config.c:306: monitor_config_write_file "/Users/onderkalaci/.config/pg_autoctl/Users/onderkalaci/Documents/pg_auto_failover/monitor/pg_autoctl.cfg"
12:27:59 DEBUG pgctl.c:376: Default settings file "/Users/onderkalaci/Documents/pg_auto_failover/monitor/postgresql-auto-failover.conf" exists
12:27:59 DEBUG pgctl.c:293: include 'postgresql-auto-failover.conf' found in "/Users/onderkalaci/Documents/pg_auto_failover/monitor/postgresql.conf"
12:27:59 DEBUG pgctl.c:118: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_controldata /Users/onderkalaci/Documents/pg_auto_failover/monitor
12:27:59 DEBUG pgsetup.c:173: Found PostgreSQL system 6830726616286946078 at "/Users/onderkalaci/Documents/pg_auto_failover/monitor", version 1201, catalog version 201909212
12:27:59 TRACE pgsetup.c:641: read_pg_pidfile: pid 86824, port 5432, host /tmp, status "ready"
12:27:59 TRACE pgsetup.c:932: pg_setup_is_ready: ready
12:27:59 ERROR pgsetup.c:795: connectionString: port=5432 dbname=template1 host=/tmp
12:27:59 DEBUG pgsql.c:151: Connecting to "port=5432 dbname=template1 host=/tmp"
12:27:59 DEBUG pgsql.c:385: SELECT pg_reload_conf();
12:27:59 INFO monitor_service.c:104: pg_auto_failover monitor is ready at postgres://[email protected]:5432/pg_auto_failover?sslmode=require
12:27:59 INFO monitor_service.c:107: Contacting the monitor to LISTEN to its events.
12:27:59 DEBUG pgsql.c:151: Connecting to "port=5432 dbname=pg_auto_failover host=/tmp user=autoctl_node"
12:27:59 DEBUG pgctl.c:1161: /Users/onderkalaci/Documents/citus_code/pgenv/pgsql/bin/pg_ctl status -D /Users/onderkalaci/Documents/pg_auto_failover/monitor [0]
I figured out why I was getting localhost in there. It was because I had export PGHOST=localhost in my ~/.bashrc file. Without this pg_autoctl worked fine when doing --auth-host=reject --auth-local=trust.
So I think there's a few things we should change in that case to sum up:
- Set
unix_socket_permissionsto0700or0770by default - Add
--pghostflag tocreate monitorso you can override thePGHOSTvariable. - Change our
initdbto not set up trust onlocalhostby default (but still do that for UNIX sockets). - BONUS: If
PGHOST/--pghostdoes not start with a/(to indicate UNIX socket dir), then allow localhost access by default.
Add --pghost flag to create monitor so you can override the PGHOST variable.
We seem to have create monitor --nodename parameter. Do you have something different in mind @JelteF?
BONUS: If PGHOST/--pghost does not start with a / (to indicate UNIX socket dir), then allow localhost access by default.
One caveat here is that on the monitor, it is not possible to set --nodename = /unix/path when --no-ssl or --ssl-self-signed is chosen. I think we have a strong dependency to have a hostname related to certificate/ssl setup.
For no-ssl case, we get the following after create monitor:
19:09:45 16348 ERROR pgctl.c:981: Failed to start PostgreSQL. pg_ctl start returned: 1
19:09:45 16348 ERROR pgctl.c:985: waiting for server to start....2020-05-25 19:09:45.807 CEST [16365] FATAL: could not load server certificate file "/Users/onderkalaci/Documents/pg_auto_failover/monitor/server.crt": No such file or directory
For --ssl-self-signed, we get the following after create monitor:
19:13:38 16382 ERROR pgctl.c:785: Generating a RSA private key
19:13:38 16382 ERROR pgctl.c:785: ...............+++++
19:13:38 16382 ERROR pgctl.c:785: ................................................+++++
19:13:38 16382 ERROR pgctl.c:785: writing new private key to '/Users/onderkalaci/Documents/pg_auto_failover/monitor/server.key'
19:13:38 16382 ERROR pgctl.c:785: -----
19:13:38 16382 ERROR pgctl.c:785: req: No value provided for Subject Attribute CN, skipped
19:13:38 16382 ERROR pgctl.c:785: req: Hit end of string before finding the equals.
19:13:38 16382 ERROR pgctl.c:785: problems making Certificate Request
19:13:38 16382 ERROR pgctl.c:1948: openssl failed with return code: 1
BONUS: If PGHOST/--pghost does not start with a / (to indicate UNIX socket dir), then allow localhost access by default.
I think instead of this hack, we could allow pg_autoctl create to accept --auth-host and auth-local parameters and pass it to initdb. And, the defaults are --auth-local=trust and --auth-host=reject
What do you think @JelteF ?
We seem to have
create monitor --nodenameparameter. Do you have something different in mind @JelteF?
For create postgres we have both a --nodename and a --pghost parameter. --nodename is the hostname that is connected to from other servers and --pghost is the hostname that pg_autoctl uses to connect to the local postgres. So it doesn't really make sense to have /unix/path as a --nodename, but it makes sense as --pghost. The --pghost flag is missing on create monitor.
One caveat here is that on the monitor, it is not possible to set
--nodename = /unix/pathwhen--no-sslor--ssl-self-signedis chosen. I think we have a strong dependency to have a hostname related to certificate/ssl setup.
Yes, I noticed this too in my initial investigation. Then @DimCitus told me the fact I mentioned above about the two different flags for create postgres.
I think instead of this hack, we could allow
pg_autoctl createto accept--auth-hostandauth-localparameters and pass it toinitdb. And, the defaults are--auth-local=trustand--auth-host=reject
I think this idea is definitely in the right direction and better than the "magic" I proposed. We should think a bit about how to make it work nicely with the flags we already have, i.e. --auth and --skip-pg-hba. I think what would be nice is:
- We add the
--auth-hostand--auth-localflags. - If they are specified we use those values
- If
--auth-localis not specified we usetrust - If
--auth-hostis not specified we use: a. If--authis specified we use it's value. b. If--skip-pg-hbais specified we usereject.
- We add the
--auth-hostand--auth-localflags.- If they are specified we use those values
- If
--auth-localis not specified we usetrust- If
--auth-hostis not specified we use: a. If--authis specified we use it's value. b. If--skip-pg-hbais specified we usereject.
I'm not too sure about this. I think it would be better to have a strong default and have users edit the HBA file if they want to change our defaults. After all when adding application nodes editing the HBA manually is already necessary.
What about using pg_ctl initdb --auth-local trust --auth-host reject and then edit the HBA normally per the --auth and --skip-pg-hba rules? I think that covers everything that has been said here, right?
I understand what you mean, which is why I put BONUS there originally. The main problem it causes is that when doing create monitor with PGHOST=localhost without --auth-host=trust then the create monitor command fails even when not specifying --run.
To fix it you have to:
- modify
pg_hba.conf - reload postgres using
pg_ctl reload - run
create monitoragain
That seems a bit annoying.
If you are opposed to adding more flags, I think that makes sense. We can also leave those out and simply always do --auth-local trust and then specify --auth-host the same as --auth (or reject if --skip-pg-hba is specified).
That would solve most cases and I don't think you gain much security from --auth-host reject if you don't specify --skip-pg-hba too. Because then you can still connect to the monitor using it's public IP, might as well make it easy for the user to connect to localhost too.
After a chat with Onder, we are coming to the following conclusions:
- the monitor is now an embedded database, not expected to be used or even usable for anything else than
pg_autoctlneeds ; we don't need to obey PGHOST nor--pghostand may use our own Unix Socket directory and connect through there always, - the default HBA given by
pg_ctl initdbcontains problematic lines: with--auth-host trustwe allow too many connection, with--auth-host rejectonly SSL connections can be made because we only even append rules to the file, and we have a reject line that is hit first.
So what I think we should do is the following:
- ignore PGHOST and skip implementing
--pghoston the monitor, - remove and replace the
pg_hba.conffile wholesale afterpg_ctl initdbto have our own lines only (local all all trust, and then the usual autoctl_node from the local network) ; on the monitor only, - see about changing
unix_socket_directoriesto use either a XDG_DATA or XDG_RUNTIME relative path, as we do for our state and pid files, - see about changing
unix_socket_permissionsto0700maybe - avoid any changes in behaviour on the “keeper” side of things (
pg_autoctl create postgres)
We could also review our --auth option parsing on the keeper and have either only that one or potentially both --auth-local and --auth-host and forward those to pg_ctl initdb. That should be a separate PR though.
- ignore PGHOST and skip implementing
--pghoston the monitor,
Fine by me
- remove and replace the
pg_hba.conffile wholesale afterpg_ctl initdbto have our own lines only (local all all trust, and then the usual autoctl_node from the local network) ; on the monitor only,
Again fine. One thing to keep in mind though is that that will also remove the huge comment at the start of pg_hba.conf. Which I think is quite useful. So it would be nice to do this in a way that we do not lose it. Some options I can think of:
- Also write it in our hardcoded
pg_hba.conf(easy, but we have to update the text manually if postgres changes it) - Remove only the last couple of lines from the file and then append our own
locallines. (seems like this might be a bit of work)
- see about changing
unix_socket_directoriesto use either a XDG_DATA or XDG_RUNTIME relative path, as we do for our state and pid files,
I think it makes sense to append to unix_socket_directories, but it seems nice to also put the socket in the default directory.
- see about changing
unix_socket_permissionsto0700maybe
I think this is more important than unix_socket_directories TBH. A trust socket should not be accessible by all system users by default.
- avoid any changes in behaviour on the “keeper” side of things (
pg_autoctl create postgres)
I think we should definitely have some sane defaults for pg_autoctl create postgres too. At a minimum we should not add host all localhost trust lines to pg_hba.conf when doing --skip-pg-hba or --auth md5. (and also add unix_socket_permissions=0700)
I don't see a reason why we should not do the thing I describe above on the worker:
simply always do
--auth-local trustand then specify--auth-hostthe same as--auth(orrejectif--skip-pg-hbais specified).
I think we should definitely have some sane defaults for pg_autoctl create postgres too.
The idea is follows: The monitor is an embedded database to our application, and we're responsible for everything about it. For create postgres, they are not ours, the customers own them. Thus, it is their responsibility to restrict/allow accesses. We don't want to do anything further than pg_auto_ctl needs.
I agree that that's the case when the database already exists. But if we create it we should not open it up by default when pg_autoctl does not need it.
agree that that's the case when the database already exists. But if we create it we should not open it up by default when pg_autoctl does not need it.
That's the default of Postgres :) As far as I can see, what we do is conveniently create the databases for the users. So, interfering with the defaults might not be a good idea.
In the future, we might consider adding --auth-local and --auth-host parameters to create postgres subcommand like here, but for now, we thought lets leave is as is.
I understand what you mean, but I disagree.
The default for postgres is to warn about it's insecure default, to try to get the user to change it. We actively disable that warning at the moment: https://github.com/citusdata/pg_auto_failover/blob/b609da294b30c4f7fece31c744fa7b1cb236f03e/src/bin/pg_autoctl/pgctl.c#L811-L812
So we already change the default behaviour to a less secure thing than the postgres default. I'd rather we change it to a more secure thing.
Postgres commiters seem to want to change these defaults (unclear to me if this is merged in PG13): https://postgrespro.com/list/thread-id/2442644#[email protected]
Since we actually already require the user to specify an auth mode for remote connections, we can easily pass that on to localhost connections too. So I do not see any reason to not do the following:
simply always do --auth-local trust and then specify --auth-host the same as --auth (or reject if --skip-pg-hba is specified).
Setting unix_socket_permissions to 0700 I can see maybe being undesirable, but a simple workaround is passing -h localhost to psql.
My main point is, we're already adding a lot of pg_hba.conf rules anyway also for create postgres. So making them a bit secure by default seems like a nice thing to do.
In the future, we might consider adding --auth-local and --auth-host parameters to create postgres subcommand like here, but for now, we thought lets leave is as is.
If we reuse our --auth flag, I actually don't see much reason to do this. --auth-local=trust we need at the moment (so a user changing this will only break things) and --auth-host we infer from our own --auth flag. So these extra flags don't seem necessary.
ignore PGHOST and skip implementing --pghost on the monitor,
I wonder if this is a good idea as we seem to have a dependency in the regression tests for PGHOST being set: https://github.com/citusdata/pg_auto_failover/pull/301#partial-pull-merging
Is my understanding accurate? If so, any suggestion/guidance on how to proceed?