On Tue, 4 May 2021 at 01:15, D. Hugh Redelmeier <[email protected]> wrote:
> | From: [email protected] > > | > ________________________________________________________________________________________________________ > | *** 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. > I suspect Coverity is confused: /* * Try all keys from PUBKEY_DB. * * Return true when searching should stop (not when it succeeded); * return false when searching can continue. * * Returns FATAL_DIAG KEY tried_cnt * false can try again * true <valid> <valid> N/A fatal error caused by KEY * true NULL NULL 0 no key found * true NULL NULL >0 no key worked * true NULL <valid> N/A KEY worked */ FATAIL_DIAG => KEY BTW, the below would defeat the code listing all the keys that were tried. 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) { > _______________________________________________ > Swan-dev mailing list > [email protected] > https://lists.libreswan.org/mailman/listinfo/swan-dev >
_______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
