[RFC,1/2] Add vpn-ipsec functions
Message ID | 1501090354-17617-1-git-send-email-jonatan.schlag@ipfire.org |
---|---|
State | New |
Headers |
Return-Path: <network-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (unknown [172.28.1.200]) by web02.ipfire.org (Postfix) with ESMTP id 6C42F61C7D for <patchwork@ipfire.org>; Wed, 26 Jul 2017 19:32:41 +0200 (CEST) Received: from mail01.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id 3E60727C8; Wed, 26 Jul 2017 19:32:41 +0200 (CEST) Received: from ipfire.localdomain (dslb-088-073-218-016.088.073.pools.vodafone-ip.de [88.73.218.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 0C55027C8; Wed, 26 Jul 2017 19:32:37 +0200 (CEST) From: Jonatan Schlag <jonatan.schlag@ipfire.org> To: network@lists.ipfire.org Subject: [RFC PATCH 1/2] Add vpn-ipsec functions Date: Wed, 26 Jul 2017 19:32:33 +0200 Message-Id: <1501090354-17617-1-git-send-email-jonatan.schlag@ipfire.org> X-Mailer: git-send-email 2.6.3 X-BeenThere: network@lists.ipfire.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List for the network package <network.lists.ipfire.org> List-Unsubscribe: <http://lists.ipfire.org/mailman/options/network>, <mailto:network-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/network/> List-Post: <mailto:network@lists.ipfire.org> List-Help: <mailto:network-request@lists.ipfire.org?subject=help> List-Subscribe: <http://lists.ipfire.org/mailman/listinfo/network>, <mailto:network-request@lists.ipfire.org?subject=subscribe> Errors-To: network-bounces@lists.ipfire.org Sender: "network" <network-bounces@lists.ipfire.org> |
Message
Jonatan Schlag
July 27, 2017, 3:32 a.m. UTC
Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
Makefile.am | 1 +
src/functions/functions.vpn-ipsec | 550 ++++++++++++++++++++++++++++++++++++++
2 files changed, 551 insertions(+)
create mode 100644 src/functions/functions.vpn-ipsec
Comments
Hi, On Wed, 2017-07-26 at 19:32 +0200, Jonatan Schlag wrote: > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org> > --- > Makefile.am | 1 + > src/functions/functions.vpn-ipsec | 550 > ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 551 insertions(+) > create mode 100644 src/functions/functions.vpn-ipsec > > diff --git a/Makefile.am b/Makefile.am > index 761e849..c14bb17 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -160,6 +160,7 @@ dist_network_SCRIPTS = \ > src/functions/functions.usb \ > src/functions/functions.util \ > src/functions/functions.vlan \ > + src/functions/functions.vpn-ipsec \ > src/functions/functions.vpn-security-policies \ > src/functions/functions.wireless \ > src/functions/functions.wpa_supplicant \ > diff --git a/src/functions/functions.vpn-ipsec > b/src/functions/functions.vpn-ipsec > new file mode 100644 > index 0000000..2bfa3a2 > --- /dev/null > +++ b/src/functions/functions.vpn-ipsec > @@ -0,0 +1,550 @@ > +#!/bin/bash > +#################################################################### > ########### > +# > # > +# IPFire.org - A linux based > firewall # > +# Copyright (C) 2017 IPFire Network Development > Team # > +# > # > +# This program is free software: you can redistribute it and/or > modify # > +# it under the terms of the GNU General Public License as published > by # > +# the Free Software Foundation, either version 3 of the License, > or # > +# (at your option) any later > version. # > +# > # > +# This program is distributed in the hope that it will be > useful, # > +# but WITHOUT ANY WARRANTY; without even the implied warranty > of # > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See > the # > +# GNU General Public License for more > details. # > +# > # > +# You should have received a copy of the GNU General Public > License # > +# along with this program. If not, see <http://www.gnu.org/licenses > />. # > +# > # > +#################################################################### > ########### > + > +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="AUTH_MODE INACTIVITY_TIMEOUT > LOCAL_ID LOCAL_PREFIX" > +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="${VPN_IPSEC_CONNECTION_CONFIG_ > SETTINGS} MODE PEER PSK" > +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="${VPN_IPSEC_CONNECTION_CONFIG_ > SETTINGS} REMOTE_ID REMOTE_PREFIX" > +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="${VPN_IPSEC_CONNECTION_CONFIG_ > SETTINGS} SECURITY_POLICY" LOL. This is a big paragraph to open... > +# Default values > +MODE="tunnel" > +AUTH_MODE="psk" > +INACTIVITY_TIMEOUT="0" > +SECURITY_POLICY="system" I think there is no need for these to be set here. Especially since variable names like MODE are quite generic. I think they should be copied into local variables in the *_new() function. You can have something like IPSEC_DEFAULT_MODE=... in the header maybe to make them easier to modify if we need to. But I am not sure if that is necessary. Should the functions be called vpn_ipsec_* or is just ipsec_* not enough? > +# This function writes all values to a via ${connection} > specificated VPN IPsec configuration file > +vpn_ipsec_connection_write_config() { > + assert [ $# -ge 1 ] > + > + local connection="${1}" > + > + if ! vpn_ipsec_connection_exists "${connection}"; then > + log ERROR "No such VPN IPsec connection: > ${connection}" > + return ${EXIT_ERROR} > + fi > + > + local path="$(vpn_ipsec_connection_path "${connection}")" > + if [ ! -w ${path} ]; then > + log ERROR "${path} is not writeable" > + return ${EXIT_ERROR} > + fi > + > + if ! settings_write "${path}" > ${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS}; then > + log ERROR "Could not write configuration settings > for VPN IPsec connection ${connection}" > + return ${EXIT_ERROR} > + fi > + > + # TODO everytime we successfully write a config we should > call some trigger to take the changes into effect > +} I think you can add the trigger already as an empty function called vpn_ipsec_reload that takes the connection name as an argument. > + > +# This funtion writes the value for one key to a via ${connection} > specificated VPN IPsec connection configuration file > +vpn_ipsec_connection_write_config_key() { > + assert [ $# -ge 3 ] > + > + local connection=${1} > + local key=${2} > + shift 2 > + > + local value="$@" > + > + if ! vpn_ipsec_connection_exists "${connection}"; then > + log ERROR "No such VPN ipsec connection: > ${connection}" > + return ${EXIT_ERROR} > + fi > + > + log DEBUG "Set '${key}' to new value '${value}' in VPN ipsec > connection '${connection}'" > + > + local ${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS} > + > + # Read the config settings > + if ! vpn_ipsec_connection_read_config "${connection}"; then > + return ${EXIT_ERROR} > + fi > + > + # Set the key to a new value > + assign "${key}" "${value}" > + > + if ! vpn_ipsec_connection_write_config "${connection}"; then > + return ${EXIT_ERROR} > + fi > + > + return ${EXIT_TRUE} > +} > + > +# Reads one or more keys out of a settings file or all if no key is > provided. > +vpn_ipsec_connection_read_config() { > + assert [ $# -ge 1 ] > + > + local connection="${1}" > + shift 1 > + > + if ! vpn_ipsec_connection_exists "${connection}"; then > + log ERROR "No such VPN IPsec connection : > ${connection}" > + return ${EXIT_ERROR} > + fi > + > + > + local args > + if [ $# -eq 0 ] && [ -n > "${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS}" ]; then > + list_append args > ${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS} > + else > + list_append args $@ > + fi > + > + local path="$(vpn_ipsec_connection_path ${connection})" > + > + if ! settings_read "${path}" ${args}; then > + log ERROR "Could not read settings for VPN IPsec > connection ${connection}" > + return ${EXIT_ERROR} > + fi > +} > + > +# Returns the path to a the configuration for given connection > +vpn_ipsec_connection_path() { > + assert [ $# -eq 1 ] > + > + local connection=${1} > + > + echo > "${NETWORK_CONFIG_DIR}/vpn/ipsec/connection/${connection}" > +} Should we not use a directory to store certificates, a description and so on? > + > +# This function checks if a vpn ipsec connection exists > +# Returns True when yes and false when not > +vpn_ipsec_connection_exists() { > + assert [ $# -eq 1 ] > + > + local connection=${1} > + > + local path=$(vpn_ipsec_connection_path "${connection}") > + > + [ -f ${path} ] && return ${EXIT_TRUE} || return > ${EXIT_FALSE} > +} Always use "" when you are passing an argument to [. > +# Handle the cli after authentification > +vpn_ipsec_connection_authentication() { > + if [ ! $# -gt 1 ]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + > + local connection=${1} > + local cmd=${2} > + shift 2 > + > + case ${cmd} in > + mode) > + vpn_ipsec_connection_authentication_mode > ${connection} $@ > + ;; > + pre-shared-key) > + vpn_ipsec_connection_authentication_psk > ${connection} $@ > + ;; > + *) > + log ERROR "Unrecognized argument: ${cmd}" > + return ${EXIT_ERROR} > + ;; > + esac > +} > + > +# Set the authentification mode > +vpn_ipsec_connection_authentication_mode() { > + if [ ! $# -eq 2]; then Space missing after the 2. > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + local connection=${1} > + local mode=${2} > + > + if ! isoneof mode "psk" "PSK"; then > + log ERROR "Auth mode '${mode}' is not valid" > + return ${EXIT_ERROR} > + fi > + > + if ! vpn_ipsec_connection_write_config_key ${connection} > "AUTH_MODE" ${mode,,}; then > + log ERROR "Could not write configuration settings" > + return ${EXIT_ERROR} > + fi > +} > + > +# Set the psk > +vpn_ipsec_connection_authentication_psk() { > + if [ ! $# -eq 2]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + local connection=${1} > + local psk=${2} > + > + # TODO Check if psk is valid > + > + if ! vpn_ipsec_connection_write_config_key ${connection} > "PSK" ${psk}; then > + log ERROR "Could not write configuration settings" > + return ${EXIT_ERROR} > + fi > + > + > +} I think there is a return missing at the end. What is a valid PSK? > +# Handle the cli after local > +vpn_ipsec_connection_local() { > + if [ ! $# -ge 2 ]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + > + local connection=${1} > + local cmd=${2} > + shift 2 > + > + case ${cmd} in > + prefix) > + vpn_ipsec_connection_prefix ${connection} > "LOCAL" $@ > + ;; > + id) > + vpn_ipsec_connection_id ${connection} > "LOCAL" $@ > + ;; Ideally I would like these in alphabetical order. Makes solving merge conflicts easier. > + *) > + log ERROR "Unrecognized argument: ${cmd}" > + return ${EXIT_ERROR} > + ;; > + esac > + > + return ${EXIT_OK} > + > +} > + > +# Set the connection mode > +vpn_ipsec_connection_mode() { > + if [ ! $# -eq 2]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + local connection=${1} > + local mode=${2} > + > + if ! isoneof mode "tunnel" "vti" "gre-transport"; then > + log ERROR "Mode '${mode}' is not valid" > + return ${EXIT_ERROR} > + fi We should have a variable for all valid modes at the top of the file. > + > + if ! vpn_ipsec_connection_write_config_key ${connection} > "MODE" ${mode}; then > + log ERROR "Could not write configuration settings" > + return ${EXIT_ERROR} > + fi > + > +} Same here. Either no empty line or a return... > + > +# Set the peer to connect to > +vpn_ipsec_connection_peer() { > + if [ ! $# -eq 2]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + local connection=${1} > + local peer=${2} > + > + if ! vpn_ipsec_connection_check_peer ${peer}; then > + log ERROR "Peer '${peer}' is not valid" > + return ${EXIT_ERROR} > + fi There is a word for "not valid". It is "invalid". > + > + if ! vpn_ipsec_connection_write_config_key ${connection} > "PEER" ${peer}; then At the top you used "" around ${connection}. I think that would be a good idea generally. > + log ERROR "Could not write configuration settings" > + return ${EXIT_ERROR} > + fi > +} > + > +#Set the local or remote id > +vpn_ipsec_connection_id() { > + if [ ! $# -eq 3]; then Space. > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + local connection=${1} > + local type=${2} > + local id=${3} > + > + if ! vpn_ipsec_connection_check_id ${id}; then > + log ERROR "Id '${id}' is not valid" > + return ${EXIT_ERROR} > + fi > + > + if ! vpn_ipsec_connection_write_config_key ${connection} > "${type}_ID" ${id}; then > + log ERROR "Could not write configuration settings" > + return ${EXIT_ERROR} > + fi > + > + return ${EXIT_OK} > +} > + > +# Set the local or remote prefix > +vpn_ipsec_connection_prefix() { > + if [ ! $# -ge 3 ]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + local connection=${1} > + local type=${2} > + shift 2 > + > + local _prefix="${type}_PREFIX" > + local "${_prefix}" > + if ! vpn_ipsec_connection_read_config ${connection} > "${_prefix}"; then > + return ${EXIT_ERROR} > + fi > + > + # Remove duplicated entries to proceed the list safely > + assign "${_prefix}" "$(list_unique ${!_prefix} )" > + > + local prefixes_added > + local prefixes_removed > + local prefixes_set > + > + while [ $# -gt 0 ]; do > + local arg="${1}" > + > + case "${arg}" in > + +*) > + list_append prefixes_added > "${arg:1}" > + ;; > + -*) > + list_append prefixes_removed > "${arg:1}" > + ;; > + [A-Z0-9]*) > + list_append prefixes_set "${arg}" > + ;; A prefix would never start with anything after G-Z. > + *) > + error "Invalid argument: ${arg}" > + return ${EXIT_ERROR} > + ;; > + esac > + shift > + done > + > + # Check if the user is trying a mixed operation > + if ! list_is_empty prefixes_set && (! list_is_empty > prefixes_added || ! list_is_empty prefixes_removed); then > + error "You cannot reset the prefix list and add or > remove prefixes at the same time" > + return ${EXIT_ERROR} > + fi > + > + # Set new prefixe list Typo > + if ! list_is_empty prefixes_set; then > + # Check if all prefixes are valid > + local prefix > + for prefix in ${prefixes_set}; do > + if ! ip_net_is_valid ${prefix}; then > + error "Unsupported prefix: > ${prefix}" > + return ${EXIT_ERROR} > + fi > + done > + > + assign "${_prefix}" "${prefixes_set}" > + > + # Perform incremental updates > + else > + local prefix > + > + # Perform all removals > + for prefix in ${prefixes_removed}; do > + if ! list_remove "${_prefix}" ${prefix}; > then > + warning "${prefix} was not on the > list and could not be removed" > + fi > + done > + > + for prefix in ${prefixes_added}; do > + if ip_net_is_valid ${prefix}; then > + if ! list_append_unique "${_prefix}" > ${prefix}; then > + warning "${prefix} is > already on the prefix list" > + fi > + else > + warning "${prefix} is not a valiv IP > net and could not be added" > + fi > + done > + fi > + > + # Check if the list contain at least one valid prefixe Typo > + if list_is_empty ${_prefix}; then > + error "Cannot save an empty prefix list" > + return ${EXIT_ERROR} > + fi > + > + # Save everything > + if ! vpn_ipsec_connection_write_config_key ${connection} > "${_prefix}" ${!_prefix}; then > + log ERROR "Could not write configuration settings" > + fi > +} > + > +# Handle the cli after remote > +vpn_ipsec_connection_remote() { > + if [ ! $# -ge 2 ]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + > + local connection=${1} > + local cmd=${2} > + shift 2 > + > + case ${cmd} in > + prefix) > + vpn_ipsec_connection_prefix ${connection} > "REMOTE" $@ > + ;; > + id) > + vpn_ipsec_connection_id ${connection} > "REMOTE" $@ > + ;; Same as above. > + *) > + log ERROR "Unrecognized argument: ${cmd}" > + return ${EXIT_ERROR} > + ;; > + esac > + > + return ${EXIT_OK} > +} > + > +# Set the inactivity timeout > +vpn_ipsec_connection_inactivity_timeout() { > + if [ ! $# -ge 2 ]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + > + local connection=${1} > + shift 1 > + local value=$@ > + > + if ! isinteger value; then > + value=$(parse_time $@) > + if [ ! $? -eq 0 ]; then > + log ERROR "Parsing the passed time was not > sucessful please check the passed values." > + return ${EXIT_ERROR} > + fi > + fi > + > + if [ ${value} -le 0 ]; then > + log ERROR "The passed time value must be in the sum > greater zero seconds." > + return ${EXIT_ERROR} > + fi > + > + if ! vpn_ipsec_connection_write_config_key ${connection} > "INACTIVITY_TIMEOUT" ${value}; then > + log ERROR "Could not write configuration settings" > + return ${EXIT_ERROR} > + fi > + > +} > + > +# Set the security policy to use > +vpn_ipsec_connection_security_policy() { > + if [ ! $# -eq 2 ]; then > + log ERROR "Not enough arguments" > + return ${EXIT_ERROR} > + fi > + local connection=${1} > + local security_policy=${2} > + > + if ! vpn_security_policy_exists ${security_policy}; then > + log ERROR "No such vpn security policy > '${security_policy}'" > + return ${EXIT_ERROR} > + fi > + > + if ! vpn_ipsec_connection_write_config_key ${connection} > "SECURITY_POLICY" ${security_policy}; then > + log ERROR "Could not write configuration settings" > + return ${EXIT_ERROR} > + fi > +} Actually we now also need to check for a security policy if it is in use when ever someone tries to delete it. If someone deleted the file we need to fall back to "system". > + > +# Check if a id is valid > +vpn_ipsec_connection_check_id() { > + assert [ $# -eq 1 ] > + local id=${1} > + > + if [[ ${id} =~ ^@[[:alnum:]]+$ ]] || ip_is_valid ${id}; then > + return ${EXIT_TRUE} > + else > + return ${EXIT_FALSE} > + fi > +} > + > +# Checks if a peer is valid > +vpn_ipsec_connection_check_peer() { > + assert [ $# -eq 1 ] > + local peer=${1} > + > + # TODO Accept also FQDNs > + if ip_is_valid ${peer}; then > + return ${EXIT_TRUE} > + else > + return ${EXIT_FALSE} > + fi > +} > + > +# This function checks if a VPN IPsec connection name is valid > +# Allowed are only A-Za-z0-9 > +vpn_ipsec_connection_check_name() { > + assert [ $# -eq 1 ] > + > + local connection=${1} > + > + [[ ${connection} =~ [^[:alnum:]$] ]] > +} > + > +# Function that creates one VPN IPsec connection > +vpn_ipsec_connection_new() { > + if [ $# -gt 1 ]; then > + error "Too many arguments" > + return ${EXIT_ERROR} > + fi > + > + local connection="${1}" > + if ! isset connection; then > + error "Please provide a connection name" > + return ${EXIT_ERROR} > + fi > + > + # Check for duplicates > + if vpn_ipsec_connection_exists "${connection}"; then > + error "The VPN IPsec connection ${connection} > already exists" > + return ${EXIT_ERROR} > + fi > + > + # Check if the name of the connection is valid > + if vpn_ipsec_connection_check_name "${connection}"; then > + error "'${connection}' contains illegal characters" > + return ${EXIT_ERROR} > + fi > + > + log DEBUG "Creating VPN IPsec connection ${connection}" I think the defaults should be set here. > + > + touch $(vpn_ipsec_connection_path ${connection}) You don't need to touch the file before you write to it. > + vpn_ipsec_connection_write_config "${connection}" > +} > + > +# Function that deletes based on the passed parameters one ore more > vpn security policies > +vpn_ipsec_connection_destroy() { > + local connection > + for connection in $@; do > + if ! vpn_ipsec_connection_exists ${connection}; then > + log ERROR "The VPN IPsec connection > ${connection} does not exist." > + continue > + fi > + > + log DEBUG "Deleting VPN IPsec connection > ${connection}" > + settings_remove $(vpn_ipsec_connection_path > ${connection}) > + done > +} -Michael
Hi, Am Do, 27. Jul, 2017 um 5:43 schrieb Michael Tremer <michael.tremer@ipfire.org>: > Hi, > > On Wed, 2017-07-26 at 19:32 +0200, Jonatan Schlag wrote: >> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org> >> --- >> Makefile.am | 1 + >> src/functions/functions.vpn-ipsec | 550 >> ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 551 insertions(+) >> create mode 100644 src/functions/functions.vpn-ipsec >> >> diff --git a/Makefile.am b/Makefile.am >> index 761e849..c14bb17 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -160,6 +160,7 @@ dist_network_SCRIPTS = \ >> src/functions/functions.usb \ >> src/functions/functions.util \ >> src/functions/functions.vlan \ >> + src/functions/functions.vpn-ipsec \ >> src/functions/functions.vpn-security-policies \ >> src/functions/functions.wireless \ >> src/functions/functions.wpa_supplicant \ >> diff --git a/src/functions/functions.vpn-ipsec >> b/src/functions/functions.vpn-ipsec >> new file mode 100644 >> index 0000000..2bfa3a2 >> --- /dev/null >> +++ b/src/functions/functions.vpn-ipsec >> @@ -0,0 +1,550 @@ >> +#!/bin/bash >> >> +#################################################################### >> ########### >> +# >> # >> +# IPFire.org - A linux based >> firewall # >> +# Copyright (C) 2017 IPFire Network Development >> Team # >> +# >> # >> +# This program is free software: you can redistribute it and/or >> modify # >> +# it under the terms of the GNU General Public License as published >> by # >> +# the Free Software Foundation, either version 3 of the License, >> or # >> +# (at your option) any later >> version. # >> +# >> # >> +# This program is distributed in the hope that it will be >> useful, # >> +# but WITHOUT ANY WARRANTY; without even the implied warranty >> of # >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See >> the # >> +# GNU General Public License for more >> details. # >> +# >> # >> +# You should have received a copy of the GNU General Public >> License # >> +# along with this program. If not, see >> <http://www.gnu.org/licenses >> />. # >> +# >> # >> >> +#################################################################### >> ########### >> + >> +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="AUTH_MODE INACTIVITY_TIMEOUT >> LOCAL_ID LOCAL_PREFIX" >> >> +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="${VPN_IPSEC_CONNECTION_CONFIG_ >> SETTINGS} MODE PEER PSK" >> >> +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="${VPN_IPSEC_CONNECTION_CONFIG_ >> SETTINGS} REMOTE_ID REMOTE_PREFIX" >> >> +VPN_IPSEC_CONNECTION_CONFIG_SETTINGS="${VPN_IPSEC_CONNECTION_CONFIG_ >> SETTINGS} SECURITY_POLICY" > > LOL. This is a big paragraph to open... > >> +# Default values >> +MODE="tunnel" >> +AUTH_MODE="psk" >> +INACTIVITY_TIMEOUT="0" >> +SECURITY_POLICY="system" > > I think there is no need for these to be set here. Especially since > variable names like MODE are quite generic. > > I think they should be copied into local variables in the *_new() > function. You can have something like IPSEC_DEFAULT_MODE=... in the > header maybe to make them easier to modify if we need to. But I am not > sure if that is necessary. > > Should the functions be called vpn_ipsec_* or is just ipsec_* not > enough? > >> +# This function writes all values to a via ${connection} >> specificated VPN IPsec configuration file >> +vpn_ipsec_connection_write_config() { >> + assert [ $# -ge 1 ] >> + >> + local connection="${1}" >> + >> + if ! vpn_ipsec_connection_exists "${connection}"; then >> + log ERROR "No such VPN IPsec connection: >> ${connection}" >> + return ${EXIT_ERROR} >> + fi >> + >> + local path="$(vpn_ipsec_connection_path "${connection}")" >> + if [ ! -w ${path} ]; then >> + log ERROR "${path} is not writeable" >> + return ${EXIT_ERROR} >> + fi >> + >> + if ! settings_write "${path}" >> ${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS}; then >> + log ERROR "Could not write configuration settings >> for VPN IPsec connection ${connection}" >> + return ${EXIT_ERROR} >> + fi >> + >> + # TODO everytime we successfully write a config we should >> call some trigger to take the changes into effect >> +} > > I think you can add the trigger already as an empty function called > vpn_ipsec_reload that takes the connection name as an argument. > >> + >> +# This funtion writes the value for one key to a via ${connection} >> specificated VPN IPsec connection configuration file >> +vpn_ipsec_connection_write_config_key() { >> + assert [ $# -ge 3 ] >> + >> + local connection=${1} >> + local key=${2} >> + shift 2 >> + >> + local value="$@" >> + >> + if ! vpn_ipsec_connection_exists "${connection}"; then >> + log ERROR "No such VPN ipsec connection: >> ${connection}" >> + return ${EXIT_ERROR} >> + fi >> + >> + log DEBUG "Set '${key}' to new value '${value}' in VPN ipsec >> connection '${connection}'" >> + >> + local ${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS} >> + >> + # Read the config settings >> + if ! vpn_ipsec_connection_read_config "${connection}"; then >> + return ${EXIT_ERROR} >> + fi >> + >> + # Set the key to a new value >> + assign "${key}" "${value}" >> + >> + if ! vpn_ipsec_connection_write_config "${connection}"; then >> + return ${EXIT_ERROR} >> + fi >> + >> + return ${EXIT_TRUE} >> +} >> + >> +# Reads one or more keys out of a settings file or all if no key is >> provided. >> +vpn_ipsec_connection_read_config() { >> + assert [ $# -ge 1 ] >> + >> + local connection="${1}" >> + shift 1 >> + >> + if ! vpn_ipsec_connection_exists "${connection}"; then >> + log ERROR "No such VPN IPsec connection : >> ${connection}" >> + return ${EXIT_ERROR} >> + fi >> + >> + >> + local args >> + if [ $# -eq 0 ] && [ -n >> "${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS}" ]; then >> + list_append args >> ${VPN_IPSEC_CONNECTION_CONFIG_SETTINGS} >> + else >> + list_append args $@ >> + fi >> + >> + local path="$(vpn_ipsec_connection_path ${connection})" >> + >> + if ! settings_read "${path}" ${args}; then >> + log ERROR "Could not read settings for VPN IPsec >> connection ${connection}" >> + return ${EXIT_ERROR} >> + fi >> +} >> + >> +# Returns the path to a the configuration for given connection >> +vpn_ipsec_connection_path() { >> + assert [ $# -eq 1 ] >> + >> + local connection=${1} >> + >> + echo >> "${NETWORK_CONFIG_DIR}/vpn/ipsec/connection/${connection}" >> +} > > Should we not use a directory to store certificates, a description and > so on? > >> + >> +# This function checks if a vpn ipsec connection exists >> +# Returns True when yes and false when not >> +vpn_ipsec_connection_exists() { >> + assert [ $# -eq 1 ] >> + >> + local connection=${1} >> + >> + local path=$(vpn_ipsec_connection_path "${connection}") >> + >> + [ -f ${path} ] && return ${EXIT_TRUE} || return >> ${EXIT_FALSE} >> +} > > Always use "" when you are passing an argument to [. > >> +# Handle the cli after authentification >> +vpn_ipsec_connection_authentication() { >> + if [ ! $# -gt 1 ]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + >> + local connection=${1} >> + local cmd=${2} >> + shift 2 >> + >> + case ${cmd} in >> + mode) >> + vpn_ipsec_connection_authentication_mode >> ${connection} $@ >> + ;; >> + pre-shared-key) >> + vpn_ipsec_connection_authentication_psk >> ${connection} $@ >> + ;; >> + *) >> + log ERROR "Unrecognized argument: ${cmd}" >> + return ${EXIT_ERROR} >> + ;; >> + esac >> +} >> + >> +# Set the authentification mode >> +vpn_ipsec_connection_authentication_mode() { >> + if [ ! $# -eq 2]; then > > Space missing after the 2. > >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + local connection=${1} >> + local mode=${2} >> + >> + if ! isoneof mode "psk" "PSK"; then >> + log ERROR "Auth mode '${mode}' is not valid" >> + return ${EXIT_ERROR} >> + fi >> + >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "AUTH_MODE" ${mode,,}; then >> + log ERROR "Could not write configuration settings" >> + return ${EXIT_ERROR} >> + fi >> +} >> + >> +# Set the psk >> +vpn_ipsec_connection_authentication_psk() { >> + if [ ! $# -eq 2]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + local connection=${1} >> + local psk=${2} >> + >> + # TODO Check if psk is valid >> + >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "PSK" ${psk}; then >> + log ERROR "Could not write configuration settings" >> + return ${EXIT_ERROR} >> + fi >> + >> + >> +} > > I think there is a return missing at the end. > > What is a valid PSK? > >> +# Handle the cli after local >> +vpn_ipsec_connection_local() { >> + if [ ! $# -ge 2 ]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + >> + local connection=${1} >> + local cmd=${2} >> + shift 2 >> + >> + case ${cmd} in >> + prefix) >> + vpn_ipsec_connection_prefix ${connection} >> "LOCAL" $@ >> + ;; >> + id) >> + vpn_ipsec_connection_id ${connection} >> "LOCAL" $@ >> + ;; > > Ideally I would like these in alphabetical order. Makes solving merge > conflicts easier. > >> + *) >> + log ERROR "Unrecognized argument: ${cmd}" >> + return ${EXIT_ERROR} >> + ;; >> + esac >> + >> + return ${EXIT_OK} >> + >> +} >> + >> +# Set the connection mode >> +vpn_ipsec_connection_mode() { >> + if [ ! $# -eq 2]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + local connection=${1} >> + local mode=${2} >> + >> + if ! isoneof mode "tunnel" "vti" "gre-transport"; then >> + log ERROR "Mode '${mode}' is not valid" >> + return ${EXIT_ERROR} >> + fi > > We should have a variable for all valid modes at the top of the file. > >> + >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "MODE" ${mode}; then >> + log ERROR "Could not write configuration settings" >> + return ${EXIT_ERROR} >> + fi >> + >> +} > > Same here. Either no empty line or a return... > >> + >> +# Set the peer to connect to >> +vpn_ipsec_connection_peer() { >> + if [ ! $# -eq 2]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + local connection=${1} >> + local peer=${2} >> + >> + if ! vpn_ipsec_connection_check_peer ${peer}; then >> + log ERROR "Peer '${peer}' is not valid" >> + return ${EXIT_ERROR} >> + fi > > There is a word for "not valid". It is "invalid". > >> + >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "PEER" ${peer}; then > > At the top you used "" around ${connection}. I think that would be a > good idea generally. > >> + log ERROR "Could not write configuration settings" >> + return ${EXIT_ERROR} >> + fi >> +} >> + >> +#Set the local or remote id >> +vpn_ipsec_connection_id() { >> + if [ ! $# -eq 3]; then > > Space. > >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + local connection=${1} >> + local type=${2} >> + local id=${3} >> + >> + if ! vpn_ipsec_connection_check_id ${id}; then >> + log ERROR "Id '${id}' is not valid" >> + return ${EXIT_ERROR} >> + fi >> + >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "${type}_ID" ${id}; then >> + log ERROR "Could not write configuration settings" >> + return ${EXIT_ERROR} >> + fi >> + >> + return ${EXIT_OK} >> +} >> + >> +# Set the local or remote prefix >> +vpn_ipsec_connection_prefix() { >> + if [ ! $# -ge 3 ]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + local connection=${1} >> + local type=${2} >> + shift 2 >> + >> + local _prefix="${type}_PREFIX" >> + local "${_prefix}" >> + if ! vpn_ipsec_connection_read_config ${connection} >> "${_prefix}"; then >> + return ${EXIT_ERROR} >> + fi >> + >> + # Remove duplicated entries to proceed the list safely >> + assign "${_prefix}" "$(list_unique ${!_prefix} )" >> + >> + local prefixes_added >> + local prefixes_removed >> + local prefixes_set >> + >> + while [ $# -gt 0 ]; do >> + local arg="${1}" >> + >> + case "${arg}" in >> + +*) >> + list_append prefixes_added >> "${arg:1}" >> + ;; >> + -*) >> + list_append prefixes_removed >> "${arg:1}" >> + ;; >> + [A-Z0-9]*) >> + list_append prefixes_set "${arg}" >> + ;; > > A prefix would never start with anything after G-Z. > >> + *) >> + error "Invalid argument: ${arg}" >> + return ${EXIT_ERROR} >> + ;; >> + esac >> + shift >> + done >> + >> + # Check if the user is trying a mixed operation >> + if ! list_is_empty prefixes_set && (! list_is_empty >> prefixes_added || ! list_is_empty prefixes_removed); then >> + error "You cannot reset the prefix list and add or >> remove prefixes at the same time" >> + return ${EXIT_ERROR} >> + fi >> + >> + # Set new prefixe list > > Typo > >> + if ! list_is_empty prefixes_set; then >> + # Check if all prefixes are valid >> + local prefix >> + for prefix in ${prefixes_set}; do >> + if ! ip_net_is_valid ${prefix}; then >> + error "Unsupported prefix: >> ${prefix}" >> + return ${EXIT_ERROR} >> + fi >> + done >> + >> + assign "${_prefix}" "${prefixes_set}" >> + >> + # Perform incremental updates >> + else >> + local prefix >> + >> + # Perform all removals >> + for prefix in ${prefixes_removed}; do >> + if ! list_remove "${_prefix}" ${prefix}; >> then >> + warning "${prefix} was not on the >> list and could not be removed" >> + fi >> + done >> + >> + for prefix in ${prefixes_added}; do >> + if ip_net_is_valid ${prefix}; then >> + if ! list_append_unique "${_prefix}" >> ${prefix}; then >> + warning "${prefix} is >> already on the prefix list" >> + fi >> + else >> + warning "${prefix} is not a valiv IP >> net and could not be added" >> + fi >> + done >> + fi >> + >> + # Check if the list contain at least one valid prefixe > > Typo > >> + if list_is_empty ${_prefix}; then >> + error "Cannot save an empty prefix list" >> + return ${EXIT_ERROR} >> + fi >> + >> + # Save everything >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "${_prefix}" ${!_prefix}; then >> + log ERROR "Could not write configuration settings" >> + fi >> +} >> + >> +# Handle the cli after remote >> +vpn_ipsec_connection_remote() { >> + if [ ! $# -ge 2 ]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + >> + local connection=${1} >> + local cmd=${2} >> + shift 2 >> + >> + case ${cmd} in >> + prefix) >> + vpn_ipsec_connection_prefix ${connection} >> "REMOTE" $@ >> + ;; >> + id) >> + vpn_ipsec_connection_id ${connection} >> "REMOTE" $@ >> + ;; > > Same as above. > >> + *) >> + log ERROR "Unrecognized argument: ${cmd}" >> + return ${EXIT_ERROR} >> + ;; >> + esac >> + >> + return ${EXIT_OK} >> +} >> + >> +# Set the inactivity timeout >> +vpn_ipsec_connection_inactivity_timeout() { >> + if [ ! $# -ge 2 ]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + >> + local connection=${1} >> + shift 1 >> + local value=$@ >> + >> + if ! isinteger value; then >> + value=$(parse_time $@) >> + if [ ! $? -eq 0 ]; then >> + log ERROR "Parsing the passed time was not >> sucessful please check the passed values." >> + return ${EXIT_ERROR} >> + fi >> + fi >> + >> + if [ ${value} -le 0 ]; then >> + log ERROR "The passed time value must be in the sum >> greater zero seconds." >> + return ${EXIT_ERROR} >> + fi >> + >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "INACTIVITY_TIMEOUT" ${value}; then >> + log ERROR "Could not write configuration settings" >> + return ${EXIT_ERROR} >> + fi >> + >> +} >> + >> +# Set the security policy to use >> +vpn_ipsec_connection_security_policy() { >> + if [ ! $# -eq 2 ]; then >> + log ERROR "Not enough arguments" >> + return ${EXIT_ERROR} >> + fi >> + local connection=${1} >> + local security_policy=${2} >> + >> + if ! vpn_security_policy_exists ${security_policy}; then >> + log ERROR "No such vpn security policy >> '${security_policy}'" >> + return ${EXIT_ERROR} >> + fi >> + >> + if ! vpn_ipsec_connection_write_config_key ${connection} >> "SECURITY_POLICY" ${security_policy}; then >> + log ERROR "Could not write configuration settings" >> + return ${EXIT_ERROR} >> + fi >> +} > > Actually we now also need to check for a security policy if it is in > use when ever someone tries to delete it. > > If someone deleted the file we need to fall back to "system". > >> + >> +# Check if a id is valid >> +vpn_ipsec_connection_check_id() { >> + assert [ $# -eq 1 ] >> + local id=${1} >> + >> + if [[ ${id} =~ ^@[[:alnum:]]+$ ]] || ip_is_valid ${id}; then >> + return ${EXIT_TRUE} >> + else >> + return ${EXIT_FALSE} >> + fi >> +} >> + >> +# Checks if a peer is valid >> +vpn_ipsec_connection_check_peer() { >> + assert [ $# -eq 1 ] >> + local peer=${1} >> + >> + # TODO Accept also FQDNs >> + if ip_is_valid ${peer}; then >> + return ${EXIT_TRUE} >> + else >> + return ${EXIT_FALSE} >> + fi >> +} >> + >> +# This function checks if a VPN IPsec connection name is valid >> +# Allowed are only A-Za-z0-9 >> +vpn_ipsec_connection_check_name() { >> + assert [ $# -eq 1 ] >> + >> + local connection=${1} >> + >> + [[ ${connection} =~ [^[:alnum:]$] ]] >> +} >> + >> +# Function that creates one VPN IPsec connection >> +vpn_ipsec_connection_new() { >> + if [ $# -gt 1 ]; then >> + error "Too many arguments" >> + return ${EXIT_ERROR} >> + fi >> + >> + local connection="${1}" >> + if ! isset connection; then >> + error "Please provide a connection name" >> + return ${EXIT_ERROR} >> + fi >> + >> + # Check for duplicates >> + if vpn_ipsec_connection_exists "${connection}"; then >> + error "The VPN IPsec connection ${connection} >> already exists" >> + return ${EXIT_ERROR} >> + fi >> + >> + # Check if the name of the connection is valid >> + if vpn_ipsec_connection_check_name "${connection}"; then >> + error "'${connection}' contains illegal characters" >> + return ${EXIT_ERROR} >> + fi >> + >> + log DEBUG "Creating VPN IPsec connection ${connection}" > > I think the defaults should be set here. > > >> + >> + touch $(vpn_ipsec_connection_path ${connection}) > > You don't need to touch the file before you write to it. Unfourtunately I have to touch the file bevore I write to it. This check fails without touch + if [ ! -w ${path} ]; then + log ERROR "${path} is not writeable" + return ${EXIT_ERROR} + fi The rest is ok i will send a improved version. Jonatan > > >> + vpn_ipsec_connection_write_config "${connection}" >> +} >> + >> +# Function that deletes based on the passed parameters one ore more >> vpn security policies >> +vpn_ipsec_connection_destroy() { >> + local connection >> + for connection in $@; do >> + if ! vpn_ipsec_connection_exists ${connection}; then >> + log ERROR "The VPN IPsec connection >> ${connection} does not exist." >> + continue >> + fi >> + >> + log DEBUG "Deleting VPN IPsec connection >> ${connection}" >> + settings_remove $(vpn_ipsec_connection_path >> ${connection}) >> + done >> +} > > -Michael