Skip to content

Commit

Permalink
Merge pull request #262 from paullouisageneau/fix-check-transaction-i…
Browse files Browse the repository at this point in the history
…d-reuse

Fix reuse of transaction ID in different STUN binding request
  • Loading branch information
paullouisageneau authored Jul 22, 2024
2 parents 93351f1 + eda1cbf commit 1284352
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
24 changes: 21 additions & 3 deletions src/agent.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ int agent_resolve_servers(juice_agent_t *agent) {
turn_server->username);
entry->turn->password = turn_server->password;
juice_random(entry->transaction_id, STUN_TRANSACTION_ID_SIZE);
entry->transaction_id_expired = false;
++agent->entries_count;

agent_arm_transmission(agent, entry, STUN_PACING_TIME * i);
Expand Down Expand Up @@ -431,6 +432,7 @@ int agent_resolve_servers(juice_agent_t *agent) {
entry->pair = NULL;
entry->record = records[i];
juice_random(entry->transaction_id, STUN_TRANSACTION_ID_SIZE);
entry->transaction_id_expired = false;
++agent->entries_count;

agent_arm_transmission(agent, entry, STUN_PACING_TIME * i);
Expand Down Expand Up @@ -832,6 +834,10 @@ int agent_bookkeeping(juice_agent_t *agent, timestamp_t *next_timestamp) {
record_str, entry->retransmissions,
entry->retransmissions >= 2 ? "s" : "");
}
if (entry->transaction_id_expired) {
juice_random(entry->transaction_id, STUN_TRANSACTION_ID_SIZE);
entry->transaction_id_expired = false;
}
int ret;
switch (entry->type) {
case AGENT_STUN_ENTRY_TYPE_RELAY:
Expand Down Expand Up @@ -900,6 +906,7 @@ int agent_bookkeeping(juice_agent_t *agent, timestamp_t *next_timestamp) {
JLOG_DEBUG("STUN entry %d: Sending keepalive", i);

juice_random(entry->transaction_id, STUN_TRANSACTION_ID_SIZE);
entry->transaction_id_expired = false;

int ret;
switch (entry->type) {
Expand Down Expand Up @@ -1087,6 +1094,7 @@ int agent_bookkeeping(juice_agent_t *agent, timestamp_t *next_timestamp) {
if (entry->pair && entry->pair == selected_pair) {
entry->state =
AGENT_STUN_ENTRY_STATE_PENDING; // we don't want keepalives
entry->transaction_id_expired = true; // this is a new request
agent_arm_transmission(agent, entry, 0); // transmit now
break;
}
Expand Down Expand Up @@ -1246,7 +1254,7 @@ int agent_dispatch_stun(juice_agent_t *agent, void *buf, size_t size, stun_messa
JLOG_VERBOSE("STUN message is a response, looking for transaction ID");
entry = agent_find_entry_from_transaction_id(agent, msg->transaction_id);
if (!entry) {
JLOG_WARN("No STUN entry matching transaction ID, ignoring");
JLOG_DEBUG("No STUN entry matching transaction ID, ignoring");
return -1;
}
} else {
Expand Down Expand Up @@ -1506,9 +1514,9 @@ int agent_process_stun_binding(juice_agent_t *agent, const stun_message_t *msg,
: "controlling");
agent->mode = entry->mode == AGENT_MODE_CONTROLLING ? AGENT_MODE_CONTROLLED
: AGENT_MODE_CONTROLLING;
agent_update_candidate_pairs(agent);

juice_random(&agent->ice_tiebreaker, sizeof(agent->ice_tiebreaker));
agent_update_candidate_pairs(agent); // expires transaction IDs

if (entry->state != AGENT_STUN_ENTRY_STATE_IDLE) { // Check might not be started
entry->state = AGENT_STUN_ENTRY_STATE_PENDING;
agent_arm_transmission(agent, entry, 0);
Expand Down Expand Up @@ -2304,6 +2312,7 @@ int agent_add_candidate_pair(juice_agent_t *agent, ice_candidate_t *local, // lo
entry->record = pos->remote->resolved;
entry->relay_entry = relay_entry;
juice_random(entry->transaction_id, STUN_TRANSACTION_ID_SIZE);
entry->transaction_id_expired = false;
++agent->entries_count;

if (remote->type == ICE_CANDIDATE_TYPE_HOST)
Expand Down Expand Up @@ -2395,6 +2404,7 @@ void agent_arm_keepalive(juice_agent_t *agent, agent_stun_entry_t *entry) {
break;
}

entry->transaction_id_expired = true;
agent_arm_transmission(agent, entry, period);
}

Expand Down Expand Up @@ -2472,6 +2482,14 @@ void agent_update_candidate_pairs(juice_agent_t *agent) {
ice_update_candidate_pair(pair, is_controlling);
}
agent_update_ordered_pairs(agent);

// Expire all transaction IDs for checks
for (int i = 0; i < agent->entries_count; ++i) {
agent_stun_entry_t *entry = agent->entries + i;
if (entry->type == AGENT_STUN_ENTRY_TYPE_CHECK) {
entry->transaction_id_expired = true;
}
}
}

void agent_update_ordered_pairs(juice_agent_t *agent) {
Expand Down
1 change: 1 addition & 0 deletions src/agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ typedef struct agent_stun_entry {
timestamp_t next_transmission;
timediff_t retransmission_timeout;
int retransmissions;
bool transaction_id_expired;

// TURN
agent_turn_state_t *turn;
Expand Down

0 comments on commit 1284352

Please sign in to comment.