Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nrpe/ssl] manage ssl protocols even if ssl authentication is unused #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[nrpe/ssl] manage ssl protocols even if ssl authentication is unused #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@
order => '01',
}

if $nrpe::ssl_cert_file_content {
contain nrpe::config::ssl
}
contain nrpe::config::ssl

concat::fragment { 'nrpe includedir':
target => $nrpe::config,
Expand Down
66 changes: 35 additions & 31 deletions manifests/config/ssl.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
content => epp(
'nrpe/nrpe.cfg-ssl.epp',
{
'ssl_version' => $nrpe::ssl_version,
'ssl_ciphers' => $nrpe::ssl_ciphers,
'nrpe_ssl_dir' => $nrpe::nrpe_ssl_dir,
'ssl_client_certs' => $_ssl_client_certs,
'ssl_logging' => nrpe::ssl_logging(
'ssl_version' => $nrpe::ssl_version,
'ssl_use_adh' => $nrpe::ssl_use_adh,
'ssl_ciphers' => $nrpe::ssl_ciphers,
'nrpe_ssl_dir' => $nrpe::nrpe_ssl_dir,
'ssl_client_certs' => $_ssl_client_certs,
'ssl_cert_file_content' => $nrpe::ssl_cert_file_content,
'ssl_logging' => nrpe::ssl_logging(
$nrpe::ssl_log_startup_params,
$nrpe::ssl_log_remote_ip,
$nrpe::ssl_log_protocol_version,
Expand All @@ -30,31 +32,33 @@
order => '02',
}

file { $nrpe::nrpe_ssl_dir:
ensure => directory,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0750',
}
file { "${nrpe::nrpe_ssl_dir}/ca-cert.pem":
ensure => file,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0640',
content => $nrpe::ssl_cacert_file_content,
}
file { "${nrpe::nrpe_ssl_dir}/nrpe-cert.pem":
ensure => file,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0640',
content => $nrpe::ssl_cert_file_content,
}
file { "${nrpe::nrpe_ssl_dir}/nrpe-key.pem":
ensure => file,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0640',
content => $nrpe::ssl_privatekey_file_content,
if $nrpe::ssl_cert_file_content {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition being on $nrpe::ssl_cert_file_content only looks odd. Regarding TLS in general, I would expect that either CA+Cert+Key are provided or none of them, and anything else is invalid and raise an error. Some systems might have an optional CA / CRL. Here the certificate without a key looks odd.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simply move if condition check from parent file to this one.

file { $nrpe::nrpe_ssl_dir:
ensure => directory,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0750',
}
file { "${nrpe::nrpe_ssl_dir}/ca-cert.pem":
ensure => file,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0640',
content => $nrpe::ssl_cacert_file_content,
}
file { "${nrpe::nrpe_ssl_dir}/nrpe-cert.pem":
ensure => file,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0640',
content => $nrpe::ssl_cert_file_content,
}
file { "${nrpe::nrpe_ssl_dir}/nrpe-key.pem":
ensure => file,
owner => 'root',
group => $nrpe::nrpe_group,
mode => '0640',
content => $nrpe::ssl_privatekey_file_content,
}
}
}
3 changes: 3 additions & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@
# A string containing the SSL CA Cert file contents.
# @param ssl_version
# The SSL Version to use. The default of `TLSv1.2+` is the most secure option available at time of writing. Avoid having to set it to a lower value if possible.
# @param ssl_use_adh
# Use anonymous diffie-hellman key exchange. The default is `2` as required (1 means enabled)
# @param ssl_ciphers
# An array of ciphers that should be allowed by NRPE. The defaults are for RSA keys and were taken from https://github.com/ssllabs/research/wiki/SSL-and-TLS-Deployment-Best-Practices.
# @param ssl_client_certs
Expand Down Expand Up @@ -124,6 +126,7 @@
Optional[String[1]] $ssl_privatekey_file_content = undef,
Optional[String[1]] $ssl_cacert_file_content = undef,
Nrpe::Sslversion $ssl_version = $nrpe::params::ssl_version,
Integer[0, 2] $ssl_use_adh = $nrpe::params::ssl_use_adh,
Array[String[1]] $ssl_ciphers = $nrpe::params::ssl_ciphers,
Enum['no','ask','require'] $ssl_client_certs = $nrpe::params::ssl_client_certs,
Boolean $ssl_log_startup_params = false,
Expand Down
4 changes: 4 additions & 0 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@
$allow_weak_random_seed = true

$ssl_version = 'TLSv1.2+'

# adh is required with cipher list below
# see: https://github.com/NagiosEnterprises/nrpe/issues/119
$ssl_use_adh = 2
$ssl_ciphers = [
'DHE-RSA-AES128-GCM-SHA256',
'DHE-RSA-AES256-GCM-SHA384',
Expand Down
12 changes: 11 additions & 1 deletion templates/nrpe.cfg-ssl.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
<%- |
$ssl_version,
Integer[0, 2] $ssl_use_adh,
Array[String[1]] $ssl_ciphers,
Stdlib::Absolutepath $nrpe_ssl_dir,
Enum['0','1','2'] $ssl_client_certs,
Pattern[/0x\h\h/] $ssl_logging,
Optional[String[1]] $ssl_cert_file_content,
| -%>

# SSL/TLS OPTIONS
Expand All @@ -21,6 +23,13 @@

ssl_version=<%= $ssl_version %>

# SSL USE ADH
# This is for backward compatibility and is DEPRECATED. Set to 1 to enable
# ADH or 2 to require ADH. 1 is currently the default but will be changed
# in a later version.

ssl_use_adh=<%= $ssl_use_adh %>

# SSL CIPHER LIST
# This lists which ciphers can be used. For backward compatibility, this
# defaults to 'ssl_cipher_list=ALL:!MD5:@STRENGTH' in this version but
Expand All @@ -30,8 +39,8 @@ ssl_version=<%= $ssl_version %>
#ssl_cipher_list=ALL:!aNULL:!eNULL:!SSLv2:!LOW:!EXP:!RC4:!MD5:@STRENGTH
ssl_cipher_list=<%= $ssl_ciphers.join(':') %>

<% if $ssl_cert_file_content { %>
# SSL Certificate and Private Key Files

ssl_cacert_file=<%= $nrpe_ssl_dir%>/ca-cert.pem
ssl_cert_file=<%= $nrpe_ssl_dir%>/nrpe-cert.pem
ssl_privatekey_file=<%= $nrpe_ssl_dir%>/nrpe-key.pem
Expand All @@ -43,6 +52,7 @@ ssl_privatekey_file=<%= $nrpe_ssl_dir%>/nrpe-key.pem
# 2 = Require client certificates

ssl_client_certs=<%= $ssl_client_certs %>
<% } %>

# SSL LOGGING
# This option determines which SSL messages are send to syslog. OR values
Expand Down