From 43e9154452d2d965414cd102932f1959b7c75863 Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 20:35:17 +0000 Subject: [PATCH 1/8] Fix documentation around down-pre as it's a modifier only The down-pre option is a modifier only (and always has been it seems, so we've been using it wrong). The script needs to be called with the down option and then told to run earlier with down-pre. --- README.md | 8 +++++--- update-systemd-resolved | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index ce4815c..88b9fba 100644 --- a/README.md +++ b/README.md @@ -42,14 +42,16 @@ otherwise the configuration provided by this script will only work on domains that cannot be resolved by the currently configured DNS servers (i.e. they must fall back after trying the ones set by your LAN's DHCP server). -Finally, update your OpenVPN configuration file and set the `up` and `down-pre` -options: +Finally, update your OpenVPN configuration file and set the `up` and `down` +options to point to the script, and `down-pre` to ensure that the script is run +before the device is closed: ``` script-security 2 setenv PATH /usr/bin up /etc/openvpn/update-systemd-resolved -down-pre /etc/openvpn/update-systemd-resolved +down /etc/openvpn/update-systemd-resolved +down-pre ``` ## Usage diff --git a/update-systemd-resolved b/update-systemd-resolved index eda856e..35b8882 100755 --- a/update-systemd-resolved +++ b/update-systemd-resolved @@ -18,10 +18,12 @@ # This script will parse DHCP options set via OpenVPN (dhcp-option) to update # systemd-resolved directly via DBus, instead of updating /etc/resolv.conf. To -# install, set as the 'up' and 'down-pre' script in your OpenVPN configuration file -# or command-line argument. For example: +# install, set as the 'up' and 'down' script in your OpenVPN configuration file +# or via the command-line arguments, alongside setting the 'down-pre' option to +# run the 'down' script before the device is closed. For example: # up /etc/openvpn/update-systemd-resolved -# down-pre /etc/openvpn/update-systemd-resolved +# down /etc/openvpn/update-systemd-resolved +# down-pre # Define what needs to be called via DBus DBUS_DEST="org.freedesktop.resolve1" From 917f2f495767b7c14c953b5a83ce0c9f3b4b2182 Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 20:36:11 +0000 Subject: [PATCH 2/8] FIx documentation for DNSSEC supporting allow-downgrade Forgot to include in the documentation that allow-downgrade is an allowed option in SetLinkDNSSEC and this script does support it. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 88b9fba..eb967f2 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ OpenVPN, either through the server, or the client, configuration: | `DOMAIN` | `example.com` | The primary domain for this host. If set multiple times, the last provided is used. Will be the primary search domain for bare hostnames. All requests for this domain as well will be routed to the `DNS` servers provided on this link. | | `DOMAIN-SEARCH` | `example.com` | Secondary domains which will be used to search for bare hostnames (after any `DOMAIN`, if set) and in the order provided. All requests for this domain will be routed to the `DNS` servers provided on this link. | | `DOMAIN-ROUTE` | `example.com` | All requests for these domains will be routed to the `DNS` servers provided on this link. They will *not* be used to search for bare hostnames, only routed. | -| `DNSSEC` | `yes`
`no`
`default` | Control of DNSSEC should be enabled (`yes`) or disabled (`no`) for any queries over this link only, or use the system default (`default`). | +| `DNSSEC` | `yes`
`no`
`allow-downgrade`
`default` | Control of DNSSEC should be enabled (`yes`) or disabled (`no`), or `allow-downgrade` to switch off DNSSEC only if the server doesn't support it, for any queries over this link only, or use the system default (`default`). | *Note*: There are no local or system options to be configured. All configuration for this script is handled though OpenVPN, including, for example, the name of From e0b59b8a6cce19a7e775610ea70e46b9ec184237 Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 20:39:51 +0000 Subject: [PATCH 3/8] Fix handling of default case for DNSSEC option The handler busctl_call doesn't seem to be properly handling the default case for SetLinkDNSSEC (i.e. the empty string: "") when it's in the array format. Rather than pass it as such, it's now passed directly as an empty string. --- update-systemd-resolved | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/update-systemd-resolved b/update-systemd-resolved index 35b8882..82de7f7 100755 --- a/update-systemd-resolved +++ b/update-systemd-resolved @@ -127,12 +127,12 @@ up() { if [[ -n "${dns_sec}" ]]; then if [[ "${dns_sec}" == "default" ]]; then # We need to provide an empty string to use the default settings - busctl_params=("$if_index" '""') + info "SetLinkDNSSEC($if_index '')" + busctl_call SetLinkDNSSEC 'is' "$if_index" "" || return $? else - busctl_params=("$if_index" "${dns_sec}") + info "SetLinkDNSSEC($if_index ${dns_sec})" + busctl_call SetLinkDNSSEC 'is' "$if_index" "${dns_sec}" || return $? fi - info "SetLinkDNSSEC(${busctl_params[*]})" - busctl_call SetLinkDNSSEC 'is' "${busctl_params[@]}" || return $? fi } From 57d8e1b99735ead9f40f2d481def283f3b2ae49d Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 20:53:47 +0000 Subject: [PATCH 4/8] Convert backticks to normal single quotes in strings I prefer it this way :) --- update-systemd-resolved | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/update-systemd-resolved b/update-systemd-resolved index 82de7f7..7082f28 100755 --- a/update-systemd-resolved +++ b/update-systemd-resolved @@ -48,7 +48,7 @@ busctl_call() { # Preserve busctl's exit status busctl call "$DBUS_DEST" "$DBUS_NODE" "${DBUS_DEST}.Manager" "$@" || { local -i status=$? - emerg "\`busctl' exited with status $status" + emerg "'busctl' exited with status $status" return $status } } @@ -78,7 +78,7 @@ up() { local if_index="$1" shift - info "Link \`$link' coming up" + info "Link '$link' coming up" # Preset values for processing -- will be altered in the various process_* # functions. @@ -96,7 +96,7 @@ up() { if declare -f "$process_setting_function" &>/dev/null; then "$process_setting_function" "$setting_value" || return $? else - warning "Not a recognized DHCP setting: \`${setting}'" + warning "Not a recognized DHCP setting: '${setting}'" fi done < <(dhcp_settings) @@ -142,7 +142,7 @@ down() { local if_index="$1" shift - info "Link \`$link' going down" + info "Link '$link' going down" busctl_call RevertLink i "$if_index" } @@ -155,7 +155,7 @@ process_dns() { elif looks_like_ipv4 "$address"; then process_dns_ipv4 "$address" || return $? else - err "Not a valid IPv6 or IPv4 address: \`$address'" + err "Not a valid IPv6 or IPv4 address: '$address'" return 1 fi } @@ -196,17 +196,17 @@ parse_ipv6() { local raw_address="$1" log_invalid_ipv6() { - local message="\`$raw_address' is not a valid IPv6 address" + local message="'$raw_address' is not a valid IPv6 address" emerg "${message}: $*" } trap -- 'unset -f log_invalid_ipv6' RETURN if [[ "$raw_address" == *::*::* ]]; then - log_invalid_ipv6 "address cannot contain more than one \`::'" + log_invalid_ipv6 "address cannot contain more than one '::'" return 1 elif [[ "$raw_address" =~ :0+:: ]] || [[ "$raw_address" =~ ::0+: ]]; then - log_invalid_ipv6 "address contains a 0-group adjacent to \`::' and is not maximally shortened" + log_invalid_ipv6 "address contains a 0-group adjacent to '::' and is not maximally shortened" return 1 fi @@ -271,7 +271,7 @@ parse_ipv6() { if [[ "$raw_address" == *::* ]]; then if (( ${#tokenized_segments[*]} == length )); then - log_invalid_ipv6 "single \`0' fields should not be compressed" + log_invalid_ipv6 "single '0' fields should not be compressed" return 1 else local -i largest_run_i=0 largest_run=0 @@ -368,7 +368,7 @@ process_dnssec() { allow-downgrade) setting="allow-downgrade" ;; *) - local message="\`$option' is not a valid DNSSEC option" + local message="'$option' is not a valid DNSSEC option" emerg "${message}" return 1 ;; esac @@ -390,11 +390,11 @@ main() { usage 'No device name specified' return 1 elif ! declare -f "${script_type}" &>/dev/null; then - usage "Invalid script type: \`${script_type}'" + usage "Invalid script type: '${script_type}'" return 1 else if ! read -r link if_index _ < <(get_link_info "$dev"); then - usage "Invalid device name: \`$dev'" + usage "Invalid device name: '$dev'" return 1 fi From 11f4ed9bb5a8c79525c1a3888945cbde8f45d20e Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 20:59:18 +0000 Subject: [PATCH 5/8] Handle the privlege drop case when down() is called If down() is called when the user/group options have been set, the OpenVPN client (and hence the script that is forked from it) no longer have the required privileges to call the RevertLink function over DBus. Rather than have the system throw a big error, make a note in the logs and exit gracefully. --- update-systemd-resolved | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/update-systemd-resolved b/update-systemd-resolved index 7082f28..422d105 100755 --- a/update-systemd-resolved +++ b/update-systemd-resolved @@ -143,7 +143,12 @@ down() { shift info "Link '$link' going down" - busctl_call RevertLink i "$if_index" + if [[ "$(whoami 2>/dev/null)" != "root" ]]; then + # Cleanly handle the priviledge dropped case by not calling RevertLink + info "Priviledges dropped in the client: Cannot call RevertLink." + else + busctl_call RevertLink i "$if_index" + fi } process_dns() { From cca16eb26abc43ac8257d3b8c931a5f2745bb450 Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 21:01:07 +0000 Subject: [PATCH 6/8] Update DNSSEC test cases to handle the altered SetLinkDNSSEC call We no longer explictly pass the quotation marks so they will need to be removed from the test. --- tests/20_dnssec_only.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/20_dnssec_only.sh b/tests/20_dnssec_only.sh index d2a56b9..80b8ee5 100644 --- a/tests/20_dnssec_only.sh +++ b/tests/20_dnssec_only.sh @@ -4,8 +4,8 @@ dev="tun20" TEST_BUSCTL_CALLED=1 declare -A test_options=( - ['default']='""' - ['Default']='""' + ['default']='' + ['Default']='' ['true']='yes' ['True']='yes' ['yes']='yes' From ed2f3cc321e303f677d667ed6a4822f9dc7c9bf0 Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 21:23:50 +0000 Subject: [PATCH 7/8] Fix handling of empty string test case for DNSSEC Earlier versions of Bash are not behaving the same around the empty string case. Resolve the test as the empty string is now a valid option. --- run-tests | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/run-tests b/run-tests index b7a19bc..3b913c1 100755 --- a/run-tests +++ b/run-tests @@ -66,16 +66,11 @@ function busctl { ;; SetLinkDNSSEC) shift 2 - if [[ "${TEST_BUSCTL_DNSSEC}" == "" ]]; then - [[ "${ip_ifindex} ${TEST_BUSCTL_DNSSEC}" == "${@}" ]] || \ - _fail "SetLinkDNSSEC was called and should not be: '${@}'" - else - [[ "${ip_ifindex} ${TEST_BUSCTL_DNSSEC}" == "${@}" ]] && \ - _pass "SetLinkDNSSEC was called correctly" || \ - _fail "SetLinkDNSSEC was not given the correct arguments:\n" \ - " Expected: '${ip_ifindex} ${TEST_BUSCTL_DNSSEC}'\n" \ - " Received: '${@}'" - fi + [[ "${ip_ifindex} ${TEST_BUSCTL_DNSSEC}" == "${@}" ]] && \ + _pass "SetLinkDNSSEC was called correctly" || \ + _fail "SetLinkDNSSEC was not given the correct arguments:\n" \ + " Expected: '${ip_ifindex} ${TEST_BUSCTL_DNSSEC}'\n" \ + " Received: '${@}'" ;; *) _fail "Unknown command called on busctl: ${1}" From 19e17eca8f1beda6095f705db71ea3b282e4eb3e Mon Sep 17 00:00:00 2001 From: Jonathan Wright Date: Mon, 12 Dec 2016 21:29:04 +0000 Subject: [PATCH 8/8] Handle empty space in the comparitor Need to handle comparisons with earlier versions of Bash which no not handle cleanly spaces at the end of the strings in comparisons (or the @ array expansion with empty strings has changed). --- run-tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-tests b/run-tests index 3b913c1..7d42cce 100755 --- a/run-tests +++ b/run-tests @@ -66,7 +66,7 @@ function busctl { ;; SetLinkDNSSEC) shift 2 - [[ "${ip_ifindex} ${TEST_BUSCTL_DNSSEC}" == "${@}" ]] && \ + [[ "${ip_ifindex} ${TEST_BUSCTL_DNSSEC}x" == "${@}x" ]] && \ _pass "SetLinkDNSSEC was called correctly" || \ _fail "SetLinkDNSSEC was not given the correct arguments:\n" \ " Expected: '${ip_ifindex} ${TEST_BUSCTL_DNSSEC}'\n" \