[1/2] ipsec: only accept valid options for a connection type

Message ID 1503934373-8996-1-git-send-email-jonatan.schlag@ipfire.org
State New
Headers show
Series
  • [1/2] ipsec: only accept valid options for a connection type
Related show

Commit Message

Jonatan Schlag Aug. 28, 2017, 3:32 p.m.
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(+)

Comments

Michael Tremer Sept. 7, 2017, 7:44 p.m. | #1
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

Patch

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}" "$@"