'Inverting' the logic yields
if ( ! (accepted_dh == NULL || accepted_dh ==
st->st_pfs_group)) {
which doesn't look right either. Mayhap it should be
/*
* Verify the pointer isn't null before comparing.
* The error is that either there's no DH or it doesn't match what was
negotiated.
* Otherwise the DH is stored.
*/
if ( accepted_dh == NULL || accepted_dh != st->st_pfs_group) {
But this still doesn't look quite right. If st->st_pfs_group is null, you want
to save the DH, right?
Maybe it wants to be
if ( accepted_dh == NULL || (st->st_pfs_group != NULL &&
accepted_dh != st->st_pfs_group)) {
N
On Sun, 3 Jun 2018 18:18:28 -0400 (EDT)
"D. Hugh Redelmeier" <[email protected]> wrote:
> /*
> * Update/check the PFS.
> *
> * For the responder, go with what ever was negotiated. For
> * the initiator, check what was negotiated against what was
> * sent.
> */
> const struct oakley_group_desc *accepted_dh =
> proto_info->attrs.transattrs.ta_dh;
> switch (st->st_sa_role) {
> case SA_INITIATOR:
> pexpect(expect_accepted);
> if (accepted_dh != NULL && accepted_dh != st->st_pfs_group) {
> loglog(RC_LOG_SERIOUS,
> "expecting %s but remote's accepted proposal
> includes %s",
> st->st_pfs_group == NULL ? "no DH" :
> st->st_pfs_group->common.fqn,
> accepted_dh == NULL ? "no DH" :
> accepted_dh->common.fqn);
> return STF_FAIL + v2N_NO_PROPOSAL_CHOSEN;
> }
> st->st_pfs_group = accepted_dh;
> break;
>
> coverity noticed that
> accepted_dh == NULL ? "no DH" :
> accepted_dh->common.fqn);
> was silly because we know that accepted_dh is not NULL (the preceding if
> checks).
>
> I'm wondering, whether the test is correct. Should it be || instead of &&?
> It's not clear to me.
> _______________________________________________
> 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
