Skip to content

Commit

Permalink
Change implementation of "no newlines in secrets" to use a type alias…
Browse files Browse the repository at this point in the history
… with Pattern, and also create a type for passwords
  • Loading branch information
nward committed May 24, 2021
1 parent d0997ed commit fe940a6
Show file tree
Hide file tree
Showing 14 changed files with 84 additions and 14 deletions.
8 changes: 2 additions & 6 deletions manifests/client.pp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Install FreeRADIUS clients (WISMs or testing servers)
define freeradius::client (
String $secret,
Freeradius::Secret $secret,
Optional[String] $shortname = $title,
Optional[String] $ip = undef,
Optional[String] $ip6 = undef,
Expand All @@ -23,7 +23,7 @@
'other',
]] $nastype = undef,
Optional[String] $login = undef,
Optional[String] $password = undef,
Optional[Freeradius::Password] $password = undef,
Optional[String] $coa_server = undef,
Optional[String] $response_window = undef,
Optional[Integer] $max_connections = undef,
Expand All @@ -42,10 +42,6 @@
$fr_basepath = $::freeradius::params::fr_basepath
$fr_group = $::freeradius::params::fr_group

if ($secret !~ /\A[^\n]+\z/) {
fail('Secrets cannot have newlines in them')
}

file { "${fr_basepath}/clients.d/${shortname}.conf":
ensure => $ensure,
mode => '0640',
Expand Down
4 changes: 2 additions & 2 deletions manifests/home_server.pp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Configure a home_server for proxy config
define freeradius::home_server (
String $secret,
Freeradius::Secret $secret,
Enum['udp', 'tcp'] $proto = 'udp',
Enum['none', 'status-server', 'request'] $status_check = 'none',
Enum['auth', 'acct', 'auth+acct', 'coa'] $type = 'auth',
Expand All @@ -11,7 +11,7 @@
Optional[Integer] $max_outstanding = undef,
Optional[Enum['no', 'yes']] $no_response_fail = undef,
Optional[Integer] $num_answers_to_alive = undef,
Optional[String] $password = undef,
Optional[Freeradius::Password] $password = undef,
Optional[Integer] $port = 1812,
Optional[Integer] $response_window = undef,
Optional[Integer] $revive_interval = undef,
Expand Down
2 changes: 1 addition & 1 deletion manifests/ldap.pp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Configure LDAP support for FreeRADIUS
define freeradius::ldap (
String $identity,
String $password,
Freeradius::Password $password,
String $basedn,
Array[String] $server = ['localhost'],
Integer $port = 389,
Expand Down
2 changes: 1 addition & 1 deletion manifests/module/eap.pp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
Optional[String] $gtc_challenge = undef,
String $gtc_auth_type = 'PAP',
String $tls_config_name = 'tls-common',
Optional[String] $tls_private_key_password = undef,
Optional[Freeradius::Password] $tls_private_key_password = undef,
String $tls_private_key_file = "\${certdir}/server.pem",
String $tls_certificate_file = "\${certdir}/server.pem",
String $tls_ca_file = "\${certdir}/ca.pem",
Expand Down
2 changes: 1 addition & 1 deletion manifests/module/ldap.pp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Array[String] $server = ['localhost'],
Integer $port = 389,
Optional[String] $identity = undef,
Optional[String] $password = undef,
Optional[Freeradius::Password] $password = undef,
Optional[Freeradius::Sasl] $sasl = {},
Optional[String] $valuepair_attribute = undef,
Optional[Array[String]] $update = undef,
Expand Down
2 changes: 1 addition & 1 deletion manifests/sql.pp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Configure SQL support for FreeRADIUS
define freeradius::sql (
Enum['mysql', 'mssql', 'oracle', 'postgresql'] $database,
String $password,
Freeradius::Password $password,
Optional[String] $server = 'localhost',
Optional[String] $login = 'radius',
Optional[String] $radius_db = 'radius',
Expand Down
2 changes: 1 addition & 1 deletion manifests/statusclient.pp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Install FreeRADIUS clients (WISMs or testing servers)
define freeradius::statusclient (
String $secret,
Freeradius::Secret $secret,
Optional[String] $ip = undef,
Optional[String] $ip6 = undef,
Optional[Integer] $port = undef,
Expand Down
14 changes: 13 additions & 1 deletion spec/defines/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,19 @@
end

it do
is_expected.to compile.and_raise_error(%r{Secrets cannot have newlines in them})
is_expected.to compile.and_raise_error(%r{parameter 'secret' expects a match for Freeradius::Secret})
end
end

context 'with password containing a newline' do
let(:params) do
super().merge(
password: "foo\nbar",
)
end

it do
is_expected.to compile.and_raise_error(%r{parameter 'password' expects a match for Freeradius::Password})
end
end
end
24 changes: 24 additions & 0 deletions spec/defines/home_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,28 @@
.with_order('10')
.with_target('/etc/raddb/proxy.conf')
end

context 'with secret containing a newline' do
let(:params) do
super().merge(
secret: "foo\nbar",
)
end

it do
is_expected.to compile.and_raise_error(%r{parameter 'secret' expects a match for Freeradius::Secret})
end
end

context 'with password containing a newline' do
let(:params) do
super().merge(
password: "foo\nbar",
)
end

it do
is_expected.to compile.and_raise_error(%r{parameter 'password' expects a match for Freeradius::Password})
end
end
end
12 changes: 12 additions & 0 deletions spec/defines/module/ldap_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,16 @@
# is_expected.to compile.and_raise_error(%r{^The `use_referral_credentials` parameter requires FreeRADIUS 3.1.x})
# end
# end

context 'with password containing a newline' do
let(:params) do
super().merge(
password: "foo\nbar",
)
end

it do
is_expected.to compile.and_raise_error(%r{parameter 'password' expects a match for Freeradius::Password})
end
end
end
12 changes: 12 additions & 0 deletions spec/defines/sql_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@
# is_expected.to compile.and_raise_error(%r{^The `pool_connect_timeout` parameter requires FreeRADIUS 3.1.x})
# end
# end

context 'with password containing a newline' do
let(:params) do
super().merge(
password: "foo\nbar",
)
end

it do
is_expected.to compile.and_raise_error(%r{parameter 'password' expects a match for Freeradius::Password})
end
end
end
end
end
12 changes: 12 additions & 0 deletions spec/defines/statusclient_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,16 @@
.that_requires('Group[radiusd]')
.that_requires('File[/etc/raddb/clients.d]')
end

context 'with secret containing a newline' do
let(:params) do
super().merge(
secret: "foo\nbar",
)
end

it do
is_expected.to compile.and_raise_error(%r{parameter 'secret' expects a match for Freeradius::Secret})
end
end
end
1 change: 1 addition & 0 deletions types/password.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type Freeradius::Password = Pattern[/\A[^\n]+\z/]
1 change: 1 addition & 0 deletions types/secret.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type Freeradius::Secret = Pattern[/\A[^\n]+\z/]

0 comments on commit fe940a6

Please sign in to comment.