local/require-signed-push: document push time requirements and improve error messages

Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>

1 files changed, 43 insertions, 6 deletions

diff --git a/local/require-signed-push b/local/require-signed-push
index f9dd405..20bc2e1 100755
--- a/local/require-signed-push
+++ b/local/require-signed-push
@@ -6,7 +6,8 @@ VERIFY_SIGS=$(git config --get gentoo.verify-signatures)
 
 # ----------------------------------------------------------------------
 # standard stuff
-die() { echo "$@" >&2; exit 1; }
+silent_die() { exit 1 }
+die() { echo "$@" >&2; silent_die }
 warn() { echo "$@" >&2; }
 
 fail_signed_push() {
@@ -71,6 +72,13 @@ verify_pusher_clock() {
 	# pusher 94BFDF4484AD142F 1468596642 +0200
 	# nonce 1468596917-ac5118c996e285ace24e
 	# -------
+	# Bad clock, ~5 mins fast
+	# -------
+	# pushtime 1468596921
+	# certificate version 0.1
+	# pusher 94BFDF4484AD142F 1468597242 +0200
+	# nonce 1468596917-ac5118c996e285ace24e
+	# -------
 
 	# This is the time, according to server clock, that the server sent out to the user
 	# also in GIT_PUSH_CERT_NONCE
@@ -88,24 +96,53 @@ verify_pusher_clock() {
 	[[ -z "$SERVER_NONCE_TIME" ]] && die "require-signed-push: Could not find push nonce"
 	[[ -z "$PUSHER_SIGN_TIME" ]] && die "require-signed-push: Could not find pusher identity"
 
+	# Timestamps:
+	# T0: nonce generation time (server, trusted input)
+	# T1: nonce signature time (client, untrusted input!)
+	# T2: signed-nonce receive time (server, trusted input)
 	T0="$SERVER_NONCE_TIME"
 	T1="$PUSHER_SIGN_TIME"
 	T2="$SERVER_PUSH_TIME"
+	# Durations:
+	# T1-T0: how long it took the client to get the nonce and sign it (depends on untrusted input)
+	# - will be negative if the client (T1) has a clock BEHIND of server (T0),
+	#   e.g. client clock is "slow"
+	#
+	# T2-T0: how long the roundtrip took (only contains trusted inputs)
+	# - will only be negative if the server clock jump backwards during the round-trip!
+	#
+	# T2-T1: how long it took the client to send the signed nonce back (depends on untrusted input)
+	# - will be negative if the client (T1) has a clock AHEAD of server (T2),
+	#   e.g. clock is "fast"
 	DELTA_T1_T0=$(( T1 - T0 ))
 	DELTA_T2_T0=$(( T2 - T0 ))
 	DELTA_T2_T1=$(( T2 - T1 ))
+
+	# Flip the signs, because we care about magnitude, not if they are fast or slow.
 	[[ $DELTA_T1_T0 -lt 0 ]] && DELTA_T1_T0=$(( DELTA_T1_T0 * -1 ))
-	[[ $DELTA_T2_T0 -lt 0 ]] && DELTA_T2_T0=$(( DELTA_T2_T0 * -1 ))
 	[[ $DELTA_T2_T1 -lt 0 ]] && DELTA_T2_T1=$(( DELTA_T2_T1 * -1 ))
+	# This one should never happen unless the server's clock has gone backwards during the round trip period.
+	[[ $DELTA_T2_T0 -lt 0 ]] && die "Server clock moved backwards during process, please report to infra@ and retry!"
 
 	CLOCK_DRIFT_LIMIT=5
 	PUSH_LIMIT=60
+	_die=0
+	# Put the stricter check first, otherwise the weaker check will never be seen.
+	if [[ $DELTA_T2_T0 -ge $PUSH_LIMIT ]]; then
+		warn "Push roundtrip took too long (push-nonce): $DELTA_T2_T0 sec vs limit $PUSH_LIMIT"
+		_die=1
+	fi
 	if [[ $DELTA_T1_T0 -ge $CLOCK_DRIFT_LIMIT ]]; then
-		warn "Your system clock is off by $DELTA_T1_T0 seconds (limit $CLOCK_DRIFT_LIMIT)"
-		die "Run NTP, rebase your commits as needed, and push again."
+		warn "Push certificate time is too skew (sign-nonce)."
+		warn "It's possible your system clock is off by up to $DELTA_T1_T0 seconds vs limit $CLOCK_DRIFT_LIMIT"
+		warn "Run NTP, pull & rebase your commits if needed, and push again."
+		_die=1
 	fi
-	if [[ $DELTA_T2_T0 -ge $PUSH_LIMIT ]]; then
-		die "Try again! Your push took $DELTA_T2_T0 seconds, (limit $PUSH_LIMIT)."
+	if [[ $_die -eq 1 ]]; then
+		warn "---cut-here---"
+		git show --format='pushtime %ct%nct %ct%nat %at%n%B' "$GIT_PUSH_CERT" 1>&2
+		warn "---cut-here---"
+		die "Time issues during git-push"
 	fi
 }