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

Merge branch 'tkm-cid-ref'

Browse Source 
This fixes IKE_SA rekey collision handling and improves error handling
if CHILD_SA setup fails.
This commit is contained in:
Tobias Brunner
2017-09-15 12:17:07 +02:00
parent da479ae2c9 fc08e6af8a
commit 42aa569617
5 changed files with 157 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/charon-tkm/src/tkm/tkm_id_manager.c
+40 -8
View File
@@ -43,7 +43,7 @@ struct private_tkm_id_manager_t {
/**
* Per-kind array of free context ids
*/
bool* ctxids[TKM_CTX_MAX];
int* ctxids[TKM_CTX_MAX];
/**
* Per-kind context limits.
@@ -85,9 +85,9 @@ METHOD(tkm_id_manager_t, acquire_id, int,
this->locks[kind]->write_lock(this->locks[kind]);
for (j = 0; j < this->limits[kind]; j++)
{
if (!this->ctxids[kind][j])
if (this->ctxids[kind][j] == 0)
{
this->ctxids[kind][j] = true;
this->ctxids[kind][j] = 1;
id = j + 1;
break;
}
@@ -103,24 +103,55 @@ METHOD(tkm_id_manager_t, acquire_id, int,
return id;
}
METHOD(tkm_id_manager_t, release_id, bool,
METHOD(tkm_id_manager_t, acquire_ref, bool,
private_tkm_id_manager_t * const this, const tkm_context_kind_t kind,
const int ref_id)
{
const int idx = ref_id - 1;
if (!is_valid_kind(kind))
{
DBG1(DBG_LIB, "tried to acquire reference for invalid context kind '%d'",
kind);
return FALSE;
}
if (ref_id < 1 || (uint64_t)ref_id > this->limits[kind])
{
DBG1(DBG_LIB, "tried to acquire reference for context id %d out of "
"bounds (max %llu)", ref_id, this->limits[kind]);
return FALSE;
}
this->locks[kind]->write_lock(this->locks[kind]);
this->ctxids[kind][idx]++;
this->locks[kind]->unlock(this->locks[kind]);
return TRUE;
}
METHOD(tkm_id_manager_t, release_id, int,
private_tkm_id_manager_t * const this, const tkm_context_kind_t kind,
const int id)
{
const int idx = id - 1;
int refcount = 0;
if (!is_valid_kind(kind))
{
DBG1(DBG_LIB, "tried to release id %d for invalid context kind '%d'",
id, kind);
return FALSE;
return -1;
}
this->locks[kind]->write_lock(this->locks[kind]);
this->ctxids[kind][idx] = false;
if (this->ctxids[kind][idx] > 0)
{
refcount = --this->ctxids[kind][idx];
}
this->locks[kind]->unlock(this->locks[kind]);
return TRUE;
return refcount;
}
@@ -147,6 +178,7 @@ tkm_id_manager_t *tkm_id_manager_create(const tkm_limits_t limits)
INIT(this,
.public = {
.acquire_id = _acquire_id,
.acquire_ref = _acquire_ref,
.release_id = _release_id,
.destroy = _destroy,
},
@@ -155,7 +187,7 @@ tkm_id_manager_t *tkm_id_manager_create(const tkm_limits_t limits)
for (i = 0; i < TKM_CTX_MAX; i++)
{
this->limits[i] = limits[i];
this->ctxids[i] = calloc(limits[i], sizeof(bool));
this->ctxids[i] = calloc(limits[i], sizeof(int));
this->locks[i] = rwlock_create(RWLOCK_TYPE_DEFAULT);
DBG2(DBG_LIB, "%N initialized, %llu slot(s)", tkm_context_kind_names, i,
limits[i]);
src/charon-tkm/src/tkm/tkm_id_manager.h
+16 -4
View File
@@ -73,16 +73,28 @@ struct tkm_id_manager_t {
int (*acquire_id)(tkm_id_manager_t * const this,
const tkm_context_kind_t kind);
/**
* Acquire reference to given context id for a specific context kind.
*
* @param kind kind of context id
* @param ref_id id to acquire a reference for
* @return TRUE if reference could be acquired,
* FALSE otherwise
*/
bool (*acquire_ref)(tkm_id_manager_t * const this,
const tkm_context_kind_t kind,
const int ref_id);
/**
* Release a previously acquired context id.
*
* @param kind kind of context id to release
* @param id id to release
* @return TRUE if id was released, FALSE otherwise
* @return current refcount if id was released, -1 otherwise
*/
bool (*release_id)(tkm_id_manager_t * const this,
const tkm_context_kind_t kind,
const int id);
int (*release_id)(tkm_id_manager_t * const this,
const tkm_context_kind_t kind,
const int id);
/**
* Destroy a tkm_id_manager instance.
src/charon-tkm/src/tkm/tkm_kernel_ipsec.c
+8
View File
@@ -134,6 +134,12 @@ METHOD(kernel_ipsec_t, add_sa, status_t,
}
esa_id = tkm->idmgr->acquire_id(tkm->idmgr, TKM_CTX_ESA);
if (esa_id == 0)
{
DBG1(DBG_KNL, "unable to acquire esa context id");
goto esa_id_failure;
}
if (!tkm->sad->insert(tkm->sad, esa_id, data->reqid, local, peer,
spi_loc, spi_rem, id->proto))
{
@@ -193,9 +199,11 @@ METHOD(kernel_ipsec_t, add_sa, status_t,
return SUCCESS;
failure:
ike_esa_reset(esa_id);
tkm->sad->remove(tkm->sad, esa_id);
sad_failure:
tkm->idmgr->release_id(tkm->idmgr, TKM_CTX_ESA, esa_id);
esa_id_failure:
chunk_free(&esa.nonce_i);
chunk_free(&esa.nonce_r);
return FAILED;
src/charon-tkm/src/tkm/tkm_keymat.c
+11 -8
View File
@@ -279,8 +279,15 @@ METHOD(keymat_v2_t, derive_ike_keys, bool,
}
isa_info = *((isa_info_t *)(rekey_skd.ptr));
DBG1(DBG_IKE, "deriving IKE keys (parent_isa: %llu, ae: %llu, nc: %llu,"
"dh: %llu, spi_loc: %llx, spi_rem: %llx)", isa_info.parent_isa_id,
" dh: %llu, spi_loc: %llx, spi_rem: %llx)", isa_info.parent_isa_id,
isa_info.ae_id, nc_id, dh_id, spi_loc, spi_rem);
if (!tkm->idmgr->acquire_ref(tkm->idmgr, TKM_CTX_AE, isa_info.ae_id))
{
DBG1(DBG_IKE, "unable to acquire reference for ae: %llu",
isa_info.ae_id);
return FALSE;
}
this->ae_ctx_id = isa_info.ae_id;
res = ike_isa_create_child(this->isa_ctx_id, isa_info.parent_isa_id, 1,
dh_id, nc_id, nonce_rem, this->initiator,
@@ -416,11 +423,6 @@ METHOD(keymat_v2_t, get_skd, pseudo_random_function_t,
*skd = chunk_create((u_char *)isa_info, sizeof(isa_info_t));
/*
* remove ae context id, since control has now been handed over to the new
* IKE SA keymat
*/
this->ae_ctx_id = 0;
return PRF_HMAC_SHA2_512;
}
@@ -462,11 +464,12 @@ METHOD(keymat_t, destroy, void,
/* only reset ae context if set */
if (this->ae_ctx_id != 0)
{
if (ike_ae_reset(this->ae_ctx_id) != TKM_OK)
int count;
count = tkm->idmgr->release_id(tkm->idmgr, TKM_CTX_AE, this->ae_ctx_id);
if (count == 0 && ike_ae_reset(this->ae_ctx_id) != TKM_OK)
{
DBG1(DBG_IKE, "failed to reset AE context %d", this->ae_ctx_id);
}
tkm->idmgr->release_id(tkm->idmgr, TKM_CTX_AE, this->ae_ctx_id);
}
DESTROY_IF(this->hash_algorithms);
src/charon-tkm/tests/id_manager_tests.c
+82 -15
View File
@@ -84,21 +84,85 @@ START_TEST(test_acquire_id_same)
}
END_TEST
START_TEST(test_release_id)
START_TEST(test_acquire_ref)
{
int i, id = 0;
bool released = false;
bool acquired = false;
tkm_id_manager_t *idmgr = tkm_id_manager_create(limits);
for (i = 0; i < TKM_CTX_MAX; i++)
{
id = idmgr->acquire_id(idmgr, i);
released = idmgr->release_id(idmgr, i, id);
acquired = idmgr->acquire_ref(idmgr, i, id);
fail_unless(acquired, "Error acquiring reference context kind %d", i);
fail_unless(released, "Error releasing id of context kind %d", i);
/* Reset test variable */
acquired = false;
}
/* Reset released variable */
released = FALSE;
idmgr->destroy(idmgr);
}
END_TEST
START_TEST(test_acquire_ref_invalid_kind)
{
bool acquired;
tkm_id_manager_t *idmgr = tkm_id_manager_create(limits);
acquired = idmgr->acquire_ref(idmgr, TKM_CTX_MAX, 1);
fail_if(acquired, "Acquired reference for invalid context kind %d", TKM_CTX_MAX);
/* Reset test variable */
acquired = 0;
acquired = idmgr->acquire_ref(idmgr, -1, 1);
fail_if(acquired, "Acquired reference for invalid context kind %d", -1);
idmgr->destroy(idmgr);
}
END_TEST
START_TEST(test_acquire_ref_invalid_id)
{
int i;
bool acquired;
tkm_id_manager_t *idmgr = tkm_id_manager_create(limits);
for (i = 0; i < TKM_CTX_MAX; i++)
{
acquired = idmgr->acquire_ref(idmgr, i, -1);
fail_if(acquired,
"Acquired reference for negative id of context kind %d", i);
/* Reset test variable */
acquired = false;
acquired = idmgr->acquire_ref(idmgr, i, limits[i] + 1);
fail_if(acquired,
"Acquired reference exceeding limit of context kind %d", i);
/* Reset test variable */
acquired = false;
}
idmgr->destroy(idmgr);
}
END_TEST
START_TEST(test_release_id)
{
int i, count, id = 0;
tkm_id_manager_t *idmgr = tkm_id_manager_create(limits);
for (i = 0; i < TKM_CTX_MAX; i++)
{
id = idmgr->acquire_id(idmgr, i);
count = idmgr->release_id(idmgr, i, id);
fail_unless(count == 0, "Error releasing id of context kind %d", i);
/* Reset count variable */
count = 0;
}
idmgr->destroy(idmgr);
@@ -107,17 +171,17 @@ END_TEST
START_TEST(test_release_id_invalid_kind)
{
bool released = TRUE;
int count = 0;
tkm_id_manager_t *idmgr = tkm_id_manager_create(limits);
released = idmgr->release_id(idmgr, TKM_CTX_MAX, 1);
fail_if(released, "Released id for invalid context kind %d", TKM_CTX_MAX);
count = idmgr->release_id(idmgr, TKM_CTX_MAX, 1);
fail_if(count >= 0, "Released id for invalid context kind %d", TKM_CTX_MAX);
/* Reset test variable */
released = TRUE;
count = 0;
released = idmgr->release_id(idmgr, -1, 1);
fail_if(released, "Released id for invalid context kind %d", -1);
count = idmgr->release_id(idmgr, -1, 1);
fail_if(count >= 0, "Released id for invalid context kind %d", -1);
idmgr->destroy(idmgr);
}
@@ -125,11 +189,11 @@ END_TEST
START_TEST(test_release_id_nonexistent)
{
bool released = FALSE;
int count = 0;
tkm_id_manager_t *idmgr = tkm_id_manager_create(limits);
released = idmgr->release_id(idmgr, TKM_CTX_NONCE, 1);
fail_unless(released, "Release of nonexistent id failed");
count = idmgr->release_id(idmgr, TKM_CTX_NONCE, 1);
fail_unless(count == 0, "Release of nonexistent id failed");
idmgr->destroy(idmgr);
}
@@ -150,6 +214,9 @@ Suite *make_id_manager_tests()
tcase_add_test(tc, test_acquire_id);
tcase_add_test(tc, test_acquire_id_invalid_kind);
tcase_add_test(tc, test_acquire_id_same);
tcase_add_test(tc, test_acquire_ref);
tcase_add_test(tc, test_acquire_ref_invalid_kind);
tcase_add_test(tc, test_acquire_ref_invalid_id);
suite_add_tcase(s, tc);
tc = tcase_create("release");