Hi,
sorry for the very late review. See the comments below:
On Mon, 2017-08-28 at 17:32 +0200, Jonatan Schlag wrote:
The valid options for a host-to-net ipsec connection and a net-to-net are different so we nedd to check if we get a valid option for the connection type.
Fixes: #11444
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org
src/functions/functions.ipsec | 129 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+)
diff --git a/src/functions/functions.ipsec b/src/functions/functions.ipsec index 6f14c8e..11a1d9a 100644 --- a/src/functions/functions.ipsec +++ b/src/functions/functions.ipsec @@ -54,6 +54,46 @@ IPSEC_DEFAULT_TYPE="net-to-net" IPSEC_VALID_MODES="gre-transport tunnel vti" IPSEC_VALID_AUTH_MODES="PSK"
+# Valid options for all types of connetions +IPSEC_ALL_VALID_OPTIONS="\
- authentication \
- down \
- disable \
- dpd \
- enable \
- inactivity_timeout \
- local \
- mode \
- remote \
- security_policy \
- start_action \
- up \
- color \
- description \
- show"
Could we order those alphabetically?
+# Valid options for net-to-net +IPSEC_NET_TOT_NET_VALID_OPTIONS="${IPSEC_ALL_VALID_OPTIONS} \
- peer"
I think it should be "TO" in the variable name. You could also use "N2N" to make it a little bit shorter maybe.
+# Valid options for RW +IPSEC_RW_VALID_OPTIONS="${IPSEC_ALL_VALID_OPTIONS} \
- pool"
+# Valid dpd options for all types of connetions +IPSEC_DPD_ALL_VALID_OPTIONS="\
- delay \
- timeout"
+# Valid dpd options for net-to-net +IPSEC_DPD_NET_TOT_NET_VALID_OPTIONS="${IPSEC_DPD_ALL_VALID_OPTIONS} \
- action"
Same as above.
+# Valid dpd options for RW +IPSEC_DPD_RW_VALID_OPTIONS="${IPSEC_DPD_ALL_VALID_OPTIONS}"
Three lines of whitespace. One should do.
cli_ipsec() { local action=${1} shift 1 @@ -79,6 +119,10 @@ cli_ipsec_connection() { key=${key//-/_} shift 2
if ! ipsec_connection_check_option "${connection}" "first"
"${key}"; then
return ${EXITR_ERROR}
fi
Typo in the EXIT_ERROR variable.
case "${key}" in authentication|down|disable|dpd|enable|inactivity_tim
eout|local|mode|peer|pool|remote|security_policy|start_action|up) ipsec_connection_${key} ${connection} "$@" @@ -148,6 +192,67 @@ ipsec_connection_get_description_title() { description_title_read $(description_format_filename "ipsec- connection" "${name}") }
+# This functions returns all options which are valid for this type of connection +ipsec_get_valid_options() {
- assert [ $# -eq 2 ]
- local connection="${1}"
- local type="${2}"
- local con_type=$(ipsec_connection_get_type "${connection}")
- assert isset con_type
- case ${con_type} in
host-to-net)
case ${type} in
first)
echo "${IPSEC_RW_VALID_OPTIONS}"
;;
dpd)
echo "${IPSEC_DPD_RW_VALID_OPTIONS}"
;;
esac
;;
net-to-net)
case ${type} in
first)
echo
"${IPSEC_NET_TOT_NET_VALID_OPTIONS}"
;;
dpd)
echo
"${IPSEC_DPD_NET_TOT_NET_VALID_OPTIONS}"
;;
esac
;;
- esac
+}
+# This Function checks if a option is valid for a given type +ipsec_connection_check_option() {
- assert [ $# -eq 3 ]
- local connection="${1}"
- local type="${2}"
- local key="${3}"
- local con_type=$(ipsec_connection_get_type "${connection}")
- assert isset con_type
- case ${con_type} in
host-to-net)
if ! isoneof key $(ipsec_get_valid_options
"${connection}" "${type}"); then
log ERROR "Invalid Argument: ${key}"
return ${EXIT_ERROR}
fi
;;
net-to-net)
if ! isoneof key $(ipsec_get_valid_options
"${connection}" "${type}"); then
log ERROR "Invalid Argument: ${key}"
return ${EXIT_ERROR}
fi
;;
- esac
+}
cli_ipsec_connection_show() { local connection="${1}"
@@ -443,6 +548,26 @@ ipsec_reload() { ipsec_strongswan_load }
+# Returns the type of a connection +ipsec_connection_get_type() {
- assert [ $# -eq 1 ]
- local connection=${1}
- if ! ipsec_connection_exists "${connection}"; then
log ERROR "No such VPN ipsec connection: ${connection}"
return ${EXIT_ERROR}
- fi
It is called a "IPsec VPN connection".
IPsec is spelled like this and the rest sounds like Yoda :)
- local TYPE
- if ! ipsec_connection_read_config "${connection}" TYPE; then
return ${EXIT_ERROR}
- fi
- echo ${TYPE}
+}
# Handle the cli after authentification ipsec_connection_authentication() { if [ ! $# -gt 1 ]; then @@ -551,6 +676,10 @@ ipsec_connection_dpd() { local cmd=${2} shift 2
- if ! ipsec_connection_check_option "${connection}" "dpd" "${cmd}";
then
return ${EXITR_ERROR}
- fi
Should the user not get a better message when DPD commands are not available?
case ${cmd} in action) ipsec_connection_dpd_action "${connection}" "$@"
-Michael