From 2ddcad3989cd8d2314453ed31ff43e122118663f Mon Sep 17 00:00:00 2001 From: Dimitri Staessens Date: Fri, 1 May 2026 23:41:49 +0200 Subject: irmd: Drop replayed flow alloc requests A duplicating link could deliver the same alloc request twice. OAP detected the replay but still replied over the wire, so the requester saw a second flow_alloc_reply on an already-allocated flow and reg_respond_alloc tripped its PENDING-state assertion. Add EREPLAY so the OAP server can signal replays distinctly; flow_accept drops them silently. As a safety net, reg_respond_alloc warn-drops late replies instead of asserting. Signed-off-by: Dimitri Staessens Signed-off-by: Sander Vrijders --- src/irmd/main.c | 7 ++++ src/irmd/oap/auth.c | 1 + src/irmd/oap/srv.c | 15 ++++++-- src/irmd/oap/tests/oap_test.c | 7 ++-- src/irmd/reg/reg.c | 6 +++- src/irmd/reg/tests/reg_test.c | 82 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 112 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/irmd/main.c b/src/irmd/main.c index a85a9bf0..5a6245e0 100644 --- a/src/irmd/main.c +++ b/src/irmd/main.c @@ -910,6 +910,10 @@ static int flow_accept(struct flow_info * flow, flow->uid = reg_get_proc_uid(flow->n_pid); err = oap_srv_process(&info, req_hdr, &resp_hdr, data, sk); + if (err == -EREPLAY) { + log_warn("Dropping replayed alloc request for %s.", name); + goto fail_replay; + } if (err < 0) { log_err("OAP processing failed for %s.", name); goto fail_oap; @@ -938,6 +942,9 @@ static int flow_accept(struct flow_info * flow, fail_oap: if (!reg_flow_is_direct(flow->id)) ipcp_flow_alloc_resp(flow, err, resp_hdr); + fail_replay: + freebuf(req_hdr); + freebuf(resp_hdr); fail_wait: reg_destroy_flow(flow->id); fail_flow: diff --git a/src/irmd/oap/auth.c b/src/irmd/oap/auth.c index 4b86f055..d165de73 100644 --- a/src/irmd/oap/auth.c +++ b/src/irmd/oap/auth.c @@ -174,6 +174,7 @@ int oap_check_hdr(const struct oap_hdr * hdr) fail_replay: pthread_mutex_unlock(&oap_auth.replay.mtx); free(new); + return -EREPLAY; fail_stamp: return -EAUTH; } diff --git a/src/irmd/oap/srv.c b/src/irmd/oap/srv.c index afc54acc..ce97654f 100644 --- a/src/irmd/oap/srv.c +++ b/src/irmd/oap/srv.c @@ -393,6 +393,7 @@ int oap_srv_process(const struct name_info * info, void * pkp = NULL; void * crt = NULL; int req_md_nid; + int ret; assert(info != NULL); assert(rsp_buf != NULL); @@ -427,8 +428,13 @@ int oap_srv_process(const struct name_info * info, id = peer_hdr.id.data; /* Logging */ - if (oap_check_hdr(&peer_hdr) < 0) { - log_err_id(id, "OAP header failed replay check."); + ret = oap_check_hdr(&peer_hdr); + if (ret == -EREPLAY) { + log_warn_id(id, "OAP header failed replay check."); + goto fail_replay; + } + if (ret < 0) { + log_err_id(id, "OAP header check failed."); goto fail_auth; } @@ -491,6 +497,11 @@ int oap_srv_process(const struct name_info * info, fail_cred: return -EAUTH; + fail_replay: + crypt_free_crt(crt); + crypt_free_key(pkp); + return -EREPLAY; + fail_kex: crypt_free_crt(crt); crypt_free_key(pkp); diff --git a/src/irmd/oap/tests/oap_test.c b/src/irmd/oap/tests/oap_test.c index a324b586..a525d988 100644 --- a/src/irmd/oap/tests/oap_test.c +++ b/src/irmd/oap/tests/oap_test.c @@ -32,6 +32,7 @@ #include #include +#include #include #include #include @@ -1053,9 +1054,9 @@ static int test_oap_replay_packet(void) freebuf(ctx.req_hdr); ctx.req_hdr = saved_req; - /* Replayed request should fail */ - if (oap_srv_process_ctx(&ctx) == 0) { - printf("Server should reject replayed packet.\n"); + /* Replay must return -EREPLAY so callers can drop silently. */ + if (oap_srv_process_ctx(&ctx) != -EREPLAY) { + printf("Replayed packet rejection != -EREPLAY.\n"); goto fail_cleanup; } diff --git a/src/irmd/reg/reg.c b/src/irmd/reg/reg.c index 0025f695..365064e5 100644 --- a/src/irmd/reg/reg.c +++ b/src/irmd/reg/reg.c @@ -1820,7 +1820,11 @@ int reg_respond_alloc(struct flow_info * info, goto fail_flow; } - assert(flow->info.state == FLOW_ALLOC_PENDING); + if (flow->info.state != FLOW_ALLOC_PENDING) { + log_warn("Flow %d already responded.", info->id); + goto fail_flow; + } + assert(flow->rsp_data.len == 0); assert(flow->rsp_data.data == NULL); diff --git a/src/irmd/reg/tests/reg_test.c b/src/irmd/reg/tests/reg_test.c index f4b0188b..5a5178c2 100644 --- a/src/irmd/reg/tests/reg_test.c +++ b/src/irmd/reg/tests/reg_test.c @@ -489,6 +489,87 @@ static int test_reg_allocate_flow_fail(void) return TEST_RC_FAIL; } +static int test_reg_respond_alloc_duplicate(void) +{ + pthread_t thr; + struct timespec abstime; + struct timespec timeo = TIMESPEC_INIT_S(1); + buffer_t rbuf = BUF_INIT; + buffer_t empty = BUF_INIT; + struct flow_info dup_info; + + struct flow_info info = { + .n_pid = TEST_PID, + .qs = qos_raw + }; + + struct flow_info n_1_info = { + .n_1_pid = TEST_N_1_PID, + .qs = qos_data, + .state = FLOW_ALLOCATED /* RESPONSE SUCCESS */ + }; + + TEST_START(); + + clock_gettime(PTHREAD_COND_CLOCK, &abstime); + ts_add(&abstime, &timeo, &abstime); + + if (reg_init() < 0) { + printf("Failed to init registry.\n"); + goto fail; + } + + if (reg_create_flow(&info) < 0) { + printf("Failed to add flow.\n"); + goto fail; + } + + info.n_1_pid = TEST_N_1_PID; + + if (reg_prepare_flow_alloc(&info) < 0) { + printf("Failed to prepare flow for alloc.\n"); + goto fail; + } + + n_1_info.id = info.id; + n_1_info.mpl = 1; + + pthread_create(&thr, NULL, test_flow_respond_alloc, &n_1_info); + + if (reg_wait_flow_allocated(&info, &rbuf, &abstime) < 0) { + printf("Flow allocation failed.\n"); + pthread_join(thr, NULL); + reg_destroy_flow(info.id); + reg_fini(); + goto fail; + } + + pthread_join(thr, NULL); + freebuf(rbuf); + + /* Duplicate reply on an already-ALLOCATED flow must not assert. */ + dup_info = n_1_info; + dup_info.state = FLOW_DEALLOCATED; + + if (reg_respond_alloc(&dup_info, &empty, -EREPLAY) != -1) { + printf("Duplicate respond_alloc should return -1.\n"); + goto fail; + } + + reg_dealloc_flow(&info); + reg_dealloc_flow_resp(&info); + reg_destroy_flow(n_1_info.id); + + reg_fini(); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail: + REG_TEST_FAIL(); + return TEST_RC_FAIL; +} + struct direct_alloc_info { struct flow_info info; buffer_t rsp; @@ -679,6 +760,7 @@ static int test_reg_flow(void) { rc |= test_reg_accept_flow_success(); rc |= test_reg_accept_flow_success_no_crypt(); rc |= test_reg_allocate_flow_fail(); + rc |= test_reg_respond_alloc_duplicate(); rc |= test_reg_direct_flow_success(); return rc; -- cgit v1.2.3