Add hostname and IP address validation for env

agent/base

@@ -100,15 +100,30 @@ fi
 # don't use colons and dashes in the date suffix
 export date_suffix=$(date --date ${date} --utc +"%Y.%m.%dT%H.%M.%S")
 
+function validate_hostname {
+    validate-hostname "${1}"
+    if [[ ${?} -ne 0 ]]; then
+        error_log "Invalid ${2}: '${1}'"
+        exit 1
+    fi
+}
+
 if [[ -z "${_pbench_hostname}" ]]; then
     export _pbench_hostname=$(hostname -s)
 fi
+validate_hostname "${_pbench_hostname}" "_pbench_hostname"
 if [[ -z "${_pbench_full_hostname}" ]]; then
     export _pbench_full_hostname=$(hostname -f)
 fi
+validate_hostname "${_pbench_full_hostname}" "_pbench_full_hostname"
 if [[ -z "${_pbench_hostname_ip}" ]]; then
     export _pbench_hostname_ip=$(hostname -i)
 fi
+validate-ipaddress "${_pbench_hostname_ip}"
+if [[ ${?} -ne 0 ]]; then
+    error_log "Invalid IP address: '${_pbench_hostname_ip}'"
+    exit 1
+fi
 
 if [[ -z "$ssh_opts" ]]; then
     ssh_opts=$(pbench-config ssh_opts results)

agent/bench-scripts/unittests

@@ -43,7 +43,7 @@ cp ${_tdir}/postprocess/* ${_testopt}/bench-scripts/postprocess
 let res=res+${?}
 cp ${_tdir}/templates/* ${_testopt}/bench-scripts/templates
 let res=res+${?}
-cp -a $_tdir/../util-scripts/{require-rpm,pbench-*} $_testopt/util-scripts/
+cp -a $_tdir/../util-scripts/{require-rpm,validate-*,pbench-*} $_testopt/util-scripts/
 let res=res+$?
 cp -rH ${_tdir}/test-bin/* ${_testopt}/unittest-scripts/
 let res=res+${?}

agent/util-scripts/gold/pbench-move-results/test-22.txt

@@ -0,0 +1,11 @@
++++ Running test-22 pbench-move-results --controller=bad_host_name.example.com
+[error][1900-01-01T00:00:00.000000] Invalid controller: 'bad_host_name.example.com'
+--- Finished test-22 pbench-move-results (status=1)
++++ pbench tree state
+/var/tmp/pbench-test-utils/pbench
+/var/tmp/pbench-test-utils/pbench/pbench.log
+/var/tmp/pbench-test-utils/pbench/tmp
+--- pbench tree state
++++ pbench.log file contents
+[error][1900-01-01T00:00:00.000000] Invalid controller: 'bad_host_name.example.com'
+--- pbench.log file contents

agent/util-scripts/gold/pbench-register-tool/test-01.txt

@@ -0,0 +1,13 @@
++++ Running test-01 pbench-register-tool --name=mpstat --group=default --remote=invalid_remote.example.com,good.example.com,bad_remote.example.com
+[error][1900-01-01T00:00:00.000000] Invalid remote specified: 'invalid_remote.example.com'
+[error][1900-01-01T00:00:00.000000] Invalid remote specified: 'bad_remote.example.com'
+--- Finished test-01 pbench-register-tool (status=1)
++++ pbench tree state
+/var/tmp/pbench-test-utils/pbench
+/var/tmp/pbench-test-utils/pbench/pbench.log
+/var/tmp/pbench-test-utils/pbench/tmp
+--- pbench tree state
++++ pbench.log file contents
+[error][1900-01-01T00:00:00.000000] Invalid remote specified: 'invalid_remote.example.com'
+[error][1900-01-01T00:00:00.000000] Invalid remote specified: 'bad_remote.example.com'
+--- pbench.log file contents

agent/util-scripts/pbench-move-results

@@ -80,6 +80,15 @@ while true; do
     esac
 done
 
+if [[ -z "${controller}" ]]; then
+    controller=${_pbench_full_hostname}
+    if [[ -z "${controller}" ]]; then
+	error_log "Missing controller name (should be \"hostname -f\" value)"
+	exit 1
+    fi
+fi
+validate_hostname "${controller}" "controller"
+
 # Move into pbench run collection directory
 cd ${pbench_run} >/dev/null
 if [[ ${?} -ne 0 ]]; then
@@ -103,14 +112,6 @@ if [[ ! -s ${tmp}/results.lis ]]; then
     exit 0
 fi
 
-if [[ -z "${controller}" ]]; then
-    controller=${_pbench_full_hostname}
-    if [[ -z "${controller}" ]]; then
-	error_log "Missing controller name (should be \"hostname -f\" value)"
-	exit 1
-    fi
-fi
-
 if [[ ! -f "${pbench_bin}/id_rsa" ]]; then
     error_log "ERROR: ${pbench_bin}/id_rsa required for moving results to archive host"
     exit 1

agent/util-scripts/pbench-register-tool

@@ -278,6 +278,19 @@ else
 	fi
 fi
 
+invalids=0
+for (( i=0; ${i} < ${#remotes_A[@]}; i++ )); do
+	remote="${remotes_A[${i}]}"
+	validate-hostname "${remote}"
+	if [[ ${?} -ne 0 ]]; then
+		error_log "Invalid remote specified: '${remote}'"
+		(( invalids++ ))
+	fi
+done
+if [[ ${invalids} -gt 0 ]]; then
+	exit 1
+fi
+
 if [[ ${_do_test_labels} -ne 0 ]]; then
 	# Used by pbench-register-tool-set to avoid duplicating logic, we
 	# have been asked to exit early successfully if all argument

agent/util-scripts/test-bin/test-client-tool-meister

@@ -54,11 +54,11 @@ register mpstat
 register dcgm
 
 if [[ "${2}" == "with-remotes" ]]; then
-    register mpstat remote_a.example.com
-    register mpstat remote_b.example.com blue
-    register node-exporter remote_b.example.com blue
-    register dcgm remote_c.example.com red
-    register pcp remote_c.example.com red
+    register mpstat "remote-a.example.com"
+    register mpstat "remote-b.example.com" blue
+    register node-exporter "remote-b.example.com" blue
+    register dcgm "remote-c.example.com" red
+    register pcp "remote-c.example.com" red
 fi
 
 # We created the benchmark_run_dir directory as the tool meister expects it to

agent/util-scripts/unittests

@@ -31,7 +31,7 @@ mv ${_testopt}/config/pbench-agent.cfg ${_testopt}/config/pbench-agent.cfg.orig
 let res=res+${?}
 cp -a ${_tdir}/../templates/* ${_testopt}/templates/
 let res=res+${?}
-cp -a ${_tdir}/{require-rpm,pbench-*} ${_testopt}/util-scripts/
+cp -a ${_tdir}/{require-rpm,validate-*,pbench-*} ${_testopt}/util-scripts/
 let res=res+${?}
 cp -a ${_tdir}/tool-meister/pbench-* ${_testopt}/util-scripts/tool-meister/
 let res=res+${?}
@@ -345,6 +345,7 @@ let errs=0
 
 declare -A tools=(
     [test-00]="pbench-register-tool"
+    [test-01]="pbench-register-tool"
     [test-05]="pbench-start-tools"
     [test-06]="pbench-stop-tools"
     [test-07]="pbench-postprocess-tools"
@@ -371,6 +372,7 @@ declare -A tools=(
     [test-19]="test-tool-trigger"
     [test-20]="pbench-move-results"
     [test-21]="pbench-move-results"
+    [test-22]="pbench-move-results"
     [test-25]="pbench-verify-sysinfo-options"
     [test-26]="pbench-verify-sysinfo-options"
     [test-27]="pbench-verify-sysinfo-options"
@@ -413,6 +415,7 @@ declare -A sortem=(
 
 declare -A options=(
     [test-00]="--name=mpstat --group=default -- --interval=10"
+    [test-01]="--name=mpstat --group=default --remote=invalid_remote.example.com,good.example.com,bad_remote.example.com"
     [test-05]="--group=default --dir=42-iter/sample42"
     [test-06]="--group=default --dir=42-iter/sample42"
     [test-07]="--group=foobar --dir=${_testdir}/42-iter/sample42"
@@ -445,6 +448,7 @@ declare -A options=(
     [test-20]="--help"
     # pbench-move-results - no args, nothing to do
     #[test-21]=""
+    [test-22]="--controller=bad_host_name.example.com"
     [test-25]="all"
     [test-26]="none"
     [test-27]="default"
@@ -488,13 +492,15 @@ declare -A options=(
 )
 
 declare -A expected_status=(
+    [test-01]=1
     [test-09]=1
     [test-10]=1
     [test-11.07]=1
     [test-11.08]=1
     [test-11.11]=1
     [test-17]=1
     [test-18]=1
+    [test-22]=1
     [test-29]=1
     [test-44]=1
     [test-46]=1

agent/util-scripts/validate-hostname

@@ -0,0 +1,20 @@
+#!/usr/bin/env python3
+"""validate-hostname - validate the given hostname uses the proper syntax.
+
+Exits with 0 on success, with 1 on failure, only emitting a message on stderr
+if the argument is missing.
+"""
+
+import sys
+
+from pbench.agent.utils import validate_hostname
+
+
+try:
+    host_name = sys.argv[1]
+except IndexError:
+    print("Missing host name argument", file=sys.stderr)
+    exit_val = 1
+else:
+    exit_val = validate_hostname(host_name)
+sys.exit(exit_val)

agent/util-scripts/validate-ipaddress

@@ -0,0 +1,23 @@
+#!/usr/bin/env python3
+"""validate-ipaddress - check the first argument to see if it is a valid IP
+address (IPv4 or IPv6).  Returns 0 on success, 1 if the IP address is not
+valid, 2 if the IP address argument is missing, and 3 if an unexpected error
+is encountered.  No error message is emitted if the IP address is invalid.
+"""
+
+import ipaddress
+import sys
+
+try:
+    ipaddress.ip_address(sys.argv[1])
+except ValueError:
+    exit_val = 1
+except IndexError:
+    print("Missing IP address argument", file=sys.stderr)
+    exit_val = 2
+except Exception as exc:
+    print(f"Error processing IP address {sys.argv[1]}: {exc}", file=sys.stderr)
+    exit_val = 3
+else:
+    exit_val = 0
+sys.exit(exit_val)

lib/pbench/agent/utils.py

@@ -1,5 +1,7 @@
+import ipaddress
 import logging
 import os
+import re
 import subprocess
 import sys
 
@@ -188,3 +190,43 @@ def collect_local_info(pbench_bin):
         hostdata[arg] = cp.stdout.strip() if cp.stdout is not None else ""
 
     return (version, seqno, sha1, hostdata)
+
+
+# Derived from https://stackoverflow.com/questions/106179/regular-expression-to-match-dns-hostname-or-ip-address
+# with a few modifications: take advantage of ignoring case; use non-capturing
+# groups to improve efficiency; take advantage of RFC 1123 modification to RFC
+# 952 relaxing first character to be letter or digit, with the repetition moved
+# to the last group to alleviate backtracking.  Or in other words, a case-blind
+# comparison seeking a letter-or-digit, optionally followed by a sequence of 0
+# to 61 letter-or-digit-or-hyphens followed by a letter-or-digit, follwed by 0
+# or more instances of that same pattern preceded by a period.
+_allowed = re.compile(
+    r"[A-Z0-9](?:[A-Z0-9\-]{0,61}[A-Z0-9])?(?:\.[A-Z0-9](?:[A-Z0-9\-]{0,61}[A-Z0-9])?)*",
+    flags=re.IGNORECASE,
+)
+
+
+def validate_hostname(host_name: str):
+    """validate_hostname - validate the given hostname uses the proper syntax.
+
+    A host name that follows RFC 952 (amended by RFC 1123) is accepted only.
+    Host names are not resolved to IP addresses, and IP addresses are also
+    accepted.
+
+    Algorithm taken from: https://stackoverflow.com/questions/2532053/validate-a-hostname-string
+
+    Returns 0 on success, 1 on failure.
+    """
+    if not host_name or len(host_name) > 255:
+        return 1
+
+    if _allowed.fullmatch(host_name):
+        return 0
+
+    # It is not a valid host name, but could be a valid IP address.
+    try:
+        ipaddress.ip_address(host_name)
+    except ValueError:
+        return 1
+
+    return 0

lib/pbench/test/unit/agent/test_validate_hostname.py

@@ -0,0 +1,15 @@
+"""Tests for validate_hostname
+"""
+
+from pbench.agent.utils import validate_hostname
+
+
+def test_validate_hostname():
+    assert validate_hostname("test") == 0
+    assert validate_hostname("test.example.com") == 0
+    assert validate_hostname("tes_t.example.com") == 1
+    assert validate_hostname("test-.example.com") == 1
+    assert validate_hostname("--run-label__") == 1
+    assert validate_hostname("127.0.0.1") == 0
+    assert validate_hostname("1270.0.0.1") == 0
+    assert validate_hostname("2001:0db8:85a3:0000:0000:8a2e:0370:7334") == 0