[Swan-dev] Coverity Scan finds issue with programs/pluto/keys.c
Andrew Cagney
andrew.cagney at gmail.com
Tue May 4 13:34:16 UTC 2021
Here's a more up-to-date comment:
* 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 NULL NULL 0 no key, try again
* false NULL NULL >0 no key worked, try again
* true <valid> <valid> N/A fatal error caused by KEY
* true NULL <valid> N/A KEY worked
On Tue, 4 May 2021 at 09:18, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>
>
> On Tue, 4 May 2021 at 01:15, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
>
>> | 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.
>>
>
> 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
>> Swan-dev at lists.libreswan.org
>> https://lists.libreswan.org/mailman/listinfo/swan-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libreswan.org/pipermail/swan-dev/attachments/20210504/372f517e/attachment-0001.html>
More information about the Swan-dev
mailing list