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" + +# Valid options for net-to-net +IPSEC_NET_TOT_NET_VALID_OPTIONS="${IPSEC_ALL_VALID_OPTIONS} \ + peer" + +# 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" + +# Valid dpd options for RW +IPSEC_DPD_RW_VALID_OPTIONS="${IPSEC_DPD_ALL_VALID_OPTIONS}" + + + 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 + case "${key}" in authentication|down|disable|dpd|enable|inactivity_timeout|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 + + 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 + case ${cmd} in action) ipsec_connection_dpd_action "${connection}" "$@"
The options for the different types of connections should be also printed correctly when we use autocompletion
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org --- src/bash-completion/network | 17 ++++++++++++++++- src/network | 3 +++ 2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/bash-completion/network b/src/bash-completion/network index 71bf245..25cf6aa 100644 --- a/src/bash-completion/network +++ b/src/bash-completion/network @@ -413,7 +413,7 @@ _network_vpn_ipsec_connection_subcommands() { shift local words=( $@ )
- local commands="authentication color description down inactivity-timeout local mode peer remote security-policy show up" + local commands="$(network raw list-valid-ipsec-options "${connection}" "first")" local cmd="$(_network_find_on_cmdline "${commands}")" if [[ -z "${cmd}" ]]; then COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") ) @@ -432,6 +432,9 @@ _network_vpn_ipsec_connection_subcommands() { description) _network_description ${args} ;; + dpd) + _network_vpn_ipsec_connection_subcommands_dpd "${connection}" ${args} + ;; local) _network_vpn_ipsec_connection_subcommands_local_remote ${connection} "local" ${args} ;; @@ -529,6 +532,18 @@ _network_vpn_ipsec_connection_subcommands_security_policy() { fi }
+_network_vpn_ipsec_connection_subcommands_dpd() { + local connection=${1} + shift + local words=( $@ ) + + local commands="$(network raw list-valid-ipsec-options "${connection}" "dpd")" + local cmd="$(_network_find_on_cmdline "${commands}")" + if [[ -z "${cmd}" ]]; then + COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") ) + return 0 + fi +} _network_vpn_security_policies() { local words=( $@ )
diff --git a/src/network b/src/network index 9a2d480..379a71d 100644 --- a/src/network +++ b/src/network @@ -1334,6 +1334,9 @@ cli_raw() { list-next-free-zones) zones_get_next_free ;; + list-valid-ipsec-options) + ipsec_get_valid_options "$@" + ;; list-zone-config-ids) zone_config_list_ids "$@" ;;
Hi,
technically this work, but is it not better to have a command that checks what type a connection is (rw or n2n) and then the right list of commands will be selected?
I think that is cleaner instead of heaving some auto-completion stuff in the main library.
Best, -Michael
On Mon, 2017-08-28 at 17:32 +0200, Jonatan Schlag wrote:
The options for the different types of connections should be also printed correctly when we use autocompletion
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org
src/bash-completion/network | 17 ++++++++++++++++- src/network | 3 +++ 2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/bash-completion/network b/src/bash-completion/network index 71bf245..25cf6aa 100644 --- a/src/bash-completion/network +++ b/src/bash-completion/network @@ -413,7 +413,7 @@ _network_vpn_ipsec_connection_subcommands() { shift local words=( $@ )
- local commands="authentication color description down inactivity-
timeout local mode peer remote security-policy show up"
- local commands="$(network raw list-valid-ipsec-options
"${connection}" "first")" local cmd="$(_network_find_on_cmdline "${commands}")" if [[ -z "${cmd}" ]]; then COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") ) @@ -432,6 +432,9 @@ _network_vpn_ipsec_connection_subcommands() { description) _network_description ${args} ;;
dpd)_network_vpn_ipsec_connection_subcommands_dpd"${connection}" ${args}
;;e ${connection} "local" ${args} ;; @@ -529,6 +532,18 @@ _network_vpn_ipsec_connection_subcommands_security_policy() { fi }
+_network_vpn_ipsec_connection_subcommands_dpd() {
- local connection=${1}
- shift
- local words=( $@ )
- local commands="$(network raw list-valid-ipsec-options
"${connection}" "dpd")"
- local cmd="$(_network_find_on_cmdline "${commands}")"
- if [[ -z "${cmd}" ]]; then
COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )return 0- fi
+} _network_vpn_security_policies() { local words=( $@ )
diff --git a/src/network b/src/network index 9a2d480..379a71d 100644 --- a/src/network +++ b/src/network @@ -1334,6 +1334,9 @@ cli_raw() { list-next-free-zones) zones_get_next_free ;;
list-valid-ipsec-options)ipsec_get_valid_options "$@";;
Hi
Am Do, 7. Sep, 2017 um 9:45 schrieb Michael Tremer michael.tremer@ipfire.org:
Hi,
technically this work, but is it not better to have a command that checks what type a connection is (rw or n2n) and then the right list of commands will be selected?
I think that is cleaner instead of heaving some auto-completion stuff in the main library.
IIt may be cleaner but so we only one place where have all possible options listed. This makes adding or changing options much smarter because we have only one place where all options are listed. So I think we should keep this solution and have an n extra variable in the auto-completion wish we have to update on every change.
Jonatan
Best, -Michael
On Mon, 2017-08-28 at 17:32 +0200, Jonatan Schlag wrote:
The options for the different types of connections should be also printed correctly when we use autocompletion
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org
src/bash-completion/network | 17 ++++++++++++++++- src/network | 3 +++ 2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/bash-completion/network b/src/bash-completion/network index 71bf245..25cf6aa 100644 --- a/src/bash-completion/network +++ b/src/bash-completion/network @@ -413,7 +413,7 @@ _network_vpn_ipsec_connection_subcommands() { shift local words=( $@ )
- local commands="authentication color description down inactivity-
timeout local mode peer remote security-policy show up"
- local commands="$(network raw list-valid-ipsec-options
"${connection}" "first")" local cmd="$(_network_find_on_cmdline "${commands}")" if [[ -z "${cmd}" ]]; then COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") ) @@ -432,6 +432,9 @@ _network_vpn_ipsec_connection_subcommands() { description) _network_description ${args} ;;
dpd)_network_vpn_ipsec_connection_subcommands_dpd"${connection}" ${args}
;;e ${connection} "local" ${args} ;; @@ -529,6 +532,18 @@ _network_vpn_ipsec_connection_subcommands_security_policy() { fi }
+_network_vpn_ipsec_connection_subcommands_dpd() {
- local connection=${1}
- shift
- local words=( $@ )
- local commands="$(network raw list-valid-ipsec-options
"${connection}" "dpd")"
- local cmd="$(_network_find_on_cmdline "${commands}")"
- if [[ -z "${cmd}" ]]; then
COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") )return 0- fi
+} _network_vpn_security_policies() { local words=( $@ )
diff --git a/src/network b/src/network index 9a2d480..379a71d 100644 --- a/src/network +++ b/src/network @@ -1334,6 +1334,9 @@ cli_raw() { list-next-free-zones) zones_get_next_free ;;
list-valid-ipsec-options)ipsec_get_valid_options "$@";;
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_timeout|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} infirst)echo "${IPSEC_RW_VALID_OPTIONS}";;dpd)echo "${IPSEC_DPD_RW_VALID_OPTIONS}";;esac;;net-to-net)case ${type} infirst)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
-
Jonatan Schlag -
Michael Tremer