[PATCH 2/4] sshsig: fix find-principals key lifespan validation

[Fabian Stelzer]

find-principals was verifying key validity when using ca certs but not
with simple key lifetimes within the allowed signers file.
Since it returns the first keys principal it finds this could result in
a principal with an expired key even though a valid one is just below.

- moves timespan validity check code from check_allowed_keys_line() into a
  seperate function
- reuses this function then in get_matching_principals_from_line()
- adds tests for both CA & normal keys

The find-principals tests using normal keys would fail before this
patch while the identical ones with CA keys suceeded.

Signed-off-by: Fabian Stelzer <fs at gigacodes.de>
---
 regress/sshsig.sh | 49 ++++++++++++++++++++++++++++++++++++++++++
 sshsig.c          | 54 +++++++++++++++++++++++++++++------------------
 2 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/regress/sshsig.sh b/regress/sshsig.sh
index fd015378..9947afc1 100644
--- a/regress/sshsig.sh
+++ b/regress/sshsig.sh
@@ -149,6 +149,26 @@ for t in $SIGNKEYS; do
 		< $DATA >/dev/null 2>&1 && \
 		fail "failed signature for $t with expired key"
 
+	# key lifespan valid
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-Overify-time="19850101" \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 || \
+		fail "failed find-principals for $t key with valid expiry interval"
+	# key not yet valid
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-Overify-time="19790101" \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 && \
+		fail "failed find-principals for $t not-yet-valid key"
+	# key expired
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-Overify-time="19990101" \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 && \
+		fail "failed find-principals for $t with expired key"
+	# NB. assumes we're not running this test in the 1980s
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 && \
+		fail "failed find-principals for $t with expired key"
+
 	# public key in revoked keys file
 	cat $pubkey > $OBJ/revoked_keys
 	(printf "$sig_principal namespaces=\"whatever\" " ;
@@ -211,6 +231,29 @@ for t in $SIGNKEYS; do
 		*) continue ;;
 	esac
 
+	# Check key lifespan on find-principals when using the CA
+	( printf "$sig_principal " ;
+	  printf "cert-authority,valid-after=\"19800101\",valid-before=\"19900101\" ";
+	  cat $CA_PUB) > $OBJ/allowed_signers
+	# key lifespan valid
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-Overify-time="19850101" \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 || \
+		fail "failed find-principals for $t key with valid expiry interval"
+	# key not yet valid
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-Overify-time="19790101" \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 && \
+		fail "failed find-principals for $t not-yet-valid key"
+	# key expired
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-Overify-time="19990101" \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 && \
+		fail "failed find-principals for $t with expired key"
+	# NB. assumes we're not running this test in the 1980s
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 && \
+		fail "failed find-principals for $t with expired key"
 
 	# correct CA key
 	(printf "$sig_principal cert-authority " ;
@@ -221,6 +264,12 @@ for t in $SIGNKEYS; do
 		< $DATA >/dev/null 2>&1 || \
 		fail "failed signature for $t cert"
 
+	# find-principals
+	${SSHKEYGEN} -vvv -Y find-principals -s $sigfile \
+		-Overify-time=19850101 \
+		-f $OBJ/allowed_signers >/dev/null 2>&1 || \
+		fail "failed find-principals for $t with ca key"
+
 	# signing key listed as cert-authority
 	(printf "$sig_principal cert-authority " ;
 	 cat $pubkey) > $OBJ/allowed_signers
diff --git a/sshsig.c b/sshsig.c
index d0d401a3..b50a9779 100644
--- a/sshsig.c
+++ b/sshsig.c
@@ -812,6 +812,35 @@ parse_principals_key_and_options(const char *path, u_long linenum, char *line,
 	return r;
 }
 
+static int
+check_key_validity(uint64_t verify_time, struct sshsigopt *sigopts, const char *path, u_long linenum)
+{
+	char tvalid[64], tverify[64];
+	/* check key time validity */
+	format_absolute_time((uint64_t)verify_time, tverify, sizeof(tverify));
+	if (sigopts->valid_after != 0 &&
+		(uint64_t)verify_time < sigopts->valid_after) {
+		format_absolute_time(sigopts->valid_after,
+			tvalid, sizeof(tvalid));
+		if (path)
+			error("%s:%lu: key is not yet valid: "
+				"verify time %s < valid-after %s", path, linenum,
+				tverify, tvalid);
+		return 1;
+	}
+	if (sigopts->valid_before != 0 &&
+		(uint64_t)verify_time > sigopts->valid_before) {
+		format_absolute_time(sigopts->valid_before,
+			tvalid, sizeof(tvalid));
+		if (path)
+			error("%s:%lu: key has expired: "
+				"verify time %s > valid-before %s", path, linenum,
+				tverify, tvalid);
+		return 1;
+	}
+	return 0;
+}
+
 static int
 check_allowed_keys_line(const char *path, u_long linenum, char *line,
     const struct sshkey *sign_key, const char *principal,
@@ -821,7 +850,6 @@ check_allowed_keys_line(const char *path, u_long linenum, char *line,
 	int r, success = 0;
 	const char *reason = NULL;
 	struct sshsigopt *sigopts = NULL;
-	char tvalid[64], tverify[64];
 
 	/* Parse the line */
 	if ((r = parse_principals_key_and_options(path, linenum, line,
@@ -856,26 +884,9 @@ check_allowed_keys_line(const char *path, u_long linenum, char *line,
 		goto done;
 	}
 
-	/* check key time validity */
-	format_absolute_time((uint64_t)verify_time, tverify, sizeof(tverify));
-	if (sigopts->valid_after != 0 &&
-	    (uint64_t)verify_time < sigopts->valid_after) {
-		format_absolute_time(sigopts->valid_after,
-		    tvalid, sizeof(tvalid));
-		error("%s:%lu: key is not yet valid: "
-		    "verify time %s < valid-after %s", path, linenum,
-		    tverify, tvalid);
+	if (check_key_validity(verify_time, sigopts, path, linenum))
 		goto done;
-	}
-	if (sigopts->valid_before != 0 &&
-	    (uint64_t)verify_time > sigopts->valid_before) {
-		format_absolute_time(sigopts->valid_before,
-		    tvalid, sizeof(tvalid));
-		error("%s:%lu: key has expired: "
-		    "verify time %s > valid-before %s", path, linenum,
-		    tverify, tvalid);
-		goto done;
-	}
+
 	success = 1;
 
  done:
@@ -998,6 +1009,9 @@ get_matching_principals_from_line(const char *path, u_long linenum, char *line,
 		goto done;
 	}
 
+	if (check_key_validity(verify_time, sigopts, NULL, 0))
+			goto done;
+
 	if (!sigopts->ca && sshkey_equal(found_key, sign_key)) {
 		/* Exact match of key */
 		debug("%s:%lu: matched key", path, linenum);
-- 
2.31.1