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

Use default ssl_protocols for ssl mailhosts #930

Merged
merged 1 commit into from
Oct 17, 2016
Merged

Use default ssl_protocols for ssl mailhosts #930

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
3 changes: 3 additions & 0 deletions manifests/resource/mailhost.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# [*index_files*] - Default index files for NGINX to read when traversing a directory
# [*ssl*] - Indicates whether to setup SSL bindings for this mailhost.
# [*ssl_cert*] - Pre-generated SSL Certificate file to reference for SSL Support. This is not generated by this module.
# [*ssl_protocols*] - SSL protocols enabled. Defaults to nginx::config::ssl_protocols
# [*ssl_ciphers*] - Override default SSL ciphers (defaults to nginx::config::ssl_ciphers)
# [*ssl_key*] - Pre-generated SSL Key file to reference for SSL Support. This is not generated by this module.
# [*ssl_port*] - Default IP Port for NGINX to listen with this SSL vHost on. Defaults to TCP 443
Expand Down Expand Up @@ -53,6 +54,7 @@
$ipv6_listen_options = 'default ipv6only=on',
$ssl = false,
$ssl_cert = undef,
$ssl_protocols = $::nginx::config::ssl_protocols,
$ssl_ciphers = $::nginx::config::ssl_ciphers,
$ssl_key = undef,
$ssl_port = undef,
Expand Down Expand Up @@ -101,6 +103,7 @@
if ($ssl_cert != undef) {
validate_string($ssl_cert)
}
validate_string($ssl_protocols)
Copy link
Member

Choose a reason for hiding this comment

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

I feel blah about having this as a string rather than an array, but I guess there's a precedent.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree an array is better but this is in line with the existing config and by the principle of least surprise I think this is the better choice.

validate_string($ssl_ciphers)
if ($ssl_key != undef) {
validate_string($ssl_key)
Expand Down
14 changes: 14 additions & 0 deletions spec/defines/resource_mailhost_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,18 @@
value: 'X-Auth-Key "secret_string"',
match: ' auth_http_header X-Auth-Key "secret_string";'
},
{
title: 'should set ssl_protocols',
attr: 'ssl_protocols',
value: 'test-ssl-protocol',
match: ' ssl_protocols test-ssl-protocol;'
},
{
title: 'should set ssl_ciphers',
attr: 'ssl_ciphers',
value: 'test-ssl-ciphers',
match: ' ssl_ciphers test-ssl-ciphers;'
},
{
title: 'should set ssl_certificate',
attr: 'ssl_cert',
Expand All @@ -290,6 +302,8 @@
ssl_port: 587,
ipv6_enable: true,
ssl: true,
ssl_protocols: 'default-protocols',
ssl_ciphers: 'default-ciphers',
ssl_cert: 'dummy.crt',
ssl_key: 'dummy.key'
}
Expand Down
2 changes: 1 addition & 1 deletion templates/mailhost/mailhost_ssl.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ server {
ssl_certificate_key <%= @ssl_key %>;
ssl_session_timeout 5m;

ssl_protocols TLSv1;
ssl_protocols <%= @ssl_protocols %>;
Copy link
Contributor

Choose a reason for hiding this comment

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

The same change applies to the puppet-nginx/templates/mailhost/mailhost.erb file. Because the SSL part of the two templates are equal I used to combine the two into one file that gets included. I'll fix the missing variable in my PR #769.

ssl_ciphers <%= @ssl_ciphers %>;
ssl_prefer_server_ciphers on;
}