From eaf1d08205642fe962061725203e6c89bd796574 Mon Sep 17 00:00:00 2001
From: afresh1 <afresh1@openbsd.org>
Date: Wed, 15 Nov 2023 02:00:02 +0000
Subject: [PATCH] Better handle ftp errors in fw_update

Trap STDERR to post-process it looking for 404 errors to handle them differently.

The fetch method now also returns different error codes for errors that can
continue on.  Currently only 404 is special and everything else should cause
fw_update to exit early without trying all the files.

Exit early if the SHA256.sig gets a 404 because that is required to figure out
what valid firmware are.
---
 usr.sbin/fw_update/fw_update.sh | 68 ++++++++++++++++++++++++++-------
 1 file changed, 55 insertions(+), 13 deletions(-)

diff --git a/usr.sbin/fw_update/fw_update.sh b/usr.sbin/fw_update/fw_update.sh
index a0814b5e1f9..38a65388d06 100644
--- a/usr.sbin/fw_update/fw_update.sh
+++ b/usr.sbin/fw_update/fw_update.sh
@@ -1,5 +1,5 @@
 #!/bin/ksh
-#	$OpenBSD: fw_update.sh,v 1.52 2023/11/15 01:54:01 afresh1 Exp $
+#	$OpenBSD: fw_update.sh,v 1.53 2023/11/15 02:00:02 afresh1 Exp $
 #
 # Copyright (c) 2021,2023 Andrew Hewus Fresh <afresh1@openbsd.org>
 #
@@ -108,21 +108,24 @@ spin() {
 
 fetch() {
 	local _src="${FWURL}/${1##*/}" _dst=$1 _user=_file _exit _error=''
+	local _ftp_errors="$FD_DIR/ftp_errors"
+	rm -f "$_ftp_errors"
 
 	# The installer uses a limited doas(1) as a tiny su(1)
 	set -o monitor # make sure ftp gets its own process group
 	(
 	_flags=-vm
 	case "$VERBOSE" in
-		0|1) _flags=-VM ;;
+		0|1) _flags=-VM ; exec 2>"$_ftp_errors" ;;
 		  2) _flags=-Vm ;;
 	esac
+
 	if [ -x /usr/bin/su ]; then
 		exec /usr/bin/su -s /bin/ksh "$_user" -c \
-		    "/usr/bin/ftp -N '${0##/}' -D 'Get/Verify' $_flags -o- '$_src'" > "$_dst"
+		    "/usr/bin/ftp -N error -D 'Get/Verify' $_flags -o- '$_src'" > "$_dst"
 	else
 		exec /usr/bin/doas -u "$_user" \
-		    /usr/bin/ftp -N "${0##/}" -D 'Get/Verify' $_flags -o- "$_src" > "$_dst"
+		    /usr/bin/ftp -N error -D 'Get/Verify' $_flags -o- "$_src" > "$_dst"
 	fi
 	) & FTPPID=$!
 	set +o monitor
@@ -152,13 +155,34 @@ fetch() {
 
 	unset FTPPID
 
-	if [ "$_exit" -ne 0 ]; then
+	if ((_exit != 0)); then
 		rm -f "$_dst"
+
+		# ftp doesn't provide useful exit codes
+		# so we have to grep its STDERR.
+		# _exit=2 means don't keep trying
+		_exit=2
+
+		# If it was 404, we might succeed at another file
+		if [ -s "$_ftp_errors" ] && \
+		    grep -q "404 Not Found" "$_ftp_errors"; then
+			_exit=1
+			_error=" (404 Not Found)"
+			rm -f "$_ftp_errors"
+		fi
+
 		warn "Cannot fetch $_src$_error"
-		return 1
 	fi
 
-	return 0
+	# If we have ftp errors, print them out,
+	# removing any cntrl characters (like 0x0d),
+	# and any leading blank lines.
+	if [ -s "$_ftp_errors" ]; then
+		sed -e 's/[[:cntrl:]]//g' \
+		    -e '/./,$!d' "$_ftp_errors" >&"$WARN_FD"
+	fi
+
+	return "$_exit"
 }
 
 # If we fail to fetch the CFILE, we don't want to try again
@@ -166,12 +190,12 @@ fetch() {
 # a blank file indicating failure.
 check_cfile() {
 	if [ -e "$CFILE" ]; then
-		[ -s "$CFILE" ] || return 1
+		[ -s "$CFILE" ] || return 2
 		return 0
 	fi
 	if ! fetch_cfile; then
 		echo -n > "$CFILE"
-		return 1
+		return 2
 	fi
 	return 0
 }
@@ -193,7 +217,7 @@ fetch_cfile() {
 }
 
 verify() {
-	check_cfile || return 1
+	check_cfile || return $?
 	# The installer sha256 lacks -C, do it by hand
 	if ! grep -Fqx "SHA256 (${1##*/}) = $( /bin/sha256 -qb "$1" )" "$CFILE"
 	then
@@ -208,7 +232,7 @@ verify() {
 # if VERBOSE is 0, don't show the checksum failure of an existing file.
 verify_existing() {
 	local _v=$VERBOSE
-	check_cfile || return 1
+	check_cfile || return $?
 
 	((_v == 0)) && "$DOWNLOAD" && _v=1
 	( VERBOSE=$_v verify "$@" )
@@ -243,7 +267,7 @@ firmware_in_dmesg() {
 }
 
 firmware_filename() {
-	check_cfile || return 1
+	check_cfile || return $?
 	sed -n "s/.*(\($1-firmware-.*\.tgz\)).*/\1/p" "$CFILE" | sed '$!d'
 }
 
@@ -600,7 +624,18 @@ if [ "${devices[*]:-}" ]; then
 
 		verify_existing=true
 		if [ "$f" = "$d" ]; then
-			f=$( firmware_filename "$d" ) || continue
+			f=$( firmware_filename "$d" ) || {
+				# Fetching the CFILE here is often the
+				# first attempt to talk to FWURL
+				# If it fails, no point in continuing.
+				if (($? > 1)); then
+					status " failed."
+					exit 1
+				fi
+
+				# otherwise we can try the next firmware
+				continue
+			}
 			if [ ! "$f" ]; then
 				if "$INSTALL" && unregister_firmware "$d"; then
 					unregister="$unregister,$d"
@@ -718,6 +753,8 @@ for f in "${add[@]}" _update_ "${update[@]}"; do
 			fi
 			fetch  "$f" &&
 			verify "$f" || {
+				integer e=$?
+
 				"$pending_status" && echo " failed."
 				status " failed (${f##*/})"
 
@@ -725,6 +762,11 @@ for f in "${add[@]}" _update_ "${update[@]}"; do
 					cat "$FD_DIR/warn" >&2
 					rm -f "$FD_DIR/warn"
 				fi
+
+				# Fetch or verify exited > 1
+				# which means we don't keep trying.
+				((e > 1)) && exit 1
+
 				continue
 			}
 		fi
-- 
2.20.1