Skip to content

Commit

Permalink
Add --auth option to configure authentication method (#28)
Browse files Browse the repository at this point in the history
we can now use optional --auth parameter to specify authentication method when creating monitor and data nodes
  • Loading branch information
mtuncer committed Jul 3, 2019
1 parent eae65c3 commit 5a50002
Show file tree
Hide file tree
Showing 17 changed files with 197 additions and 30 deletions.
26 changes: 26 additions & 0 deletions docs/operations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,32 @@ environment variable, and checks whether PostgreSQL is already running. If
Postgres is detected, the new node is registered in SINGLE mode, bypassing
the monitor's role assignment policy.

.. _pg_auto_failover_security:

Security
--------
Connections between monitor and data nodes use *trust* authentication by
default. This lets accounts used by ``pg_auto_failover`` to connect to nodes
without needing a password. Default behaviour could be changed using ``--auth``
parameter when creating monitor or data Node. Any auth method supported by
PostgreSQL could be used here. Please refer to `PostgreSQL pg_hba documentation`__
for available options.

__ https://www.postgresql.org/docs/current/auth-pg-hba-conf.html

Security for following connections should be considered when setting up
`.pgpass` file.
1. health check connection from monitor for `pgautofailover_monitor` user
2. connections for `pg_autoctl` command from data nodes to monitor for `autoctl_node` user
3. replication connections from secondary to primary data nodes for `replication` user.
Notice that primary and secondary nodes change during failover. Thus this setting
should be done on both primary and secondary nodes.
4. settings need to be updated after a new node is added.
See `PostgreSQL documentation`__ on setting up `.pgpass` file.

__ https://www.postgresql.org/docs/current/libpq-pgpass.html


Operations
----------

Expand Down
16 changes: 16 additions & 0 deletions docs/ref/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ commands are dealing with the monitor:
--pgdata path to data directory
--pgport PostgreSQL's port number
--nodename hostname by which postgres is reachable
--auth authentication method for connections from data nodes

The ``--pgdata`` option is mandatory and default to the environment
variable ``PGDATA``. The ``--pgport`` default value is 5432, and the
Expand Down Expand Up @@ -119,6 +120,13 @@ commands are dealing with the monitor:
You may use the `--nodename` command line option to bypass the whole
DNS lookup based process and force the local node name to a fixed
value.

The ``--auth`` option allows setting up authentication method to be used
for connections from data nodes with ``autoctl_node`` user.
If this option is used, please make sure password is manually set on
the monitor, and appropriate setting is added to `.pgpass` file on data node.

See :ref:`pg_auto_failover_security` for notes on `.pgpass`

- ``pg_autoctl run``

Expand Down Expand Up @@ -269,6 +277,7 @@ The other commands accept the same set of options.
--nodename pg_auto_failover node
--formation pg_auto_failover formation
--monitor pg_auto_failover Monitor Postgres URL
--auth authentication method for connections from monitor
--allow-removing-pgdata Allow pg_autoctl to remove the database directory

Three different modes of initialization are supported by this command,
Expand Down Expand Up @@ -321,6 +330,13 @@ When `--nodename` is omitted, it is computed as above (see
:ref:`_pg_autoctl_create_monitor`), with the difference that step 1 uses the
monitor IP and port rather than the public service 8.8.8.8:53.

The ``--auth`` option allows setting up authentication method to be used when
monitor node makes a connection to data node with `pgautofailover_monitor` user.
If this option is, please make sure password is manually set on the data
node, and appropriate setting is added to `.pgpass` file on monitor node.

See :ref:`pg_auto_failover_security` for notes on `.pgpass`

.. _pg_autoctl_configuration:

pg_autoctl configuration and state files
Expand Down
8 changes: 8 additions & 0 deletions src/bin/pg_autoctl/cli_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ bool allowRemovingPgdata = false;
* { "listen", required_argument, NULL, 'l' },
* { "proxyport", required_argument, NULL, 'y' },
* { "username", required_argument, NULL, 'U' },
{ "auth", required_argument, NULL, 'A' },
* { "dbname", required_argument, NULL, 'd' },
* { "nodename", required_argument, NULL, 'n' },
* { "formation", required_argument, NULL, 'f' },
Expand Down Expand Up @@ -149,6 +150,13 @@ cli_create_node_getopts(int argc, char **argv,
break;
}

case 'A':
{
/* { "auth", required_argument, NULL, 'A' }, */
strlcpy(LocalOptionConfig.pgSetup.authMethod, optarg, NAMEDATALEN);
log_trace("--auth %s", LocalOptionConfig.pgSetup.authMethod);
break;
}
case 'd':
{
/* { "dbname", required_argument, NULL, 'd' } */
Expand Down
16 changes: 13 additions & 3 deletions src/bin/pg_autoctl/cli_create_drop_node.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ CommandLine create_monitor_command =
" --pgctl path to pg_ctl\n" \
" --pgdata path to data directory\n" \
" --pgport PostgreSQL's port number\n" \
" --nodename hostname by which postgres is reachable\n",
" --nodename hostname by which postgres is reachable\n" \
" --auth authentication method for connections from data nodes\n",
cli_create_monitor_getopts,
cli_create_monitor);

Expand All @@ -72,6 +73,7 @@ CommandLine create_postgres_command =
" --nodename pg_auto_failover node\n"
" --formation pg_auto_failover formation\n"
" --monitor pg_auto_failover Monitor Postgres URL\n"
" --auth authentication method for connections from monitor\n"
KEEPER_CLI_ALLOW_RM_PGDATA_OPTION,
cli_create_postgres_getopts,
cli_create_postgres);
Expand Down Expand Up @@ -195,6 +197,7 @@ cli_create_postgres_getopts(int argc, char **argv)
{ "pgport", required_argument, NULL, 'p' },
{ "listen", required_argument, NULL, 'l' },
{ "username", required_argument, NULL, 'U' },
{ "auth", required_argument, NULL, 'A' },
{ "dbname", required_argument, NULL, 'd' },
{ "nodename", required_argument, NULL, 'n' },
{ "formation", required_argument, NULL, 'f' },
Expand All @@ -206,7 +209,7 @@ cli_create_postgres_getopts(int argc, char **argv)

int optind =
cli_create_node_getopts(argc, argv,
long_options, "C:D:h:p:l:U:d:n:f:m:R",
long_options, "C:D:h:p:l:U:A:d:n:f:m:R",
&options);

/* publish our option parsing in the global variable */
Expand Down Expand Up @@ -262,6 +265,7 @@ cli_create_monitor_getopts(int argc, char **argv)
{ "pgport", required_argument, NULL, 'p' },
{ "nodename", required_argument, NULL, 'n' },
{ "listen", required_argument, NULL, 'l' },
{ "auth", required_argument, NULL, 'A' },
{ "help", no_argument, NULL, 0 },
{ NULL, 0, NULL, 0 }
};
Expand All @@ -271,7 +275,7 @@ cli_create_monitor_getopts(int argc, char **argv)

optind = 0;

while ((c = getopt_long(argc, argv, "C:D:p:",
while ((c = getopt_long(argc, argv, "C:D:p:A:",
long_options, &option_index)) != -1)
{
switch (c)
Expand Down Expand Up @@ -317,6 +321,12 @@ cli_create_monitor_getopts(int argc, char **argv)
break;
}

case 'A':
{
strlcpy(options.pgSetup.authMethod, optarg, NAMEDATALEN);
log_trace("--auth %s", options.pgSetup.authMethod);
break;
}
default:
{
/* getopt_long already wrote an error message */
Expand Down
3 changes: 2 additions & 1 deletion src/bin/pg_autoctl/cli_do_misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ keeper_cli_create_monitor_user(int argc, char **argv)

if (!primary_create_user_with_hba(&postgres,
PG_AUTOCTL_HEALTH_USERNAME, password,
monitorHostname))
monitorHostname,
pg_setup_get_auth_method(&(config.pgSetup))))
{
log_fatal("Failed to create the database user that the pg_auto_failover "
" monitor uses for health checks, see above for details");
Expand Down
3 changes: 2 additions & 1 deletion src/bin/pg_autoctl/cli_do_root.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ keeper_cli_keeper_setup_getopts(int argc, char **argv)
{ "listen", required_argument, NULL, 'l' },
{ "proxyport", required_argument, NULL, 'y' },
{ "username", required_argument, NULL, 'U' },
{ "auth", required_argument, NULL, 'A' },
{ "dbname", required_argument, NULL, 'd' },
{ "nodename", required_argument, NULL, 'n' },
{ "formation", required_argument, NULL, 'f' },
Expand All @@ -250,7 +251,7 @@ keeper_cli_keeper_setup_getopts(int argc, char **argv)

int optind =
cli_create_node_getopts(argc, argv,
long_options, "C:D:h:p:l:y:U:d:n:f:g:m:R",
long_options, "C:D:h:p:l:y:U:A:d:n:f:g:m:R",
&options);

/* publish our option parsing in the global variable */
Expand Down
1 change: 1 addition & 0 deletions src/bin/pg_autoctl/defaults.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#define POSTGRES_DEFAULT_LISTEN_ADDRESSES "*"
#define DEFAULT_DATABASE_NAME "postgres"
#define DEFAULT_USERNAME "postgres"
#define DEFAULT_AUTH_METHOD "trust"
#define REPLICATION_SLOT_NAME_DEFAULT "pgautofailover_standby"
#define REPLICATION_PASSWORD_DEFAULT NULL
#define FORMATION_DEFAULT "default"
Expand Down
4 changes: 3 additions & 1 deletion src/bin/pg_autoctl/fsm_transition.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ fsm_init_primary(Keeper *keeper)
char *password = NULL;
char monitorHostname[_POSIX_HOST_NAME_MAX];
int monitorPort = 0;
char *authMethod = NULL;

log_info("Initialising postgres as a primary");

Expand Down Expand Up @@ -99,10 +100,11 @@ fsm_init_primary(Keeper *keeper)
}

password = NULL;
authMethod = pg_setup_get_auth_method(&(config->pgSetup));

if (!primary_create_user_with_hba(postgres,
PG_AUTOCTL_HEALTH_USERNAME, password,
monitorHostname))
monitorHostname, authMethod))
{
log_error("Failed to initialise postgres as primary because creating the "
"database user that the pg_auto_failover monitor "
Expand Down
3 changes: 2 additions & 1 deletion src/bin/pg_autoctl/keeper_pg_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ reach_initial_state(Keeper *keeper)
(void) pghba_enable_lan_cidr(&keeper->postgres.sqlClient,
HBA_DATABASE_ALL, NULL,
keeper->config.nodename,
NULL, "trust", NULL);
NULL, DEFAULT_AUTH_METHOD, NULL);

}
}
else
Expand Down
6 changes: 6 additions & 0 deletions src/bin/pg_autoctl/monitor_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
make_strbuf_option("postgresql", "listen_addresses", "listen", \
false, MAXPGPATH, config->pgSetup.listen_addresses)

#define OPTION_POSTGRESQL_AUTH_METHOD(config) \
make_strbuf_option("postgresql", "auth_method", "auth", \
false, MAXPGPATH, config->pgSetup.authMethod)


#define SET_INI_OPTIONS_ARRAY(config) \
{ \
Expand All @@ -72,6 +76,7 @@
OPTION_POSTGRESQL_HOST(config), \
OPTION_POSTGRESQL_PORT(config), \
OPTION_POSTGRESQL_LISTEN_ADDRESSES(config), \
OPTION_POSTGRESQL_AUTH_METHOD(config), \
INI_OPTION_LAST \
}

Expand Down Expand Up @@ -311,6 +316,7 @@ monitor_config_log_settings(MonitorConfig config)
log_debug("postgresql.dbname: %s", config.pgSetup.dbname);
log_debug("postgresql.host: %s", config.pgSetup.pghost);
log_debug("postgresql.port: %d", config.pgSetup.pgport);
log_debug("postgresql.auth: %s", config.pgSetup.authMethod);
}


Expand Down
4 changes: 3 additions & 1 deletion src/bin/pg_autoctl/monitor_pg_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ monitor_install(const char *nodename,
HBA_DATABASE_DBNAME,
PG_AUTOCTL_MONITOR_DBNAME,
nodename,
PG_AUTOCTL_MONITOR_USERNAME, "trust", NULL))
PG_AUTOCTL_MONITOR_USERNAME,
pg_setup_get_auth_method(&pgSetup),
NULL))
{
log_warn("Failed to grant connection to local network.");
return false;
Expand Down
29 changes: 29 additions & 0 deletions src/bin/pg_autoctl/pgsetup.c
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,17 @@ pg_setup_init(PostgresSetup *pgSetup,
POSTGRES_DEFAULT_LISTEN_ADDRESSES, MAXPGPATH);
}


/*
* If --auth is given, then set our authMethod to this value
* otherwise it remains empty
*/
if (!IS_EMPTY_STRING_BUFFER(options->authMethod))
{
strlcpy(pgSetup->authMethod,
options->authMethod, NAMEDATALEN);
}

/*
* And we always double-check with PGDATA/postmaster.pid if we have it, and
* we should have it in the normal/expected case.
Expand Down Expand Up @@ -835,6 +846,24 @@ pg_setup_get_username(PostgresSetup *pgSetup)
}


/*
* pg_setup_get_auth_method returns pgSetup->authMethod when it exists, otherwise it
* returns DEFAULT_AUTH_METHOD
*/
char *
pg_setup_get_auth_method(PostgresSetup *pgSetup)
{
if (!IS_EMPTY_STRING_BUFFER(pgSetup->authMethod))
{
return pgSetup->authMethod;
}

log_trace("auth method not configured, falling back to default value : %s", DEFAULT_AUTH_METHOD);

return DEFAULT_AUTH_METHOD;
}


/*
* pg_setup_set_absolute_pgdata uses realpath(3) to make sure that
* we re using the absolute real pathname for PGDATA in our setup, so that
Expand Down
2 changes: 2 additions & 0 deletions src/bin/pg_autoctl/pgsetup.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ typedef struct pg_setup
int pgport; /* PGPORT */
char listen_addresses[MAXPGPATH]; /* listen_addresses */
int proxyport; /* Proxy port */
char authMethod[NAMEDATALEN]; /* auth method, defaults to trust */
PostmasterStatus pm_status; /* Postmaster status */
bool is_in_recovery; /* select pg_is_in_recovery() */
PostgresControlData control; /* pg_controldata pgdata */
Expand All @@ -131,6 +132,7 @@ bool pg_setup_is_running(PostgresSetup *pgSetup);
bool pg_setup_is_primary(PostgresSetup *pgSetup);
bool pg_setup_is_ready(PostgresSetup *pgSetup, bool pg_is_not_running_is_ok);
char *pg_setup_get_username(PostgresSetup *pgSetup);
char *pg_setup_get_auth_method(PostgresSetup *pgSetup);
bool pg_setup_set_absolute_pgdata(PostgresSetup *pgSetup);

PgInstanceKind nodeKindFromString(const char *nodeKind);
Expand Down
11 changes: 8 additions & 3 deletions src/bin/pg_autoctl/primary_standby.c
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ postgres_add_default_settings(LocalPostgresServer *postgres)
*/
bool
primary_create_user_with_hba(LocalPostgresServer *postgres, char *userName,
char *password, char *hostname)
char *password, char *hostname, char *authMethod)
{
PGSQL *pgsql = &(postgres->sqlClient);
bool login = true;
Expand All @@ -357,7 +357,7 @@ primary_create_user_with_hba(LocalPostgresServer *postgres, char *userName,
}

if (!pghba_ensure_host_rule_exists(hbaFilePath, HBA_DATABASE_ALL, NULL, userName,
hostname, "trust"))
hostname, authMethod))
{
log_error("Failed to set the pg_hba rule for user \"%s\"", userName);
return false;
Expand Down Expand Up @@ -411,7 +411,12 @@ primary_add_standby_to_hba(LocalPostgresServer *postgres, char *standbyHostname,
PGSQL *pgsql = &(postgres->sqlClient);
PostgresSetup *postgresSetup = &(postgres->postgresSetup);
char hbaFilePath[MAXPGPATH];
char* authMethod = replicationPassword ? "md5" : "trust";
char *authMethod = "trust";

if (replicationPassword)
{
authMethod = pg_setup_get_auth_method(postgresSetup);
}

log_trace("primary_add_standby_to_hba");

Expand Down
2 changes: 1 addition & 1 deletion src/bin/pg_autoctl/primary_standby.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ bool primary_enable_synchronous_replication(LocalPostgresServer *postgres);
bool primary_disable_synchronous_replication(LocalPostgresServer *postgres);
bool postgres_add_default_settings(LocalPostgresServer *postgres);
bool primary_create_user_with_hba(LocalPostgresServer *postgres, char *userName,
char *password, char *hostname);
char *password, char *hostname, char *authMethod);
bool primary_create_replication_user(LocalPostgresServer *postgres,
char *replicationUser,
char *replicationPassword);
Expand Down

0 comments on commit 5a50002

Please sign in to comment.