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

Jump to bottom

Add TLS and x509-name verification support #118

Merged
merged 6 commits into from
Feb 2, 2015
Merged

Add TLS and x509-name verification support #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b189c07
Add TLS and x509-name verification support
maetthu-indermuehle Jan 7, 2015
ad1456e
Add TLS support
maetthu-indermuehle Jan 9, 2015
64d7bf0
Add TLS support
maetthu-indermuehle Jan 9, 2015
882f701
Add TLS and x509-name verification support
maetthu-indermuehle Jan 12, 2015
a7c78a0
Merge remote-tracking branch 'upstream/master'
maetthu-indermuehle Jan 13, 2015
130cab5
Add TLS and x509-name verification support
maetthu-indermuehle Jan 16, 2015
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Puppet module to manage OpenVPN servers
* Support for multiple server instances
* Support for LDAP-Authentication
* Support for server instance in client mode
* Support for TLS

## Supported OS

Expand Down
14 changes: 14 additions & 0 deletions manifests/ca.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@
# and KEY_CN in vars
# Default: None
#
# [*tls_auth*]
# Boolean. Determins if a tls key is generated
# Default: False
#
# === Examples
#
# openvpn::ca {
Expand Down Expand Up @@ -99,6 +103,7 @@
$key_cn = '',
$key_name = '',
$key_ou = '',
$tls_auth = false,
) {

include openvpn
Expand Down Expand Up @@ -194,6 +199,15 @@
provider => 'shell',
require => Exec["generate server cert ${name}"],
}
if $tls_auth {
exec { "generate tls key for ${name}":
command => "openvpn --genkey --secret keys/ta.key",
cwd => "/etc/openvpn/${name}/easy-rsa",
creates => "/etc/openvpn/${name}/easy-rsa/keys/ta.key",
provider => 'shell',
require => Exec["generate server cert ${name}"],
}
}

file { "/etc/openvpn/${name}/easy-rsa/keys/crl.pem":
ensure => link,
Expand Down
22 changes: 22 additions & 0 deletions manifests/client.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,17 @@
# Boolean. Set if username and password required
# Default: false
#
# [*tls_auth*]
# Boolean. Activates tls-auth to Add an additional layer of HMAC
# authentication on top of the TLS control channel to protect
# against DoS attacks. This has to be set to the same value as on the
# Server
# Default: false
#
# [*x509_name*]
# Common name of openvpn server to make an x509-name verification
# Default: undef
#
# [*setenv*]
# Hash. Set a custom environmental variable name=value to pass to script.
# Default: {}
Expand Down Expand Up @@ -165,6 +176,8 @@
$setenv_safe = {},
$up = '',
$down = '',
$tls_auth = false,
$x509_name = undef,
$sndbuf = undef,
$rcvbuf = undef,
) {
Expand Down Expand Up @@ -207,6 +220,15 @@
target => "/etc/openvpn/${server}/easy-rsa/keys/ca.crt",
require => Exec["generate certificate for ${name} in context of ${server}"],
}
if $tls_auth {
file { "/etc/openvpn/${server}/download-configs/${name}/keys/${name}/ta.key":
ensure => link,
target => "/etc/openvpn/${server}/easy-rsa/keys/ta.key",
require => Exec["generate certificate for ${name} in context of ${server}"],
before => Exec["tar the thing ${server} with ${name}"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Please specify the dependency on the other side (Exec["tar the thing ${server} with ${name}"]) with require.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I do this I have to copy the whole code since this file is only required as long as tls_auth is true, otherwise it is not required. Or do you have any other suggestion how to solve that nicely?

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right... My bad.

notify => Exec["tar the thing ${server} with ${name}"],
}
}

file { "/etc/openvpn/${server}/download-configs/${name}/${name}.conf":
owner => root,
Expand Down
24 changes: 21 additions & 3 deletions manifests/server.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,17 @@
# and KEY_CN in vars
# Default: None
#
# [*tls_auth*]
# Boolean. Activates tls-auth to Add an additional layer of HMAC
# authentication on top of the TLS control channel to protect
# against DoS attacks.
# Default: false
#
# [*tls_server*]
# Boolean. If proto not tcp it lets you choose if the parameter
# tls-server is set or not.
# Default: false
#
# [*server_poll_timeout*]
# Integer. Value for timeout before trying the next server.
# Default: undef
Expand Down Expand Up @@ -378,6 +389,8 @@
$cipher = '',
$persist_key = false,
$persist_tun = false,
$tls_auth = false,
$tls_server = false,
$server_poll_timeout = undef,
$ping_timer_rem = false,
$sndbuf = undef,
Expand All @@ -389,9 +402,13 @@
Openvpn::Server[$name] ~>
Class['openvpn::service']

$tls_server = $proto ? {
/tcp/ => true,
default => false
if $tls_server {
$real_tls_server = $tls_server
} else {
$real_tls_server = $proto ? {
/tcp/ => true,
default => false
}
}

$group_to_set = $group ? {
Expand Down Expand Up @@ -439,6 +456,7 @@
key_cn => $key_cn,
key_name => $key_name,
key_ou => $key_ou,
tls_auth => $tls_auth,
}
} else {
# VPN Client Mode
Expand Down
11 changes: 8 additions & 3 deletions spec/defines/openvpn_client_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^verb\s+3$/)}
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^mute\s+20$/)}
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^auth-retry\s+none$/)}

it { should_not contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^tls-client$/)}
it { should_not contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^verify-x509-name\s+"test_server"\s+name$/)}
it { should_not contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^sndbuf/)}
it { should_not contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^rcvbuf/)}
end
Expand All @@ -82,8 +83,10 @@
'verb' => '1',
'setenv' => {'CLIENT_CERT' => '0'},
'setenv_safe' => {'FORWARD_COMPATIBLE' => '1'},
'sndbuf' => 393216,
'rcvbuf' => 393215,
'tls_auth' => true,
'x509_name' => 'test_server',
'sndbuf' => 393216,
'rcvbuf' => 393215,
} }
let(:facts) { {
:fqdn => 'somehost',
Expand All @@ -108,6 +111,8 @@
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^setenv\s+CLIENT_CERT\s+0$/)}
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^setenv_safe\s+FORWARD_COMPATIBLE\s+1$/)}
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^cipher\s+BF-CBC$/)}
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^tls-client$/)}
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^verify-x509-name\s+"test_server"\s+name$/)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add some reverse tests? So that it does not appear in the config if the parameter has not been specified?

it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^sndbuf\s+393216$/)}
it { should contain_file('/etc/openvpn/test_server/download-configs/test_client/test_client.conf').with_content(/^rcvbuf\s+393215$/)}
end
Expand Down
9 changes: 8 additions & 1 deletion spec/defines/openvpn_server_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@
it { should_not contain_file('/etc/openvpn/test_server.conf').with_content(/persist-tun/) }
it { should_not contain_file('/etc/openvpn/test_server.conf').with_content(%r{^duplicate-cn$}) }
it { should_not contain_file('/etc/openvpn/test_server.conf').with_content(/^ns-cert-type server/) }
it { should_not contain_file('/etc/openvpn/test_server.conf').with_content(%r{^tls-auth\s+/etc/openvpn/test_server/keys/ta.key\s+0$}) }

end

context "creating a server setting all parameters" do
Expand Down Expand Up @@ -143,6 +145,8 @@
'persist_key' => true,
'persist_tun' => true,
'duplicate_cn' => true,
'tls_auth' => true,
'tls_server' => true,
} }

let(:facts) { {
Expand Down Expand Up @@ -189,6 +193,8 @@
it { should contain_file('/etc/openvpn/test_server.conf').with_content(%r{^down "/tmp/down"$}) }
it { should contain_file('/etc/openvpn/test_server.conf').with_content(%r{^script-security 2$}) }
it { should contain_file('/etc/openvpn/test_server.conf').with_content(%r{^duplicate-cn$}) }
it { should contain_file('/etc/openvpn/test_server.conf').with_content(%r{^tls-server$}) }
it { should contain_file('/etc/openvpn/test_server.conf').with_content(%r{^tls-auth\s+/etc/openvpn/test_server/keys/ta.key\s+0$}) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add some reverse tests? So that it does not appear in the config if the parameter has not been specified?


it { should_not contain_file('/etc/openvpn/test_server.conf').with_content(/^server-poll-timeout/) }
it { should_not contain_file('/etc/openvpn/test_server.conf').with_content(/^ping-timer-rem/) }
Expand All @@ -209,7 +215,8 @@
:key_expire => 365,
:key_cn => 'yolo',
:key_name => 'burp',
:key_ou => 'NSA')
:key_ou => 'NSA',
:tls_auth => true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a trailing comma so we have less changes next time.

}

end
Expand Down
11 changes: 11 additions & 0 deletions templates/client.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,14 @@ up "<%= @up %>"
<% if @down != '' -%>
down "<%= @down %>"
<% end -%>
<% if @tls_auth -%>

# tls authentification
tls-client
tls-auth keys/<%= @name %>/ta.key 1
<% end -%>
<% if @x509_name -%>

# x509 name verification
verify-x509-name "<%= @x509_name %>" name
<% end -%>
7 changes: 6 additions & 1 deletion templates/server.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ proto <%= @proto %>-server
proto <%= @proto %>
<% end -%>
port <%= @port %>
<% if @tls_server -%>
<% if @real_tls_server -%>
tls-server
<% end -%>
<% if @compression != '' -%>
Expand Down Expand Up @@ -126,3 +126,8 @@ duplicate-cn
<% if @ping_timer_rem -%>
ping-timer-rem
<% end -%>
<% if @tls_auth -%>

# tls authentification
tls-auth /etc/openvpn/<%= @name %>/keys/ta.key 0
<% end -%>