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

Add ability to set ssl-settings globally - fixes #670 #1382

Merged
merged 1 commit into from
May 5, 2020
Merged

Add ability to set ssl-settings globally - fixes #670 #1382

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
20 changes: 17 additions & 3 deletions manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,29 @@
$server_tokens = $nginx::server_tokens
$spdy = $nginx::spdy
$http2 = $nginx::http2
$ssl_buffer_size = $nginx::ssl_buffer_size
$ssl_ciphers = $nginx::ssl_ciphers
$ssl_crl = $nginx::ssl_crl
$ssl_dhparam = $nginx::ssl_dhparam
$ssl_ecdh_curve = $nginx::ssl_ecdh_curve
$ssl_session_cache = $nginx::ssl_session_cache
$ssl_session_timeout = $nginx::ssl_session_timeout
$ssl_session_tickets = $nginx::ssl_session_tickets
$ssl_session_ticket_key = $nginx::ssl_session_ticket_key
$ssl_stapling = $nginx::ssl_stapling
$ssl_stapling_file = $nginx::ssl_stapling_file
$ssl_stapling_responder = $nginx::ssl_stapling_responder
$ssl_stapling_verify = $nginx::ssl_stapling_verify
$ssl_trusted_certificate = $nginx::ssl_trusted_certificate
$ssl_password_file = $nginx::ssl_password_file
$ssl_prefer_server_ciphers = $nginx::ssl_prefer_server_ciphers
$ssl_protocols = $nginx::ssl_protocols
$ssl_verify_depth = $nginx::ssl_verify_depth
$types_hash_bucket_size = $nginx::types_hash_bucket_size
$types_hash_max_size = $nginx::types_hash_max_size
$worker_connections = $nginx::worker_connections
$worker_processes = $nginx::worker_processes
$worker_rlimit_nofile = $nginx::worker_rlimit_nofile
$ssl_prefer_server_ciphers = $nginx::ssl_prefer_server_ciphers
$ssl_protocols = $nginx::ssl_protocols
$ssl_ciphers = $nginx::ssl_ciphers
$include_modules_enabled = $nginx::include_modules_enabled

# Non-configurable settings
Expand Down
17 changes: 15 additions & 2 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@
Enum['on', 'off'] $spdy = 'off',
Enum['on', 'off'] $http2 = 'off',
Enum['on', 'off'] $ssl_stapling = 'off',
Enum['on', 'off'] $ssl_stapling_verify = 'off',
Stdlib::Absolutepath $snippets_dir = $nginx::params::snippets_dir,
Boolean $manage_snippets_dir = true,
$types_hash_bucket_size = '512',
Expand All @@ -161,9 +162,21 @@
Enum['on', 'off'] $ssl_prefer_server_ciphers = 'on',
Variant[Integer, Enum['auto']] $worker_processes = 'auto',
Integer $worker_rlimit_nofile = 1024,
$ssl_protocols = 'TLSv1 TLSv1.1 TLSv1.2',
$ssl_ciphers = 'ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS', # lint:ignore:140chars
String $ssl_protocols = 'TLSv1 TLSv1.1 TLSv1.2',
String $ssl_ciphers = 'ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS', # lint:ignore:140chars
Optional[Stdlib::Unixpath] $ssl_dhparam = undef,
Optional[String] $ssl_ecdh_curve = undef,
String $ssl_session_cache = 'shared:SSL:10m',
String $ssl_session_timeout = '5m',
Optional[Enum['on', 'off']] $ssl_session_tickets = undef,
Optional[Stdlib::Absolutepath] $ssl_session_ticket_key = undef,
Optional[String] $ssl_buffer_size = undef,
Optional[Stdlib::Absolutepath] $ssl_crl = undef,
Optional[Stdlib::Absolutepath] $ssl_stapling_file = undef,
Optional[String] $ssl_stapling_responder = undef,
Optional[Stdlib::Absolutepath] $ssl_trusted_certificate = undef,
Optional[Integer] $ssl_verify_depth = undef,
Optional[Stdlib::Absolutepath] $ssl_password_file = undef,

### START Package Configuration ###
$package_ensure = present,
Expand Down
18 changes: 9 additions & 9 deletions manifests/resource/server.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,32 +172,32 @@
Optional[Variant[String, Boolean]] $ssl_cert = undef,
Optional[String] $ssl_client_cert = undef,
String $ssl_verify_client = 'on',
Optional[String] $ssl_dhparam = $nginx::ssl_dhparam,
Optional[String] $ssl_dhparam = undef,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this change ok, as if $nginx::ssl_dhparam is defined it will now get written to nginx.conf or is this considered a backward-incompatible change even if nginx should be working as before this change?

Copy link
Member

Choose a reason for hiding this comment

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

I do not think this change is backwards-incompatible, because the value was undefbefore and can be overwritten any time. However, you are using a different data type in init.pp and server.pp.

Unfortunately I can not have a look at the failing Travis CI tests, because the website is under maintenance.

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind a breaking change for this since it makes sense IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dhoppe i made the settings in init.pp more strict as these are new parameters. I didn't want to break anything with my changes if somebody is already using nginx::resource::server.

There are also $ssl_stapling and $ssl_stapling_verify which are Enum['on', 'off'] on init.pp and Boolean in server.pp
Unifying them will be definitely a breaking-change and should probably be done in a separate ticket?

If there is any new data type to strict just tell me and i will change it :) I would like to see this change as soon as possible released as we would like to move some ssl-settings vom vhost/servers to the global config in our infrastructure.

Optional[String] $ssl_ecdh_curve = undef,
Boolean $ssl_redirect = false,
Optional[Integer] $ssl_redirect_port = undef,
Optional[Variant[String, Boolean]] $ssl_key = undef,
Integer $ssl_port = 443,
Enum['on', 'off'] $ssl_prefer_server_ciphers = $nginx::ssl_prefer_server_ciphers,
String $ssl_protocols = $nginx::ssl_protocols,
$ssl_buffer_size = undef,
String $ssl_ciphers = $nginx::ssl_ciphers,
String $ssl_cache = 'shared:SSL:10m',
Optional[Enum['on', 'off']] $ssl_prefer_server_ciphers = undef,
Optional[String] $ssl_protocols = undef,
Optional[String] $ssl_buffer_size = undef,
Optional[String] $ssl_ciphers = undef,
Optional[String] $ssl_cache = undef,
Optional[String] $ssl_crl = undef,
Boolean $ssl_stapling = false,
Optional[String] $ssl_stapling_file = undef,
Optional[String] $ssl_stapling_responder = undef,
Boolean $ssl_stapling_verify = false,
String $ssl_session_timeout = '5m',
Optional[String] $ssl_session_tickets = undef,
Optional[String] $ssl_session_timeout = undef,
Optional[Enum['on', 'off']] $ssl_session_tickets = undef,
Optional[String] $ssl_session_ticket_key = undef,
Optional[String] $ssl_trusted_cert = undef,
Optional[Integer] $ssl_verify_depth = undef,
Optional[Stdlib::Absolutepath] $ssl_password_file = undef,
Enum['on', 'off'] $spdy = $nginx::spdy,
Enum['on', 'off'] $http2 = $nginx::http2,
Optional[String] $proxy = undef,
Optional[String]$proxy_redirect = undef,
Optional[String] $proxy_redirect = undef,
String $proxy_read_timeout = $nginx::proxy_read_timeout,
String $proxy_send_timeout = $nginx::proxy_send_timeout,
$proxy_connect_timeout = $nginx::proxy_connect_timeout,
Expand Down
3 changes: 3 additions & 0 deletions spec/acceptance/nginx_server_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class { 'nginx': }
it { is_expected.to be_file }
it { is_expected.not_to contain 'ssl on;' } # As of nginx 1.15 (1.16 stable), this will not be set.
it { is_expected.to contain 'listen *:443 ssl;' }
it { is_expected.not_to contain 'shared:SSL:10m;' }
end

describe file('/etc/nginx/sites-enabled/www.puppetlabs.com.conf') do
Expand Down Expand Up @@ -119,6 +120,7 @@ class { 'nginx': }
nginx::resource::server { 'www.puppetlabs.com':
ensure => present,
ssl => true,
ssl_cache => 'shared:SSL:10m',
ssl_cert => '/etc/pki/tls/certs/crypted.cert',
ssl_key => '/etc/pki/tls/private/crypted.key',
ssl_password_file => '/etc/pki/tls/private/crypted.pass',
Expand All @@ -134,6 +136,7 @@ class { 'nginx': }

describe file('/etc/nginx/sites-available/www.puppetlabs.com.conf') do
it { is_expected.to be_file }
it { is_expected.to contain 'ssl_session_cache shared:SSL:10m;' }
it { is_expected.to contain 'ssl_password_file /etc/pki/tls/private/crypted.pass;' }
end

Expand Down
156 changes: 156 additions & 0 deletions spec/classes/nginx_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,162 @@
attr: 'proxy_busy_buffers_size',
value: '16k',
match: ' proxy_busy_buffers_size 16k;'
},
{
title: 'should set ssl_stapling_verify',
attr: 'ssl_stapling_verify',
value: 'on',
match: ' ssl_stapling_verify on;'
},
{
title: 'should set ssl_protocols',
attr: 'ssl_protocols',
value: 'TLSv1.2',
match: ' ssl_protocols TLSv1.2;'
},
{
title: 'should set ssl_ciphers',
attr: 'ssl_ciphers',
value: 'ECDHE-ECDSA-CHACHA20-POLY1305',
match: ' ssl_ciphers ECDHE-ECDSA-CHACHA20-POLY1305;'
},
{
title: 'should set ssl_dhparam',
attr: 'ssl_dhparam',
value: '/path/to/dhparam',
match: ' ssl_dhparam /path/to/dhparam;'
},
{
title: 'should not set ssl_ecdh_curve',
attr: 'ssl_ecdh_curve',
value: :undef,
notmatch: 'ssl_ecdh_curve'
},
{
title: 'should set ssl_ecdh_curve',
attr: 'ssl_ecdh_curve',
value: 'prime256v1:secp384r1',
match: ' ssl_ecdh_curve prime256v1:secp384r1;'
},
{
title: 'should set ssl_session_cache',
attr: 'ssl_session_cache',
value: 'shared:SSL:10m',
match: ' ssl_session_cache shared:SSL:10m;'
},
{
title: 'should set ssl_session_timeout',
attr: 'ssl_session_timeout',
value: '5m',
match: ' ssl_session_timeout 5m;'
},
{
title: 'should not set ssl_session_tickets',
attr: 'ssl_session_tickets',
value: :undef,
notmatch: 'ssl_session_tickets'
},
{
title: 'should set ssl_session_tickets',
attr: 'ssl_session_tickets',
value: 'on',
match: ' ssl_session_tickets on;'
},
{
title: 'should not set ssl_session_ticket_key',
attr: 'ssl_session_ticket_key',
value: :undef,
notmatch: 'ssl_session_ticket_key'
},
{
title: 'should set ssl_session_ticket_key',
attr: 'ssl_session_ticket_key',
value: '/path/to/ticket_key',
match: ' ssl_session_ticket_key /path/to/ticket_key;'
},
{
title: 'should not set ssl_buffer_size',
attr: 'ssl_buffer_size',
value: :undef,
notmatch: 'ssl_buffer_size'
},
{
title: 'should set ssl_buffer_size',
attr: 'ssl_buffer_size',
value: '16k',
match: ' ssl_buffer_size 16k;'
},
{
title: 'should not set ssl_crl',
attr: 'ssl_crl',
value: :undef,
notmatch: 'ssl_crl'
},
{
title: 'should set ssl_crl',
attr: 'ssl_crl',
value: '/path/to/crl',
match: ' ssl_crl /path/to/crl;'
},
{
title: 'should not set ssl_stapling_file',
attr: 'ssl_stapling_file',
value: :undef,
notmatch: 'ssl_stapling_file'
},
{
title: 'should set ssl_stapling_file',
attr: 'ssl_stapling_file',
value: '/path/to/stapling_file',
match: ' ssl_stapling_file /path/to/stapling_file;'
},
{
title: 'should not set ssl_stapling_responder',
attr: 'ssl_stapling_responder',
value: :undef,
notmatch: 'ssl_stapling_responder'
},
{
title: 'should set ssl_stapling_responder',
attr: 'ssl_stapling_responder',
value: 'http://stapling.responder/',
match: ' ssl_stapling_responder http://stapling.responder/;'
},
{
title: 'should not set ssl_trusted_certificate',
attr: 'ssl_trusted_certificate',
value: :undef,
notmatch: 'ssl_trusted_certificate'
},
{
title: 'should set ssl_trusted_certificate',
attr: 'ssl_trusted_certificate',
value: '/path/to/trusted_cert',
match: ' ssl_trusted_certificate /path/to/trusted_cert;'
},
{
title: 'should not set ssl_verify_depth',
attr: 'ssl_verify_depth',
value: :undef,
notmatch: 'ssl_verify_depth'
},
{
title: 'should set ssl_verify_depth',
attr: 'ssl_verify_depth',
value: 5,
match: ' ssl_verify_depth 5;'
},
{
title: 'should not set ssl_password_file',
attr: 'ssl_password_file',
value: :undef,
notmatch: 'ssl_password_file'
},
{
title: 'should set ssl_password_file',
attr: 'ssl_password_file',
value: '/path/to/password_file',
match: ' ssl_password_file /path/to/password_file;'
}
].each do |param|
context "when #{param[:attr]} is #{param[:value]}" do
Expand Down
55 changes: 55 additions & 0 deletions templates/conf.d/nginx.conf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,61 @@ http {
<% if @fastcgi_cache_use_stale -%>
fastcgi_cache_use_stale <%= @fastcgi_cache_use_stale %>;
<% end -%>

<% if @ssl_dhparam -%>
ssl_dhparam <%= @ssl_dhparam %>;
<% end -%>
<% if @ssl_ecdh_curve -%>
ssl_ecdh_curve <%= @ssl_ecdh_curve %>;
<% end -%>
<% if @ssl_session_cache -%>
ssl_session_cache <%= @ssl_session_cache %>;
<% end -%>
<% if @ssl_session_timeout -%>
ssl_session_timeout <%= @ssl_session_timeout %>;
<% end -%>
<% if @ssl_session_tickets -%>
ssl_session_tickets <%= @ssl_session_tickets %>;
<% end -%>
<% if @ssl_session_ticket_key -%>
ssl_session_ticket_key <%= @ssl_session_ticket_key %>;
<% end -%>
<% if @ssl_buffer_size -%>
ssl_buffer_size <%= @ssl_buffer_size %>;
<% end -%>
<% if @ssl_protocols -%>
ssl_protocols <%= @ssl_protocols %>;
<% end -%>
<% if @ssl_ciphers -%>
ssl_ciphers <%= @ssl_ciphers %>;
<% end -%>
<% if @ssl_prefer_server_ciphers -%>
ssl_prefer_server_ciphers <%= @ssl_prefer_server_ciphers %>;
<% end -%>
<% if @ssl_crl -%>
ssl_crl <%= @ssl_crl %>;
<% end -%>
<% if @ssl_stapling -%>
ssl_stapling <%= @ssl_stapling %>;
<% end -%>
<% if @ssl_stapling_file -%>
ssl_stapling_file <%= @ssl_stapling_file %>;
<% end -%>
<% if @ssl_stapling_responder -%>
ssl_stapling_responder <%= @ssl_stapling_responder %>;
<% end -%>
<% if @ssl_stapling_verify -%>
ssl_stapling_verify <%= @ssl_stapling_verify %>;
<% end -%>
<% if @ssl_trusted_certificate -%>
ssl_trusted_certificate <%= @ssl_trusted_certificate %>;
<% end -%>
<% if @ssl_verify_depth -%>
ssl_verify_depth <%= @ssl_verify_depth %>;
<% end -%>
<% if @ssl_password_file -%>
ssl_password_file <%= @ssl_password_file %>;
<% end -%>
<% if @http_cfg_append -%>

<%- field_width = @http_cfg_append.inject(0) { |l,(k,v)| k.size > l ? k.size : l } -%>
Expand Down
10 changes: 10 additions & 0 deletions templates/server/server_ssl_settings.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
<%- if defined? @ssl_ecdh_curve -%>
ssl_ecdh_curve <%= @ssl_ecdh_curve %>;
<%- end -%>
<% if @ssl_cache -%>
ssl_session_cache <%= @ssl_cache %>;
<% end -%>
<% if @ssl_session_timeout -%>
ssl_session_timeout <%= @ssl_session_timeout %>;
<% end -%>
<% if @ssl_session_tickets -%>
ssl_session_tickets <%= @ssl_session_tickets %>;
<% end -%>
Expand All @@ -28,9 +32,15 @@
<% if @ssl_buffer_size -%>
ssl_buffer_size <%= @ssl_buffer_size %>;
<% end -%>
<% if @ssl_protocols -%>
ssl_protocols <%= @ssl_protocols %>;
<% end -%>
<% if @ssl_ciphers -%>
ssl_ciphers <%= @ssl_ciphers %>;
<% end -%>
<% if @ssl_prefer_server_ciphers -%>
ssl_prefer_server_ciphers <%= @ssl_prefer_server_ciphers %>;
<% end -%>
<% if @ssl_crl -%>
ssl_crl <%= @ssl_crl %>;
<% end -%>
Expand Down