<div dir="ltr"><div>Here's a more up-to-date comment:</div><div><br> * Try all keys from PUBKEY_DB.<br> *<br> * Return true when searching should stop (not when it succeeded);<br> * return false when searching can continue.<br> *<br> * Returns FATAL_DIAG KEY tried_cnt<br> * false NULL NULL 0 no key, try again<br> * false NULL NULL >0 no key worked, try again<br> * true <valid> <valid> N/A fatal error caused by KEY<br> * true NULL <valid> N/A KEY worked</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 4 May 2021 at 09:18, Andrew Cagney <<a href="mailto:andrew.cagney@gmail.com">andrew.cagney@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 4 May 2021 at 01:15, D. Hugh Redelmeier <<a href="mailto:hugh@mimosa.com" target="_blank">hugh@mimosa.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">| From: <a href="mailto:scan-admin@coverity.com" target="_blank">scan-admin@coverity.com</a><br>
<br>
| ________________________________________________________________________________________________________<br>
| *** CID 1504578: (FORWARD_NULL)<br>
| /programs/pluto/keys.c: 529 in authsig_and_log_using_pubkey()<br>
| 523 stop = try_all_keys("preloaded", pluto_pubkeys, &s);<br>
| 524 }<br>
| 525 <br>
| 526 if (s.fatal_diag != NULL) {<br>
| 527 LLOG_JAMBUF(RC_LOG_SERIOUS, ike->sa.st_logger, buf) {<br>
| 528 jam(buf, "authentication aborted: problem with '");<br>
| >>> CID 1504578: (FORWARD_NULL)<br>
| >>> Passing null pointer "&s.key->id" to "jam_id", which dereferences it.<br>
| 529 jam_id(buf, &s.key->id, jam_sanitized_bytes);<br>
| 530 jam(buf, "': ");<br>
| 531 jam_diag(buf, s.fatal_diag);<br>
| 532 pfree_diag(&s.fatal_diag);<br>
| 533 }<br>
| 534 return STF_FATAL;<br>
| /programs/pluto/keys.c: 529 in authsig_and_log_using_pubkey()<br>
| 523 stop = try_all_keys("preloaded", pluto_pubkeys, &s);<br>
| 524 }<br>
| 525 <br>
| 526 if (s.fatal_diag != NULL) {<br>
| 527 LLOG_JAMBUF(RC_LOG_SERIOUS, ike->sa.st_logger, buf) {<br>
| 528 jam(buf, "authentication aborted: problem with '");<br>
| >>> CID 1504578: (FORWARD_NULL)<br>
| >>> Passing null pointer "&s.key->id" to "jam_id", which dereferences it.<br>
| 529 jam_id(buf, &s.key->id, jam_sanitized_bytes);<br>
| 530 jam(buf, "': ");<br>
| 531 jam_diag(buf, s.fatal_diag);<br>
| 532 pfree_diag(&s.fatal_diag);<br>
| 533 }<br>
| 534 return STF_FATAL;<br>
<br>
I'm suspicious about the case where both calls to try_all_keys return<br>
true. I'll call this "double stop".<br>
<br>
According to the comment on try_all_keys, not much is specified about<br>
the results when false is returned.<br>
<br>
Perhaps something like this would fix the (possibly non-existent)<br>
problem. Clearly the message and the message generation should be<br>
improved.<br>
<br>
It isn't clear to me that this actually fixes the problem that<br>
Coverity Scan saw.<br></blockquote><div><br></div><div>I suspect Coverity is confused:</div><div><br></div><div>/*<br> * Try all keys from PUBKEY_DB.<br> *<br> * Return true when searching should stop (not when it succeeded);<br> * return false when searching can continue.<br> *<br> * Returns FATAL_DIAG KEY tried_cnt<br> * false can try again<br> * true <valid> <valid> N/A fatal error caused by KEY<br> * true NULL NULL 0 no key found<br> * true NULL NULL >0 no key worked<br> * true NULL <valid> N/A KEY worked<br> */</div><div><br></div><div>FATAIL_DIAG => KEY</div><div><br></div><div> BTW, the below would defeat the code listing all the keys that were tried.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
diff --git a/programs/pluto/keys.c b/programs/pluto/keys.c<br>
index 78aa0ec8bd..013fb47885 100644<br>
--- a/programs/pluto/keys.c<br>
+++ b/programs/pluto/keys.c<br>
@@ -521,6 +521,12 @@ stf_status authsig_and_log_using_pubkey(struct ike_sa *ike,<br>
bool stop = try_all_keys("peer", ike->sa.st_remote_certs.pubkey_db, &s);<br>
if (!stop) {<br>
stop = try_all_keys("preloaded", pluto_pubkeys, &s);<br>
+ if (!stop) {<br>
+ LLOG_JAMBUF(RC_LOG_SERIOUS, ike->sa.st_logger, buf) {<br>
+ jam(buf, "authentication aborted: could not find public keyskeys");<br>
+ }<br>
+ return STF_FATAL;<br>
+ }<br>
}<br>
<br>
if (s.fatal_diag != NULL) {<br>
_______________________________________________<br>
Swan-dev mailing list<br>
<a href="mailto:Swan-dev@lists.libreswan.org" target="_blank">Swan-dev@lists.libreswan.org</a><br>
<a href="https://lists.libreswan.org/mailman/listinfo/swan-dev" rel="noreferrer" target="_blank">https://lists.libreswan.org/mailman/listinfo/swan-dev</a><br>
</blockquote></div></div>
</blockquote></div>