Skip to content

Commit 766508d

Browse files
tuxmeasmortex
tuxmea
and
smortex
authored
pg_hba_rule does not properly verify address parameter
According to PostgreSQL documentation there are only specific data possible for address: https://www.postgresql.org/docs/current/auth-pg-hba-conf.html - IPV4 CIDR - IPV6 CIDR - FQDN - the strings 'samenet' or 'samehost' - a domain - a domain with a starting dot * rubocop cleanup * Update manifests/server/pg_hba_rule.pp Co-authored-by: Romain Tartière <romain@blogreen.org> * Update manifests/server/pg_hba_rule.pp Co-authored-by: Romain Tartière <romain@blogreen.org> * switch to individua, local data type * lint fixes * types may not finish with a trailing comma adding lint:ignore comment * lint fix single whitespace * lint fix next one: there should be a single space before an opening brace * lint: empty space also before curly * adopt spec tests to new data type Co-authored-by: Romain Tartière <romain@blogreen.org>
1 parent 6bad26d commit 766508d

7 files changed

Lines changed: 97 additions & 36 deletions

File tree

‎manifests/backup/pg_dump.pp‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,10 @@
4141
# Weekdays on which the backup job should run. Defaults to `*`. This parameter is passed directly to the cron resource.
4242
#
4343
class postgresql::backup::pg_dump (
44+
String[1] $dir,
4445
Boolean $compress = true,
4546
Array $databases = [],
4647
Boolean $delete_before_dump = false,
47-
String[1] $dir,
4848
String[1] $dir_group = '0',
4949
String[1] $dir_mode = '0700',
5050
String[1] $dir_owner = 'root',
@@ -112,18 +112,19 @@
112112
owner => 'root',
113113
group => '0', # Use GID for compat with Linux and BSD.
114114
content => epp($template, {
115-
compress => $compress,
116-
databases => $databases,
117-
db_user => $db_user,
118-
delete_before_dump => $delete_before_dump,
119-
dir => $dir,
120-
format => $format,
121-
optional_args => $optional_args,
122-
post_script => $post_script,
123-
pre_script => $pre_script,
124-
rotate => $rotate,
125-
success_file_path => $success_file_path,
126-
}),
115+
compress => $compress,
116+
databases => $databases,
117+
db_user => $db_user,
118+
delete_before_dump => $delete_before_dump,
119+
dir => $dir,
120+
format => $format,
121+
optional_args => $optional_args,
122+
post_script => $post_script,
123+
pre_script => $pre_script,
124+
rotate => $rotate,
125+
success_file_path => $success_file_path,
126+
}
127+
),
127128
}
128129

129130
# Create password file for pg_dump
@@ -132,10 +133,11 @@
132133
mode => '0600',
133134
owner => 'root',
134135
group => '0', # Use GID for compat with Linux and BSD.
135-
content => inline_epp('*:*:*:<%= $db_user %>:<%= $db_password %>',{
136-
db_password => $db_password,
137-
db_user => $db_user,
138-
}),
136+
content => inline_epp ( '*:*:*:<%= $db_user %>:<%= $db_password %>', {
137+
db_password => $db_password,
138+
db_user => $db_user,
139+
}
140+
),
139141
show_diff => false,
140142
}
141143

‎manifests/server/default_privileges.pp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
/(?i:^SEQUENCES$)/,
2424
/(?i:^TABLES$)/,
2525
/(?i:^TYPES$)/,
26-
/(?i:^SCHEMAS$)/
26+
/(?i:^SCHEMAS$)/ # lint:ignore:trailing_comma
2727
] $object_type,
2828
String $schema = 'public',
2929
String $psql_db = $postgresql::server::default_database,

‎manifests/server/grant.pp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
/(?i:^TABLE$)/,
3030
#/(?i:^TABLESPACE$)/,
3131
/(?i:^SCHEMA$)/,
32-
/(?i:^SEQUENCE$)/
32+
/(?i:^SEQUENCE$)/ # lint:ignore:trailing_comma
3333
#/(?i:^VIEW$)/
3434
] $object_type = 'database',
3535
Optional[Variant[Array[String,2,2],String[1]]] $object_name = undef,

‎manifests/server/pg_hba_rule.pp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
# @param database Sets a comma-separated list of databases that this rule matches.
66
# @param user Sets a comma-separated list of users that this rule matches.
77
# @param auth_method Provides the method that is used for authentication for the connection that this rule matches. Described further in the PostgreSQL pg_hba.conf documentation.
8-
# @param address Sets a CIDR based address for this rule matching when the type is not 'local'.
8+
# @param address Sets a address for this rule matching when the type is not 'local'. Value can either be IPv4 CIDR, IPv6 CIDR, a FQDN, the strings 'all', 'samehost' or 'samenet' or a domain either with or without starting dot (.) https://www.postgresql.org/docs/current/auth-pg-hba-conf.html
99
# @param description Defines a longer description for this rule, if required. This description is placed in the comments above the rule in pg_hba.conf. Default value: 'none'.
1010
# @param auth_option For certain auth_method settings there are extra options that can be passed. Consult the PostgreSQL pg_hba.conf documentation for further details.
1111
# @param order Sets an order for placing the rule in pg_hba.conf. This can be either a string or an integer. If it is an integer, it will be converted to a string by zero-padding it to three digits. E.g. 42 will be zero-padded to the string '042'. The pg_hba_rule fragments are sorted using the alpha sorting order. Default value: 150.
@@ -16,7 +16,7 @@
1616
String $database,
1717
String $user,
1818
String $auth_method,
19-
Optional[String] $address = undef,
19+
Optional[Postgresql::Pg_hba_rule_address] $address = undef,
2020
String $description = 'none',
2121
Optional[String] $auth_option = undef,
2222
Variant[String, Integer] $order = 150,

‎manifests/server/role.pp‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,16 @@
9595

9696
if $password_sql =~ Deferred {
9797
$create_role_command = Deferred('sprintf', ["CREATE ROLE \"%s\" %s %s %s %s %s %s CONNECTION LIMIT %s",
98-
$username,
99-
$password_sql,
100-
$login_sql,
101-
$createrole_sql,
102-
$createdb_sql,
103-
$superuser_sql,
104-
$replication_sql,
105-
$connection_limit])
98+
$username,
99+
$password_sql,
100+
$login_sql,
101+
$createrole_sql,
102+
$createdb_sql,
103+
$superuser_sql,
104+
$replication_sql,
105+
$connection_limit,
106+
]
107+
)
106108
} else {
107109
$create_role_command = "CREATE ROLE \"${username}\" ${password_sql} ${login_sql} ${createrole_sql} ${createdb_sql} ${superuser_sql} ${replication_sql} CONNECTION LIMIT ${connection_limit}"
108110
}
@@ -152,11 +154,13 @@
152154

153155
if $password_hash_unsensitive and $update_password {
154156
if $password_hash_unsensitive =~ Deferred {
155-
$pwd_hash_sql = Deferred('postgresql::postgresql_password',[$username,
156-
$password_hash,
157-
false,
158-
$hash,
159-
$salt])
157+
$pwd_hash_sql = Deferred ( 'postgresql::postgresql_password', [$username,
158+
$password_hash,
159+
false,
160+
$hash,
161+
$salt,
162+
]
163+
)
160164
}
161165
else {
162166
$pwd_hash_sql = postgresql::postgresql_password(
@@ -170,8 +174,10 @@
170174
if $pwd_hash_sql =~ Deferred {
171175
$pw_command = Deferred('sprintf', ["ALTER ROLE \"%s\" ENCRYPTED PASSWORD '%s'", $username, $pwd_hash_sql])
172176
$unless_pw_command = Deferred('sprintf', ["SELECT 1 FROM pg_shadow WHERE usename = '%s' AND passwd = '%s'",
173-
$username,
174-
$pwd_hash_sql])
177+
$username,
178+
$pwd_hash_sql,
179+
]
180+
)
175181
} else {
176182
$pw_command = "ALTER ROLE \"${username}\" ENCRYPTED PASSWORD '${pwd_hash_sql}'"
177183
$unless_pw_command = "SELECT 1 FROM pg_shadow WHERE usename = '${username}' AND passwd = '${pwd_hash_sql}'"

‎spec/defines/server/pg_hba_rule_spec.rb‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,5 +225,48 @@ class { 'postgresql::server': }
225225
is_expected.to contain_concat__fragment('pg_hba_rule_test').with(order: '1234')
226226
end
227227
end
228+
229+
context 'pg_hba_rule with dot domain' do
230+
let :pre_condition do
231+
<<-MANIFEST
232+
class { 'postgresql::server': }
233+
MANIFEST
234+
end
235+
236+
let :params do
237+
{
238+
type: 'host',
239+
database: 'all',
240+
user: 'all',
241+
address: '.domain.tld',
242+
auth_method: 'md5',
243+
target: target,
244+
}
245+
end
246+
247+
it do
248+
is_expected.to contain_concat__fragment('pg_hba_rule_test').with(content: %r{host\s+all\s+all\s+\.domain\.tld\s+md5})
249+
end
250+
end
251+
context 'pg_hba_rule with illegal address' do
252+
let :pre_condition do
253+
<<-MANIFEST
254+
class { 'postgresql::server': }
255+
MANIFEST
256+
end
257+
258+
let :params do
259+
{
260+
type: 'host',
261+
database: 'all',
262+
user: 'all',
263+
address: '/45',
264+
auth_method: 'md5',
265+
target: target,
266+
}
267+
end
268+
269+
it { is_expected.to compile.and_raise_error(%r{parameter 'address' expects a Postgresql::Pg_hba_rule_address}) }
270+
end
228271
end
229272
end

‎types/pg_hba_rule_address.pp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# @summary Supported address types
2+
# @see https://www.postgresql.org/docs/current/auth-pg-hba-conf.html
3+
type Postgresql::Pg_hba_rule_address = Variant[
4+
Stdlib::IP::Address::V4::CIDR,
5+
Stdlib::IP::Address::V6::CIDR,
6+
Stdlib::Fqdn,
7+
Enum['all', 'samehost', 'samenet'],
8+
# RegExp for a DNS domain - also starting with a single dot
9+
Pattern[/^\.(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9]$/],
10+
]

0 commit comments

Comments
 (0)