mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-19 10:52:04 +00:00
feat: validate the initial database name at create and in the guest
DBName was passed through unvalidated, so a name such as my-db was accepted at CreateDBInstance and only quoted into safety by psql inside the guest. Engine gains a validateDBName rule and ValidateDBName, and rds-init re-checks the same rule in load_handoff before initdb, where a failure would clear the datadir and spend the one-shot master password. PostgreSQL caps the name at NAMEDATALEN-1.
This commit is contained in:
+1
-1
@@ -863,7 +863,7 @@ Statuses: `creating`, `available`, `modifying`, `backing-up`, `rebooting`, `stop
|
||||
|
||||
| Command | Implemented Flags | Missing Flags | Status |
|
||||
|---------|-------------------|---------------|--------|
|
||||
| `create-db-instance` | `--db-instance-identifier`, `--engine` (postgres), `--engine-version` (18), `--db-instance-class`, `--allocated-storage` (20–65536 GiB), `--storage-type` (gp3), `--master-username`, `--master-user-password`, `--db-name`, `--port` (1150–65535), `--db-subnet-group-name` (unnamed → the account's default VPC subnet), `--vpc-security-group-ids`, `--db-parameter-group-name`, `--backup-retention-period` (0–7, default 7), `--preferred-backup-window`, `--preferred-maintenance-window` (unnamed → assigned from the identifier), `--deletion-protection`, `--tags`, `--storage-encrypted` (true only) | `--auto-minor-version-upgrade`, `--copy-tags-to-snapshot`, Performance Insights / Enhanced Monitoring flags (all accepted, no-op); see "Rejected Parameters" for those that fail loudly | **DONE** |
|
||||
| `create-db-instance` | `--db-instance-identifier`, `--engine` (postgres), `--engine-version` (18), `--db-instance-class`, `--allocated-storage` (20–65536 GiB), `--storage-type` (gp3), `--master-username`, `--master-user-password`, `--db-name` (letters, digits and underscores, beginning with a letter, max 63), `--port` (1150–65535), `--db-subnet-group-name` (unnamed → the account's default VPC subnet), `--vpc-security-group-ids`, `--db-parameter-group-name`, `--backup-retention-period` (0–7, default 7), `--preferred-backup-window`, `--preferred-maintenance-window` (unnamed → assigned from the identifier), `--deletion-protection`, `--tags`, `--storage-encrypted` (true only) | `--auto-minor-version-upgrade`, `--copy-tags-to-snapshot`, Performance Insights / Enhanced Monitoring flags (all accepted, no-op); see "Rejected Parameters" for those that fail loudly | **DONE** |
|
||||
| `describe-db-instances` | `--db-instance-identifier` | `--filters`, `--max-records`, `--marker` (parsed, not applied) | **DONE** |
|
||||
| `modify-db-instance` | `--db-instance-identifier`, `--master-user-password`, `--allocated-storage` (grow only, stop/start), `--db-instance-class` (VM replace), `--db-parameter-group-name`, `--vpc-security-group-ids`, `--deletion-protection`, `--backup-retention-period`, `--preferred-backup-window`, `--preferred-maintenance-window`, `--apply-immediately` | `--new-db-instance-identifier`, `--engine-version`, `--db-port-number`, `--db-subnet-group-name`, `--max-allocated-storage`, `--ca-certificate-identifier` (all rejected, not ignored) | **DONE** |
|
||||
| `delete-db-instance` | `--db-instance-identifier`, `--skip-final-snapshot`, `--final-db-snapshot-identifier` (exactly one is required) | `--delete-automated-backups` (no-op — automated snapshots are always purged with the instance) | **DONE** — `DeletionProtection` blocks the call; a final snapshot **retains** the data volume until that snapshot is deleted |
|
||||
|
||||
@@ -71,6 +71,10 @@ MOUNTS_FILE="${RDS_MOUNTS_FILE:-/proc/mounts}"
|
||||
# AWS, so a customer's `GRANT rds_superuser TO ...` works here too.
|
||||
MASTER_GROUP_ROLE="rds_superuser"
|
||||
|
||||
# NAMEDATALEN-1: the server truncates a longer identifier, so the database
|
||||
# created would not be the one the customer asked for.
|
||||
MAX_DB_NAME_LEN=63
|
||||
|
||||
log() { echo "[rds-init] $*"; }
|
||||
fail() { echo "[rds-init] ERROR: $*" >&2; exit 1; }
|
||||
|
||||
@@ -134,6 +138,20 @@ load_handoff() {
|
||||
;;
|
||||
esac
|
||||
|
||||
# The same rule the control plane applies, for the same reason the reserved
|
||||
# name is re-checked: an unusable name reaches initdb, fails at CREATE
|
||||
# DATABASE and spends the one-shot password on the way. psql quotes the
|
||||
# identifier, so here this is defence in depth rather than the barrier.
|
||||
case "${RDS_DB_NAME}" in
|
||||
'') ;;
|
||||
[!A-Za-z]* | *[!A-Za-z0-9_]*)
|
||||
fail "RDS_DB_NAME ${RDS_DB_NAME} must begin with a letter and hold only letters, digits and underscores; the control plane should have rejected it"
|
||||
;;
|
||||
esac
|
||||
if [ "${#RDS_DB_NAME}" -gt "${MAX_DB_NAME_LEN}" ]; then
|
||||
fail "RDS_DB_NAME is longer than the ${MAX_DB_NAME_LEN} characters PostgreSQL allows; the control plane should have rejected it"
|
||||
fi
|
||||
|
||||
case "${RDS_PORT}" in
|
||||
'' | *[!0-9]*) fail "bootstrap config has a non-numeric RDS_PORT: ${RDS_PORT}" ;;
|
||||
esac
|
||||
|
||||
@@ -207,7 +207,10 @@ write_handoff() {
|
||||
# password carrying shell metacharacters — or a newline — survive the
|
||||
# handoff intact and reach the checks that exist for it.
|
||||
[ -n "$2" ] && echo "RDS_MASTER_PASSWORD='$2'"
|
||||
[ -n "$3" ] && echo "RDS_DB_NAME=$3"
|
||||
# Quoted as rds-agent's shellQuote writes it, so a name carrying a space
|
||||
# or a shell metacharacter survives the handoff and reaches the check
|
||||
# that exists for it rather than being mangled on the way.
|
||||
[ -n "$3" ] && echo "RDS_DB_NAME='$3'"
|
||||
[ -n "${PENDING:-}" ] && echo "RDS_BOOTSTRAP_PENDING=${PENDING}"
|
||||
[ -n "${PAYLOAD_ID:-}" ] && echo "RDS_PAYLOAD_ID=${PAYLOAD_ID}"
|
||||
# Last, and unconditional: an optional line failing its test would make
|
||||
@@ -811,6 +814,31 @@ for reserved in postgres rds_superuser rdsadmin pg_toast_owner PostGres; do
|
||||
unset MASTER_USER
|
||||
done
|
||||
|
||||
# --- Case 10: an initial database name the control plane would have refused ---
|
||||
# The name is interpolated into a CREATE DATABASE, and a failure there is a
|
||||
# failure after initdb: the datadir is cleared and the one-shot password is
|
||||
# gone. Refusing before initdb is what keeps the create retryable.
|
||||
for badname in 'my-db' 'my db' 'my/db' 'my.db' '1db' \
|
||||
'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; do
|
||||
reset_state
|
||||
write_handoff initialize 's3cr3t' "${badname}"
|
||||
run_fails "bad-dbname-${badname}"
|
||||
[ -s "${INITDB_CALLS}" ] \
|
||||
&& fail "bad-dbname-${badname}: initdb ran before the name was refused" \
|
||||
|| pass "bad-dbname-${badname}: refused before initdb"
|
||||
done
|
||||
|
||||
# The rule is a rejection of malformed names, not of every name: the boundary
|
||||
# length and an underscore have to keep working.
|
||||
reset_state
|
||||
write_handoff initialize 's3cr3t' \
|
||||
'a_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
|
||||
write_parameters
|
||||
if run_ok "dbname-at-the-limit"; then
|
||||
grep -q 'CREATE DATABASE' "${PSQL_CALLS}" \
|
||||
&& pass "dbname-at-the-limit: accepted" || fail "dbname-at-the-limit: no CREATE DATABASE"
|
||||
fi
|
||||
|
||||
if [ "${FAILS}" -eq 0 ]; then
|
||||
echo "PASS: all rds-init cases"
|
||||
exit 0
|
||||
|
||||
@@ -594,6 +594,10 @@ func TestValidateCreateRequest_RejectsMalformedRequests(t *testing.T) {
|
||||
{"PortTooLow", func(in *rds.CreateDBInstanceInput) { in.Port = aws.Int64(80) }, awserrors.ErrorInvalidParameterValue},
|
||||
{"ReservedUsername", func(in *rds.CreateDBInstanceInput) { in.MasterUsername = aws.String("rdsadmin") }, awserrors.ErrorInvalidParameterValue},
|
||||
{"ShortPassword", func(in *rds.CreateDBInstanceInput) { in.MasterUserPassword = aws.String("short") }, awserrors.ErrorInvalidParameterValue},
|
||||
{"DBNameHyphen", func(in *rds.CreateDBInstanceInput) { in.DBName = aws.String("my-db") }, awserrors.ErrorInvalidParameterValue},
|
||||
{"DBNameTooLong", func(in *rds.CreateDBInstanceInput) {
|
||||
in.DBName = aws.String(strings.Repeat("a", 64))
|
||||
}, awserrors.ErrorInvalidParameterValue},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
|
||||
@@ -26,6 +26,9 @@ type Engine struct {
|
||||
// The engine's own identifier limit for a role name. The character rule is
|
||||
// shared across engines; the length is not.
|
||||
maxUsernameLen int
|
||||
// The engine's rule for an initial database name, which the guest
|
||||
// interpolates into a CREATE DATABASE.
|
||||
validateDBName func(string) error
|
||||
|
||||
// The engine's parameter table, keyed by parameter name. The generic spec
|
||||
// machinery is shared; only the table and its formulas are per-engine.
|
||||
@@ -47,6 +50,11 @@ var enginesByFamily = indexEnginesByFamily()
|
||||
func indexEnginesByFamily() map[string]Engine {
|
||||
out := make(map[string]Engine, len(engines))
|
||||
for _, engine := range engines {
|
||||
// An engine registered without its own name rule would panic on the first
|
||||
// create that names a database, rather than here where every build sees it.
|
||||
if engine.validateDBName == nil {
|
||||
panic("rds: engine " + engine.Name + " registers no DBName rule")
|
||||
}
|
||||
family := engine.ParameterGroupFamily()
|
||||
if _, exists := out[family]; exists {
|
||||
panic("rds: parameter group family " + family + " is claimed by two engines")
|
||||
@@ -160,6 +168,36 @@ func (e Engine) ValidateUsernameNotReserved(username string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// The initial database, which AWS leaves optional: an empty name creates no
|
||||
// database at all rather than one named by default.
|
||||
func (e Engine) ValidateDBName(name string) error {
|
||||
return e.validateDBName(name)
|
||||
}
|
||||
|
||||
// The shared rule, taking each engine's own identifier limit. The character set
|
||||
// is narrower than either engine accepts, because it is also what makes the name
|
||||
// safe to interpolate into the CREATE DATABASE rds-init builds inside the guest.
|
||||
func dbNameRule(maxLen int) func(string) error {
|
||||
return func(name string) error {
|
||||
switch {
|
||||
case name == "":
|
||||
return nil
|
||||
case len(name) > maxLen:
|
||||
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"DBName must be at most %d characters", maxLen)
|
||||
case !isLetter(rune(name[0])):
|
||||
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "DBName must begin with a letter")
|
||||
}
|
||||
for _, r := range name {
|
||||
if !isLetter(r) && !isDigit(r) && r != '_' {
|
||||
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"DBName may contain only letters, digits and underscores")
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
// Bounds and the printable-ASCII range AWS accepts. The password is never
|
||||
// inspected beyond this and never stored in cleartext past the first bootstrap
|
||||
// fetch.
|
||||
|
||||
@@ -20,8 +20,9 @@ var enginePostgres = Engine{
|
||||
reservedUsernames: []string{"rdsadmin", "postgres", "rds_superuser"},
|
||||
reservedUsernamePrefixes: []string{"pg_"},
|
||||
// NAMEDATALEN-1, where the engine's own limit and AWS's documented one
|
||||
// coincide.
|
||||
// coincide. A database name is the same identifier limit.
|
||||
maxUsernameLen: 63,
|
||||
validateDBName: dbNameRule(63),
|
||||
catalog: postgresParameterCatalog,
|
||||
validateCombinations: validatePostgresParameterCombinations,
|
||||
}
|
||||
|
||||
@@ -75,6 +75,39 @@ func TestEngineValidateMasterUsername(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The name is interpolated into a CREATE DATABASE in the guest, so the rule is
|
||||
// narrower than the engine's own: it is what makes the interpolation safe.
|
||||
func TestEngineValidateDBName(t *testing.T) {
|
||||
engine, err := LookupEngine("postgres")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Empty is not a rejection: AWS creates no initial database when none is named.
|
||||
for _, name := range []string{"", "orders", "order_db1", "Orders"} {
|
||||
assert.NoError(t, engine.ValidateDBName(name), "DBName %q", name)
|
||||
}
|
||||
|
||||
cases := map[string]string{
|
||||
"hyphen": "my-db",
|
||||
"leading digit": "1db",
|
||||
"space": "my db",
|
||||
"dot": "my.db",
|
||||
// MariaDB maps a database name onto a directory name, so these are
|
||||
// structurally dangerous there rather than merely awkward.
|
||||
"slash": "my/db",
|
||||
"backslash": `my\db`,
|
||||
"trailing space": "orders ",
|
||||
"quote": "orders'",
|
||||
"too long": strings.Repeat("a", 64),
|
||||
}
|
||||
for name, dbName := range cases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
err := engine.ValidateDBName(dbName)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateMasterUserPassword(t *testing.T) {
|
||||
assert.NoError(t, ValidateMasterUserPassword("Sup3rSecret!"))
|
||||
assert.NoError(t, ValidateMasterUserPassword(strings.Repeat("x", maxMasterPasswordLen)))
|
||||
|
||||
@@ -27,6 +27,7 @@ var engineUnderTest = Engine{
|
||||
reservedUsernames: []string{"root"},
|
||||
reservedUsernamePrefixes: []string{"testdb_"},
|
||||
maxUsernameLen: 80,
|
||||
validateDBName: dbNameRule(64),
|
||||
catalog: buildParameterCatalog(
|
||||
ParameterSpec{
|
||||
Name: "buffer_size", DataType: paramTypeInteger, ApplyType: ApplyTypeStatic,
|
||||
|
||||
@@ -117,6 +117,10 @@ func (s *Service) validateCreateRequest(input *rds.CreateDBInstanceInput) (*vali
|
||||
if err := ValidateMasterUserPassword(masterPassword); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
dbName := aws.StringValue(input.DBName)
|
||||
if err := engine.ValidateDBName(dbName); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
port := engine.DefaultPort
|
||||
if input.Port != nil {
|
||||
@@ -168,7 +172,7 @@ func (s *Service) validateCreateRequest(input *rds.CreateDBInstanceInput) (*vali
|
||||
Port: port,
|
||||
MasterUsername: masterUsername,
|
||||
MasterPassword: masterPassword,
|
||||
DBName: aws.StringValue(input.DBName),
|
||||
DBName: dbName,
|
||||
SecurityGroupIDs: aws.StringValueSlice(input.VpcSecurityGroupIds),
|
||||
DBSubnetGroupName: aws.StringValue(input.DBSubnetGroupName),
|
||||
DBParameterGroupName: paramGroup,
|
||||
|
||||
Reference in New Issue
Block a user