[Swan-dev] Coverity Scan finds issue with programs/pluto/keys.c

[D. Hugh Redelmeier]

| From: scan-admin at coverity.com

| ________________________________________________________________________________________________________
| *** CID 1504578:    (FORWARD_NULL)
| /programs/pluto/keys.c: 529 in authsig_and_log_using_pubkey()
| 523     		stop = try_all_keys("preloaded", pluto_pubkeys, &s);
| 524     	}
| 525     
| 526     	if (s.fatal_diag != NULL) {
| 527     		LLOG_JAMBUF(RC_LOG_SERIOUS, ike->sa.st_logger, buf) {
| 528     			jam(buf, "authentication aborted: problem with '");
| >>>     CID 1504578:    (FORWARD_NULL)
| >>>     Passing null pointer "&s.key->id" to "jam_id", which dereferences it.
| 529     			jam_id(buf, &s.key->id, jam_sanitized_bytes);
| 530     			jam(buf, "': ");
| 531     			jam_diag(buf, s.fatal_diag);
| 532     			pfree_diag(&s.fatal_diag);
| 533     		}
| 534     		return STF_FATAL;
| /programs/pluto/keys.c: 529 in authsig_and_log_using_pubkey()
| 523     		stop = try_all_keys("preloaded", pluto_pubkeys, &s);
| 524     	}
| 525     
| 526     	if (s.fatal_diag != NULL) {
| 527     		LLOG_JAMBUF(RC_LOG_SERIOUS, ike->sa.st_logger, buf) {
| 528     			jam(buf, "authentication aborted: problem with '");
| >>>     CID 1504578:    (FORWARD_NULL)
| >>>     Passing null pointer "&s.key->id" to "jam_id", which dereferences it.
| 529     			jam_id(buf, &s.key->id, jam_sanitized_bytes);
| 530     			jam(buf, "': ");
| 531     			jam_diag(buf, s.fatal_diag);
| 532     			pfree_diag(&s.fatal_diag);
| 533     		}
| 534     		return STF_FATAL;

I'm suspicious about the case where both calls to try_all_keys return
true.  I'll call this "double stop".

According to the comment on try_all_keys, not much is specified about
the results when false is returned.

Perhaps something like this would fix the (possibly non-existent)
problem.  Clearly the message and the message generation should be
improved.

It isn't clear to me that this actually fixes the problem that
Coverity Scan saw.

diff --git a/programs/pluto/keys.c b/programs/pluto/keys.c
index 78aa0ec8bd..013fb47885 100644
--- a/programs/pluto/keys.c
+++ b/programs/pluto/keys.c
@@ -521,6 +521,12 @@ stf_status authsig_and_log_using_pubkey(struct ike_sa *ike,
 	bool stop = try_all_keys("peer", ike->sa.st_remote_certs.pubkey_db, &s);
 	if (!stop) {
 		stop = try_all_keys("preloaded", pluto_pubkeys, &s);
+		if (!stop) {
+			LLOG_JAMBUF(RC_LOG_SERIOUS, ike->sa.st_logger, buf) {
+				jam(buf, "authentication aborted: could not find public keyskeys");
+			}
+			return STF_FATAL;
+		}
 	}
 
 	if (s.fatal_diag != NULL) {