denozord/strongswan-ext
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

child-sa: Update status flags based on success of SA addition

Browse Source 
Both variables `inbound_installed` and `outbound_state` are used in
`child_sa_t::destroy()` to determine whether inbound and outbound state
have to be deleted. They are assigned prior to the call to
`kernel_interface_t::add_sa()`. As this call may fail, the destructor may
try to delete a state which it has not been added.
By making the assignment of these variables dependent on the success of
the state addition, we can make sure, a `child_sa_t::destroy()` only
deletes states it has added.

Also removed the redundant checks for `my_spi` and `other_spi` being set
along with the check for the above flags. It seems that when the flags
are set, the SPIs *must* be set.

Signed-off-by: Thomas Egerer <[email protected]>
This commit is contained in:
Thomas Egerer
2024-11-29 13:46:12 +01:00
committed by Tobias Brunner
parent 8b69327ad2
commit fefea48724
1 changed files with 16 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libcharon/sa/child_sa.c
+16 -7
View File
@@ -620,7 +620,7 @@ static status_t update_usebytes(private_child_sa_t *this, bool inbound)
if (inbound) if (inbound)
{ {
if (this->my_spi && this->inbound_installed) if (this->inbound_installed)
{ {
kernel_ipsec_sa_id_t id = { kernel_ipsec_sa_id_t id = {
.src = this->other_addr, .src = this->other_addr,
@@ -654,7 +654,7 @@ static status_t update_usebytes(private_child_sa_t *this, bool inbound)
} }
else else
{ {
if (this->other_spi && (this->outbound_state & CHILD_OUTBOUND_SA)) if (this->outbound_state & CHILD_OUTBOUND_SA)
{ {
kernel_ipsec_sa_id_t id = { kernel_ipsec_sa_id_t id = {
.src = this->my_addr, .src = this->my_addr,
@@ -939,7 +939,6 @@ static status_t install_internal(private_child_sa_t *this, chunk_t encr,
this->my_cpi = cpi; this->my_cpi = cpi;
dst_ts = my_ts; dst_ts = my_ts;
src_ts = other_ts; src_ts = other_ts;
this->inbound_installed = TRUE;
} }
else else
{ {
@@ -954,7 +953,6 @@ static status_t install_internal(private_child_sa_t *this, chunk_t encr,
{ {
tfc = this->config->get_tfc(this->config); tfc = this->config->get_tfc(this->config);
} }
this->outbound_state |= CHILD_OUTBOUND_SA;
} }
DBG2(DBG_CHD, "adding %s %N SA", inbound ? "inbound" : "outbound", DBG2(DBG_CHD, "adding %s %N SA", inbound ? "inbound" : "outbound",
@@ -1061,6 +1059,17 @@ static status_t install_internal(private_child_sa_t *this, chunk_t encr,
other_ts->destroy(other_ts); other_ts->destroy(other_ts);
free(lifetime); free(lifetime);
if (status == SUCCESS)
{
if (inbound)
{
this->inbound_installed = TRUE;
}
else
{
this->outbound_state |= CHILD_OUTBOUND_SA;
}
}
return status; return status;
} }
@@ -1623,7 +1632,7 @@ static status_t update_sas(private_child_sa_t *this, host_t *me, host_t *other,
bool encap, uint32_t reqid) bool encap, uint32_t reqid)
{ {
/* update our (initiator) SA */ /* update our (initiator) SA */
if (this->my_spi && this->inbound_installed) if (this->inbound_installed)
{ {
kernel_ipsec_sa_id_t id = { kernel_ipsec_sa_id_t id = {
.src = this->other_addr, .src = this->other_addr,
@@ -1649,7 +1658,7 @@ static status_t update_sas(private_child_sa_t *this, host_t *me, host_t *other,
} }
/* update his (responder) SA */ /* update his (responder) SA */
if (this->other_spi && (this->outbound_state & CHILD_OUTBOUND_SA)) if (this->outbound_state & CHILD_OUTBOUND_SA)
{ {
kernel_ipsec_sa_id_t id = { kernel_ipsec_sa_id_t id = {
.src = this->my_addr, .src = this->my_addr,
@@ -1960,7 +1969,7 @@ METHOD(child_sa_t, destroy, void,
}; };
charon->kernel->del_sa(charon->kernel, &id, &sa); charon->kernel->del_sa(charon->kernel, &id, &sa);
} }
if (this->other_spi && (this->outbound_state & CHILD_OUTBOUND_SA)) if (this->outbound_state & CHILD_OUTBOUND_SA)
{ {
kernel_ipsec_sa_id_t id = { kernel_ipsec_sa_id_t id = {
.src = this->my_addr, .src = this->my_addr,