mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
fix: disallow reserved master names during rds password apply
This commit is contained in:
@@ -155,6 +155,12 @@ func (e *postgresEngine) SetPassword(ctx context.Context, username, password str
|
||||
return fmt.Errorf("set-password requires both %s and %s",
|
||||
handlers_rds.CommandParamMasterUsername, handlers_rds.CommandParamMasterUserPassword)
|
||||
}
|
||||
// The statement below runs as the cluster superuser over the socket under
|
||||
// peer auth, so a reserved name in the command payload would hand the
|
||||
// customer the bootstrap superuser rather than rotate their own role.
|
||||
if err := handlers_rds.EnginePostgres().ValidateUsernameNotReserved(username); err != nil {
|
||||
return fmt.Errorf("refusing to set the password of a role the engine reserves: %w", err)
|
||||
}
|
||||
const sql = `\getenv master RDS_MASTER_USERNAME
|
||||
\getenv password RDS_MASTER_PASSWORD
|
||||
ALTER ROLE :"master" WITH LOGIN PASSWORD :'password';
|
||||
|
||||
@@ -61,7 +61,7 @@ func TestCommandRegistry_SetPasswordCarriesBothParameters(t *testing.T) {
|
||||
CommandID: "cmd-1",
|
||||
Type: handlers_rds.CommandSetPassword,
|
||||
Parameters: []handlers_rds.Parameter{
|
||||
{Name: handlers_rds.CommandParamMasterUsername, Value: "postgres"},
|
||||
{Name: handlers_rds.CommandParamMasterUsername, Value: "mulgamaster"},
|
||||
{Name: handlers_rds.CommandParamMasterUserPassword, Value: "n3w-pw"},
|
||||
},
|
||||
})
|
||||
@@ -69,8 +69,8 @@ func TestCommandRegistry_SetPasswordCarriesBothParameters(t *testing.T) {
|
||||
if reply.Status != handlers_rds.CommandStatusSucceeded {
|
||||
t.Fatalf("reply = %+v, want succeeded", reply)
|
||||
}
|
||||
if engine.username != "postgres" || engine.password != "n3w-pw" {
|
||||
t.Errorf("engine got %q/%q, want postgres/n3w-pw", engine.username, engine.password)
|
||||
if engine.username != "mulgamaster" || engine.password != "n3w-pw" {
|
||||
t.Errorf("engine got %q/%q, want mulgamaster/n3w-pw", engine.username, engine.password)
|
||||
}
|
||||
// The reply is recorded by the control plane; the new password must not be
|
||||
// in it.
|
||||
@@ -162,7 +162,7 @@ func TestPostgresEngine_SetPasswordKeepsTheSecretOutOfArgv(t *testing.T) {
|
||||
runner := &recordingRunner{}
|
||||
engine := newTestEngine(t, runner.run)
|
||||
|
||||
if err := engine.SetPassword(context.Background(), "postgres", "n3w-pw"); err != nil {
|
||||
if err := engine.SetPassword(context.Background(), "mulgamaster", "n3w-pw"); err != nil {
|
||||
t.Fatalf("SetPassword: %v", err)
|
||||
}
|
||||
if len(runner.calls) != 1 {
|
||||
@@ -191,7 +191,7 @@ func TestPostgresEngine_SetPasswordRedactsTheSecretFromAFailure(t *testing.T) {
|
||||
runner := &recordingRunner{err: errors.New(`ERROR: syntax error at or near "n3w-pw"`)}
|
||||
engine := newTestEngine(t, runner.run)
|
||||
|
||||
err := engine.SetPassword(context.Background(), "postgres", "n3w-pw")
|
||||
err := engine.SetPassword(context.Background(), "mulgamaster", "n3w-pw")
|
||||
if err == nil {
|
||||
t.Fatal("SetPassword succeeded against a failing psql")
|
||||
}
|
||||
@@ -207,7 +207,7 @@ func TestPostgresEngine_SetPasswordRejectsAnIncompleteCommand(t *testing.T) {
|
||||
runner := &recordingRunner{}
|
||||
engine := newTestEngine(t, runner.run)
|
||||
|
||||
if err := engine.SetPassword(context.Background(), "postgres", ""); err == nil {
|
||||
if err := engine.SetPassword(context.Background(), "mulgamaster", ""); err == nil {
|
||||
t.Error("SetPassword accepted an empty password")
|
||||
}
|
||||
if len(runner.calls) != 0 {
|
||||
@@ -215,6 +215,26 @@ func TestPostgresEngine_SetPasswordRejectsAnIncompleteCommand(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// D21: the apply runs as the cluster superuser under peer auth, so a reserved
|
||||
// name arriving in a command payload would rotate the bootstrap superuser's
|
||||
// password rather than the customer's own role.
|
||||
func TestPostgresEngine_SetPasswordRefusesReservedRoles(t *testing.T) {
|
||||
for _, username := range []string{"postgres", "rds_superuser", "rdsadmin", "pg_toast_owner", "PostGres"} {
|
||||
t.Run(username, func(t *testing.T) {
|
||||
runner := &recordingRunner{}
|
||||
engine := newTestEngine(t, runner.run)
|
||||
|
||||
err := engine.SetPassword(context.Background(), username, "n3w-pw")
|
||||
if err == nil {
|
||||
t.Fatalf("SetPassword accepted the reserved role %q", username)
|
||||
}
|
||||
if len(runner.calls) != 0 {
|
||||
t.Errorf("ran %d commands for a reserved role, want 0", len(runner.calls))
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The parameters land where rds-init installs them, so the next boot overwrites
|
||||
// the file rather than shadowing it with a second copy.
|
||||
func TestPostgresEngine_ApplyParametersInstallsAndReloads(t *testing.T) {
|
||||
|
||||
@@ -98,9 +98,14 @@ load_handoff() {
|
||||
# The master role is created NOSUPERUSER, so a master named after the
|
||||
# bootstrap superuser would leave the cluster without one and strip rds-agent
|
||||
# of the privileged SQL it reaches over peer auth — unrepairable, since the
|
||||
# datadir bootstraps once. The control plane reserves both names already.
|
||||
case "${RDS_MASTER_USERNAME}" in
|
||||
"${PG_USER}" | "${MASTER_GROUP_ROLE}")
|
||||
# datadir bootstraps once.
|
||||
#
|
||||
# The same set the control plane reserves, matched the same way: anything it
|
||||
# refuses must be refused here too, or the name reaches initdb, fails at role
|
||||
# creation and spends the one-shot password on the way.
|
||||
_master_lower=$(printf '%s' "${RDS_MASTER_USERNAME}" | tr '[:upper:]' '[:lower:]')
|
||||
case "${_master_lower}" in
|
||||
"${PG_USER}" | "${MASTER_GROUP_ROLE}" | rdsadmin | pg_*)
|
||||
fail "RDS_MASTER_USERNAME ${RDS_MASTER_USERNAME} is reserved by the platform; the control plane should have rejected it"
|
||||
;;
|
||||
esac
|
||||
|
||||
@@ -508,7 +508,9 @@ run_fails "no-handoff"
|
||||
# The master role is created NOSUPERUSER, so one named after the bootstrap
|
||||
# superuser would leave the cluster without a superuser at all and strip
|
||||
# rds-agent of the privileged SQL it needs — on a datadir that bootstraps once.
|
||||
for reserved in postgres rds_superuser; do
|
||||
# The set and the case-insensitive matching are the control plane's: a name it
|
||||
# refuses must not reach initdb here and spend the one-shot password.
|
||||
for reserved in postgres rds_superuser rdsadmin pg_toast_owner PostGres; do
|
||||
reset_state
|
||||
MASTER_USER="${reserved}"
|
||||
write_handoff initialize 's3cr3t' appdb
|
||||
|
||||
@@ -96,7 +96,14 @@ func (e Engine) ValidateMasterUsername(username string) error {
|
||||
"MasterUsername may contain only letters, digits and underscores")
|
||||
}
|
||||
}
|
||||
lower := strings.ToLower(username)
|
||||
return e.ValidateUsernameNotReserved(username)
|
||||
}
|
||||
|
||||
// The reserved-role half of the check on its own, exported for the in-guest
|
||||
// agent: its live password apply runs as the cluster superuser, so it re-checks
|
||||
// the name it is handed rather than trusting the control plane to have done it.
|
||||
func (e Engine) ValidateUsernameNotReserved(username string) error {
|
||||
lower := strings.ToLower(strings.TrimSpace(username))
|
||||
if slices.Contains(e.reservedUsernames, lower) {
|
||||
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"MasterUsername %q is reserved by %s", username, e.Name)
|
||||
|
||||
@@ -13,3 +13,8 @@ var enginePostgres = Engine{
|
||||
reservedUsernames: []string{"rdsadmin", "postgres", "rds_superuser"},
|
||||
reservedUsernamePrefixes: []string{"pg_"},
|
||||
}
|
||||
|
||||
// The pinned engine as a value, for the in-guest agent: it enforces the same
|
||||
// reserved-role set before altering a role as the cluster superuser, and a
|
||||
// lookup by name there could only fail silently into an empty set.
|
||||
func EnginePostgres() Engine { return enginePostgres }
|
||||
|
||||
Reference in New Issue
Block a user