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

Advanced client management #36

Merged
merged 4 commits into from
Mar 21, 2015
Merged

Advanced client management #36

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e6e2221
Allow to export separated client config and wrote ovpn_getclient_all.
ypid Mar 12, 2015
fd4a5dc
EASYRSA_PKI might not be defined.
ypid Mar 12, 2015
f431d17
Fixed spelling.
ypid Mar 13, 2015
47cc0e3
Fixed based on the review by @kylemanna. Thanks.
ypid Mar 14, 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
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Upstream links:

* Retrieve the client configuration with embedded certificates

docker run --volumes-from $OVPN_DATA --rm kylemanna/openvpn ovpn_getclient CLIENTNAME > CLIENTNAME.ovpn
docker run --volumes-from $OVPN_DATA --rm kylemanna/openvpn ovpn_getclient CLIENTNAME combined > CLIENTNAME.ovpn
Copy link
Owner

Choose a reason for hiding this comment

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

Can you make the default combined and not specify it? There are existing tutorials that will break (https://www.digitalocean.com/community/tutorials/how-to-run-openvpn-in-a-docker-container-on-ubuntu-14-04) instantly with this change.

Advanced users will easily find the usage / docs for these features. It's distracting to first timer and less advanced users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started with combined being the default, but then I thought, that the other feature might not be visible to the user this way. So I made it explicate. But ok, I will change it back.


* Create an environment variable with the name DEBUG and value of 1 to enable debug output (using "docker -e").

Expand Down Expand Up @@ -105,7 +105,7 @@ packets, etc).
simplicity. It's highly recommended to secure the CA key with some
passphrase to protect against a filesystem compromise. A more secure system
would put the EasyRSA PKI CA on an offline system (can use the same Docker
image to accomplish this).
image and the script ovpn_copy_server_files to accomplish this).
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of mentioning the file, perhaps a pointer to the doc you wrote would be more useful?

* It would be impossible for an adversary to sign bad or forged certificates
without first cracking the key's passphase should the adversary have root
access to the filesystem.
Expand Down
66 changes: 58 additions & 8 deletions bin/ovpn_getclient
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,42 @@
#

if [ "$DEBUG" == "1" ]; then
set -x
set -x
fi

set -e

source "$OPENVPN/ovpn_env.sh"
cn=$1
if [ -z "$OPENVPN" ]; then
Copy link
Owner

Choose a reason for hiding this comment

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

Why would $OPENVPN or $EASYRSA_PKI ever be unset? They are defined in the Dockerfile and overriden with docker run -e .... Is there a situation that makes this necessary? Otherwise it's just adding noise to the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for this is simple. Users (including myself) might run this script outside of docker too.

export OPENVPN="$PWD"
fi
if ! source "$OPENVPN/ovpn_env.sh"; then
echo "Could not source $OPENVPN/ovpn_env.sh."
exit 1
fi
if [ -z "$EASYRSA_PKI" ]; then
export EASYRSA_PKI="$OPENVPN/pki"
fi

cn="$1"
parm="$2"

if [ ! -f "$EASYRSA_PKI/private/${cn}.key" ]; then
echo "Unable to find ${cn}, please try again or generate the key first"
>&2 "Unable to find \"${cn}\", please try again or generate the key first" 1>&2
Copy link
Owner

Choose a reason for hiding this comment

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

This appears to be a typo, did you mean the following?
echo "Unable to find \"${cn}\", please try again or generate the key first" >&2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

exit 1
fi

cat <<EOF
get_client_config() {
mode="$1"
echo "
client
nobind
dev tun
remote-cert-tls server

remote $OVPN_CN $OVPN_PORT $OVPN_PROTO
"
if [ "$mode" == "combined" ]; then
echo "
<key>
$(cat $EASYRSA_PKI/private/${cn}.key)
</key>
Expand All @@ -40,9 +57,16 @@ $(cat $EASYRSA_PKI/dh.pem)
$(cat $EASYRSA_PKI/ta.key)
</tls-auth>
key-direction 1

remote $OVPN_CN $OVPN_PORT $OVPN_PROTO
EOF
"
else
echo "
key ${cn}.key
ca ca.crt
cert ${cn}.crt
dh dh.pem
tls-auth ta.key 1
"
fi

if [ "$OVPN_DEFROUTE" != "0" ];then
echo "redirect-gateway def1"
Expand All @@ -51,3 +75,29 @@ fi
if [ -n "$OVPN_MTU" ]; then
echo "tun-mtu $OVPN_MTU"
fi
}

dir="$OPENVPN/clients/$cn"
case "$parm" in
"separated")
mkdir -p "$dir"
get_client_config "$parm" > "$dir/${cn}.ovpn"
cp "$EASYRSA_PKI/private/${cn}.key" "$dir/${cn}.key"
cp "$EASYRSA_PKI/ca.crt" "$dir/ca.crt"
cp "$EASYRSA_PKI/issued/${cn}.crt" "$dir/${cn}.crt"
cp "$EASYRSA_PKI/dh.pem" "$dir/dh.pem"
cp "$EASYRSA_PKI/ta.key" "$dir/ta.key"
;;
"combined")
get_client_config "combined"
;;
"combined-save")
get_client_config "combined" > "$dir/${cn}-combined.ovpn"
;;
*)
Copy link
Owner

Choose a reason for hiding this comment

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

Default case should be combined as mentioned above to not break existing tutorials.

>&2 echo "This script can produce the client configuration in to formats."
>&2 echo " 1. combined: All needed configuration and cryptographic material is in one file (Use \"combined-save\" to write the configuration file in the same path as the separated parameter does)."
>&2 echo " 2. separated: Separated files."
>&2 echo "Please specific one of those options as second parameter."
;;
Copy link
Owner

Choose a reason for hiding this comment

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

I agree with redirecting stderr, but putting the redirection at the beginning of the line feels strange. Most people expect IO redirection at the end of lines by convention. Approach of least astonishment is preferred here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, redirection at the end of the line is probably more common. Changed it.

esac
25 changes: 25 additions & 0 deletions bin/ovpn_getclient_all
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/bin/bash
## @licence AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>
## @author Copyright (C) 2015 Robin Schneider <[email protected]>

if [ -z "$OPENVPN" ]; then
export OPENVPN="$PWD"
fi
if ! source "$OPENVPN/ovpn_env.sh"; then
echo "Could not source $OPENVPN/ovpn_env.sh."
exit 1
fi
if [ -z "$EASYRSA_PKI" ]; then
export EASYRSA_PKI="$OPENVPN/pki"
fi

pushd "$EASYRSA_PKI"
for name in issued/*.crt; do
name=${name%.crt}
name=${name#issued/}
if [ "$name" != "$OVPN_CN" ]; then
ovpn_getclient "$name" separated
ovpn_getclient "$name" combined-save
fi
done
popd
2 changes: 1 addition & 1 deletion docs/advanced.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ The ovpn_genconfig script is intended for simple configurations that apply to th
docker run --rm -v $PWD:/etc/openvpn -it kylemanna/openvpn ovpn_initpki
vim openvpn.conf
docker run --rm -v $PWD:/etc/openvpn -it kylemanna/openvpn easyrsa build-client-full CLIENTNAME nopass
docker run --rm -v $PWD:/etc/openvpn kylemanna/openvpn ovpn_getclient CLIENTNAME > CLIENTNAME.ovpn
docker run --rm -v $PWD:/etc/openvpn kylemanna/openvpn ovpn_getclient CLIENTNAME combined > CLIENTNAME.ovpn
Copy link
Owner

Choose a reason for hiding this comment

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

Default should be combined.


* Start the server with:

Expand Down
28 changes: 28 additions & 0 deletions docs/clients.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Advanced client management

## Client configuration mode

The `ovpn_getclient` can produce two different versions of the configuration.

1. combined: All needed configuration and cryptographic material is in one file (Use "combined-save" to write the configuration file in the same path as the separated parameter does).
2. separated: Separated files.

Note that some client software might be picky about which configuration format it accepts.

## Batch mode

If you have more than a few clients, you will want to generate and update your client configuration in batch. For this task the script `ovpn_getclient_all` was written, which writes out the configuration for each client to a separate directory called `clients/$cn`.

Execute the following to generate the configuration for all clients:

docker run --rm -t -i -v /tmp/openvpn:/etc/openvpn kylemanna/openvpn ovpn_getclient_all

After doing so, you will find the following files in each of the `$cn` directories:

ca.crt
dh.pem
$cn-combined.ovpn # Combined configuration file format, you your client recognices this file then only this file is needed.
Copy link
Owner

Choose a reason for hiding this comment

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

Grammar / readability issues with "you your client recognices".

$cn.ovpn # Separated configuration. This configuration file requires the other files ca.crt dh.pem $cn.crt $cn.key ta.key
$cn.crt
$cn.key
ta.key
Copy link
Owner

Choose a reason for hiding this comment

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

This looks great! Docs for the win.

You should also elaborate on the expectation that the files are in the volume mount as opposed to a docker volume container that's used everywhere else for this image. This will confuse people not familiar with Docker.