Skip to content

Commit

Permalink
Adding authmethod command line arg for super and replication users.
Browse files Browse the repository at this point in the history
  • Loading branch information
emre committed Nov 28, 2017
1 parent 0257a3c commit d23713a
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 33 deletions.
79 changes: 61 additions & 18 deletions cmd/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ type config struct {
pgListenAddress string
pgPort string
pgBinPath string
pgReplAuthMethod string
pgReplUsername string
pgReplPassword string
pgReplPasswordFile string
pgSUAuthMethod string
pgSUUsername string
pgSUPassword string
pgSUPasswordFile string
Expand Down Expand Up @@ -112,9 +114,11 @@ func init() {
cmdKeeper.PersistentFlags().StringVar(&cfg.pgListenAddress, "pg-listen-address", "localhost", "postgresql instance listening address")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgPort, "pg-port", "5432", "postgresql instance listening port")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgBinPath, "pg-bin-path", "", "absolute path to postgresql binaries. If empty they will be searched in the current PATH")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgReplAuthMethod, "pg-repl-auth-method", "md5", "postgres replication user auth method. Default is md5.")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgReplUsername, "pg-repl-username", "", "postgres replication user name. Required. It'll be created on db initialization. Must be the same for all keepers.")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgReplPassword, "pg-repl-password", "", "postgres replication user password. Only one of --pg-repl-password or --pg-repl-passwordfile must be provided. Must be the same for all keepers.")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgReplPasswordFile, "pg-repl-passwordfile", "", "postgres replication user password file. Only one of --pg-repl-password or --pg-repl-passwordfile must be provided. Must be the same for all keepers.")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgSUAuthMethod, "pg-su-auth-method", "md5", "postgres superuser auth method. Default is md5.")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgSUUsername, "pg-su-username", user, "postgres superuser user name. Used for keeper managed instance access and pg_rewind based synchronization. It'll be created on db initialization. Defaults to the name of the effective user running stolon-keeper. Must be the same for all keepers.")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgSUPassword, "pg-su-password", "", "postgres superuser password. Only one of --pg-su-password or --pg-su-passwordfile must be provided. Must be the same for all keepers.")
cmdKeeper.PersistentFlags().StringVar(&cfg.pgSUPasswordFile, "pg-su-passwordfile", "", "postgres superuser password file. Only one of --pg-su-password or --pg-su-passwordfile must be provided. Must be the same for all keepers)")
Expand Down Expand Up @@ -183,46 +187,58 @@ func (p *PostgresKeeper) mandatoryPGParameters() common.Parameters {
func (p *PostgresKeeper) getSUConnParams(db, followedDB *cluster.DB) pg.ConnParams {
cp := pg.ConnParams{
"user": p.pgSUUsername,
"password": p.pgSUPassword,
"host": followedDB.Status.ListenAddress,
"port": followedDB.Status.Port,
"application_name": common.StolonName(db.UID),
"dbname": "postgres",
"sslmode": "disable",
}
if p.pgSUAuthMethod != "trust" {
cp.Set("password", p.pgSUPassword)
}
return cp
}

func (p *PostgresKeeper) getReplConnParams(db, followedDB *cluster.DB) pg.ConnParams {
return pg.ConnParams{
cp := pg.ConnParams{
"user": p.pgReplUsername,
"password": p.pgReplPassword,
"host": followedDB.Status.ListenAddress,
"port": followedDB.Status.Port,
"application_name": common.StolonName(db.UID),
"sslmode": "disable",
}
if p.pgReplAuthMethod != "trust" {
cp.Set("password", p.pgReplPassword)
}
return cp
}

func (p *PostgresKeeper) getLocalConnParams() pg.ConnParams {
return pg.ConnParams{
"user": p.pgSUUsername,
"password": p.pgSUPassword,
"host": common.PgUnixSocketDirectories,
"port": p.pgPort,
"dbname": "postgres",
"sslmode": "disable",
cp := pg.ConnParams{
"user": p.pgSUUsername,
"host": common.PgUnixSocketDirectories,
"port": p.pgPort,
"dbname": "postgres",
"sslmode": "disable",
}
if p.pgSUAuthMethod != "trust" {
cp.Set("password", p.pgSUPassword)
}
return cp
}

func (p *PostgresKeeper) getLocalReplConnParams() pg.ConnParams {
return pg.ConnParams{
cp := pg.ConnParams{
"user": p.pgReplUsername,
"password": p.pgReplPassword,
"host": common.PgUnixSocketDirectories,
"port": p.pgPort,
"sslmode": "disable",
}
if p.pgReplAuthMethod != "trust" {
cp.Set("password", p.pgReplPassword)
}
return cp
}

func (p *PostgresKeeper) createPGParameters(db *cluster.DB) common.Parameters {
Expand Down Expand Up @@ -347,8 +363,10 @@ type PostgresKeeper struct {
pgListenAddress string
pgPort string
pgBinPath string
pgReplAuthMethod string
pgReplUsername string
pgReplPassword string
pgSUAuthMethod string
pgSUUsername string
pgSUPassword string
pgInitialSUUsername string
Expand Down Expand Up @@ -404,8 +422,10 @@ func NewPostgresKeeper(cfg *config, stop chan bool, end chan error) (*PostgresKe
pgListenAddress: cfg.pgListenAddress,
pgPort: cfg.pgPort,
pgBinPath: cfg.pgBinPath,
pgReplAuthMethod: cfg.pgReplAuthMethod,
pgReplUsername: cfg.pgReplUsername,
pgReplPassword: cfg.pgReplPassword,
pgSUAuthMethod: cfg.pgSUAuthMethod,
pgSUUsername: cfg.pgSUUsername,
pgSUPassword: cfg.pgSUPassword,
pgInitialSUUsername: cfg.pgInitialSUUsername,
Expand Down Expand Up @@ -659,7 +679,7 @@ func (p *PostgresKeeper) Start() {

// TODO(sgotti) reconfigure the various configurations options
// (RequestTimeout) after a changed cluster config
pgm := postgresql.NewManager(p.pgBinPath, p.dataDir, p.getLocalConnParams(), p.getLocalReplConnParams(), p.pgSUUsername, p.pgSUPassword, p.pgReplUsername, p.pgReplPassword, p.requestTimeout)
pgm := postgresql.NewManager(p.pgBinPath, p.dataDir, p.getLocalConnParams(), p.getLocalReplConnParams(), p.pgSUAuthMethod, p.pgSUUsername, p.pgSUPassword, p.pgReplAuthMethod, p.pgReplUsername, p.pgReplPassword, p.requestTimeout)
p.pgm = pgm

p.pgm.Stop(true)
Expand Down Expand Up @@ -1579,6 +1599,9 @@ func main() {

func keeper(cmd *cobra.Command, args []string) {
var err error
validAuthMethods := make(map[string]struct{})
validAuthMethods["trust"] = struct{}{}
validAuthMethods["md5"] = struct{}{}
switch cfg.logLevel {
case "error":
slog.SetLevel(zap.ErrorLevel)
Expand Down Expand Up @@ -1610,26 +1633,46 @@ func keeper(cmd *cobra.Command, args []string) {
die("cannot create data dir: %v", err)
}

if _, ok := validAuthMethods[cfg.pgReplAuthMethod]; !ok {
die("--pg-repl-auth-method must be one of: md5, trust")
}
if cfg.pgReplUsername == "" {
die("--pg-repl-username is required")
}

if cfg.pgReplPassword == "" && cfg.pgReplPasswordFile == "" {
if cfg.pgReplAuthMethod == "trust" {
stdout("warning: not utilizing a password for replication between hosts is extremely dangerous")
if cfg.pgReplPassword != "" || cfg.pgReplPasswordFile != "" {
die("can not utilize --pg-repl-auth-method trust together with --pg-repl-password or --pg-repl-passwordfile")
}
}
if cfg.pgSUAuthMethod == "trust" {
stdout("warning: not utilizing a password for superuser is extremely dangerous")
if cfg.pgSUPassword != "" || cfg.pgSUPasswordFile != "" {
die("can not utilize --pg-su-auth-method trust together with --pg-su-password or --pg-su-passwordfile")
}
}
if cfg.pgReplAuthMethod != "trust" && cfg.pgReplPassword == "" && cfg.pgReplPasswordFile == "" {
die("one of --pg-repl-password or --pg-repl-passwordfile is required")
}
if cfg.pgReplPassword != "" && cfg.pgReplPasswordFile != "" {
if cfg.pgReplAuthMethod != "trust" && cfg.pgReplPassword != "" && cfg.pgReplPasswordFile != "" {
die("only one of --pg-repl-password or --pg-repl-passwordfile must be provided")
}
if cfg.pgSUPassword == "" && cfg.pgSUPasswordFile == "" {
if _, ok := validAuthMethods[cfg.pgSUAuthMethod]; !ok {
die("--pg-su-auth-method must be one of: md5, password, trust")
}
if cfg.pgSUAuthMethod != "trust" && cfg.pgSUPassword == "" && cfg.pgSUPasswordFile == "" {
die("one of --pg-su-password or --pg-su-passwordfile is required")
}
if cfg.pgSUPassword != "" && cfg.pgSUPasswordFile != "" {
if cfg.pgSUAuthMethod != "trust" && cfg.pgSUPassword != "" && cfg.pgSUPasswordFile != "" {
die("only one of --pg-su-password or --pg-su-passwordfile must be provided")
}

if cfg.pgSUUsername == cfg.pgReplUsername {
stdout("warning: superuser name and replication user name are the same. Different users are suggested.")
if cfg.pgSUPassword != cfg.pgReplPassword {
if cfg.pgReplAuthMethod != cfg.pgSUAuthMethod {
die("do not support different auth methods when utilizing superuser for replication.")
}
if cfg.pgSUPassword != cfg.pgReplPassword && cfg.pgSUAuthMethod != "trust" && cfg.pgReplAuthMethod != "trust" {
die("provided superuser name and replication user name are the same but provided passwords are different")
}
}
Expand Down
4 changes: 3 additions & 1 deletion doc/architecture.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ The replication user is used for:
* managing/querying the keepers' controlled instances
* replication between postgres instances

Currently only md5 password based authentication is supported. In future different authentication mechanism will be added.
Currently trust (password-less) and md5 password based authentication are supported. In the future, different authentication mechanisms will be added.

To avoid security problems (user credentials cannot be globally defined in the cluster specification since if not correctly secured it could be read by anyone accessing the cluster store) these users and their related passwords must be provided as options to the stolon keepers and their values MUST be the same for all the keepers (or different things will break). These options are `--pg-su-username`, `--pg-su-password/--pg-su-passwordfile`, `--pg-repl-username` and `--pg-repl-password/--pg-repl-passwordfile`

Utilizing `--pg-su-auth-method/--pg-repl-auth-method` trust is not recommended in production environments, but they may be used in place of password authentication. If the same user is utilized as superuser and replication user, the passwords and auth methods must match.

When a keeper initializes a new pg db cluster, the provided superuser and replication user will be created.

#### Exceeding postgres max_connections
Expand Down
47 changes: 33 additions & 14 deletions pkg/postgresql/postgresql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ type Manager struct {
curHba []string
localConnParams ConnParams
replConnParams ConnParams
suAuthMethod string
suUsername string
suPassword string
replAuthMethod string
replUsername string
replPassword string
requestTimeout time.Duration
Expand All @@ -76,16 +78,18 @@ type InitConfig struct {
DataChecksums bool
}

func NewManager(pgBinPath string, dataDir string, localConnParams, replConnParams ConnParams, suUsername, suPassword, replUsername, replPassword string, requestTimeout time.Duration) *Manager {
func NewManager(pgBinPath string, dataDir string, localConnParams, replConnParams ConnParams, suAuthMethod, suUsername, suPassword, replAuthMethod, replUsername, replPassword string, requestTimeout time.Duration) *Manager {
return &Manager{
pgBinPath: pgBinPath,
dataDir: filepath.Join(dataDir, "postgres"),
parameters: make(common.Parameters),
curParameters: make(common.Parameters),
replConnParams: replConnParams,
localConnParams: localConnParams,
suAuthMethod: suAuthMethod,
suUsername: suUsername,
suPassword: suPassword,
replAuthMethod: replAuthMethod,
replUsername: replUsername,
replPassword: replPassword,
requestTimeout: requestTimeout,
Expand Down Expand Up @@ -136,7 +140,10 @@ func (p *Manager) Init(initConfig *InitConfig) error {
pwfile.WriteString(p.suPassword)

name := filepath.Join(p.pgBinPath, "initdb")
cmd := exec.Command(name, "-D", p.dataDir, "-U", p.suUsername, "--pwfile", pwfile.Name())
cmd := exec.Command(name, "-D", p.dataDir, "-U", p.suUsername)
if p.suAuthMethod == "md5" {
cmd.Args = append(cmd.Args, "--pwfile", pwfile.Name())
}
log.Debugw("execing cmd", "cmd", cmd)

if initConfig.Locale != "" {
Expand Down Expand Up @@ -362,13 +369,19 @@ func (p *Manager) SetupRoles() error {

if p.suUsername == p.replUsername {
log.Infow("adding replication role to superuser")
if err := alterRole(ctx, p.localConnParams, []string{"replication"}, p.suUsername, p.suPassword); err != nil {
return fmt.Errorf("error adding replication role to superuser: %v", err)
if p.suAuthMethod == "trust" {
if err := alterPasswordlessRole(ctx, p.localConnParams, []string{"replication"}, p.suUsername); err != nil {
return fmt.Errorf("error adding replication role to superuser: %v", err)
}
} else {
if err := alterRole(ctx, p.localConnParams, []string{"replication"}, p.suUsername, p.suPassword); err != nil {
return fmt.Errorf("error adding replication role to superuser: %v", err)
}
}
log.Infow("replication role added to superuser")
} else {
// Configure superuser role password
if p.suPassword != "" {
// Configure superuser role password if auth method is not trust
if p.suAuthMethod != "trust" && p.suPassword != "" {
log.Infow("setting superuser password")
if err := setPassword(ctx, p.localConnParams, p.suUsername, p.suPassword); err != nil {
return fmt.Errorf("error setting superuser password: %v", err)
Expand All @@ -377,8 +390,14 @@ func (p *Manager) SetupRoles() error {
}
roles := []string{"login", "replication"}
log.Infow("creating replication role")
if err := createRole(ctx, p.localConnParams, roles, p.replUsername, p.replPassword); err != nil {
return fmt.Errorf("error creating replication role: %v", err)
if p.replAuthMethod != "trust" {
if err := createRole(ctx, p.localConnParams, roles, p.replUsername, p.replPassword); err != nil {
return fmt.Errorf("error creating replication role: %v", err)
}
} else {
if err := createPasswordlessRole(ctx, p.localConnParams, roles, p.replUsername); err != nil {
return fmt.Errorf("error creating replication role: %v", err)
}
}
log.Infow("replication role created", "role", p.replUsername)
}
Expand Down Expand Up @@ -630,19 +649,19 @@ func (p *Manager) writePgHba() error {
// Minimal entries for local normal and replication connections needed by the stolon keeper
// Matched local connections are for postgres database and suUsername user with md5 auth
// Matched local replicaton connections are for replUsername user with md5 auth
f.WriteString(fmt.Sprintf("local postgres %s md5\n", p.suUsername))
f.WriteString(fmt.Sprintf("local replication %s md5\n", p.replUsername))
f.WriteString(fmt.Sprintf("local postgres %s %s\n", p.suUsername, p.suAuthMethod))
f.WriteString(fmt.Sprintf("local replication %s %s\n", p.replUsername, p.replAuthMethod))

// By default accept all connections for the superuser suUsername with md5 auth
// Used for pg_rewind resyncronization
// TODO(sgotti) Configure this dynamically based on our followers provided by the clusterview
f.WriteString(fmt.Sprintf("host all %s %s md5\n", p.suUsername, "0.0.0.0/0"))
f.WriteString(fmt.Sprintf("host all %s %s md5\n", p.suUsername, "::0/0"))
f.WriteString(fmt.Sprintf("host all %s %s %s\n", p.suUsername, "0.0.0.0/0", p.suAuthMethod))
f.WriteString(fmt.Sprintf("host all %s %s %s\n", p.suUsername, "::0/0", p.suAuthMethod))

// By default accept all replication connections for the replication user with md5 auth
// TODO(sgotti) Configure this dynamically based on our followers provided by the clusterview
f.WriteString(fmt.Sprintf("host replication %s %s md5\n", p.replUsername, "0.0.0.0/0"))
f.WriteString(fmt.Sprintf("host replication %s %s md5\n", p.replUsername, "::0/0"))
f.WriteString(fmt.Sprintf("host replication %s %s %s\n", p.replUsername, "0.0.0.0/0", p.replAuthMethod))
f.WriteString(fmt.Sprintf("host replication %s %s %s\n", p.replUsername, "::0/0", p.replAuthMethod))

if p.hba != nil {
for _, e := range p.hba {
Expand Down
22 changes: 22 additions & 0 deletions pkg/postgresql/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,17 @@ func createRole(ctx context.Context, connParams ConnParams, roles []string, user
return err
}

func createPasswordlessRole(ctx context.Context, connParams ConnParams, roles []string, username string) error {
db, err := sql.Open("postgres", connParams.ConnString())
if err != nil {
return err
}
defer db.Close()

_, err = dbExec(ctx, db, fmt.Sprintf(`create role "%s" with login replication;`, username))
return err
}

func alterRole(ctx context.Context, connParams ConnParams, roles []string, username, password string) error {
db, err := sql.Open("postgres", connParams.ConnString())
if err != nil {
Expand All @@ -128,6 +139,17 @@ func alterRole(ctx context.Context, connParams ConnParams, roles []string, usern
return err
}

func alterPasswordlessRole(ctx context.Context, connParams ConnParams, roles []string, username string) error {
db, err := sql.Open("postgres", connParams.ConnString())
if err != nil {
return err
}
defer db.Close()

_, err = dbExec(ctx, db, fmt.Sprintf(`alter role "%s" with login replication;`, username))
return err
}

func getReplicatinSlots(ctx context.Context, connParams ConnParams) ([]string, error) {
db, err := sql.Open("postgres", connParams.ConnString())
if err != nil {
Expand Down

0 comments on commit d23713a

Please sign in to comment.