Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sip: fix TCP source port #921

Merged
merged 2 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/re_sip.h
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ int sip_dialog_update(struct sip_dialog *dlg, const struct sip_msg *msg);
bool sip_dialog_rseq_valid(struct sip_dialog *dlg, const struct sip_msg *msg);
const char *sip_dialog_callid(const struct sip_dialog *dlg);
int sip_dialog_set_callid(struct sip_dialog *dlg, const char *callid);
void sip_dialog_set_srcport(struct sip_dialog *dlg, uint16_t srcport);
uint16_t sip_dialog_srcport(struct sip_dialog *dlg);
const char *sip_dialog_uri(const struct sip_dialog *dlg);
uint32_t sip_dialog_lseq(const struct sip_dialog *dlg);
enum sip_transp sip_dialog_tp(const struct sip_dialog *dlg);
Expand Down
32 changes: 32 additions & 0 deletions src/sip/dialog.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ struct sip_dialog {
uint32_t rseq;
size_t cpos;
enum sip_transp tp;
uint32_t srcport;
};


Expand Down Expand Up @@ -578,6 +579,37 @@ int sip_dialog_set_callid(struct sip_dialog *dlg, const char *callid)
}


/**
* Set TCP source port for a SIP Dialog
*
* @param dlg SIP Dialog
* @param srcport The TCP source port to be used
*/
void sip_dialog_set_srcport(struct sip_dialog *dlg, uint16_t srcport)
{
if (!dlg)
return;

dlg->srcport = srcport;
}


/**
* Get TCP source port for a SIP Dialog
*
* @param dlg SIP Dialog
*
* @return TCP source port
*/
uint16_t sip_dialog_srcport(struct sip_dialog *dlg)
{
if (!dlg)
return 0;

return dlg->srcport;
}


/**
* Get the local sequence number from a SIP Dialog
*
Expand Down
7 changes: 7 additions & 0 deletions src/sip/request.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ struct sip_request {
bool canceled;
bool provrecv;
uint16_t port;
uint16_t srcport;
};


Expand Down Expand Up @@ -224,6 +225,11 @@ static int request(struct sip_request *req, enum sip_transp tp,
if (!req->branch || !mb)
goto out;

if (req->srcport) {
struct sip_conncfg cfg = {.srcport = req->srcport};
err = sip_conncfg_set(req->sip, dst, &cfg);
}

if (!req->stateful) {
err = sip_send_conn(req->sip, NULL, tp, dst, mb,
connect_handler, req);
Expand Down Expand Up @@ -940,6 +946,7 @@ int sip_drequestf(struct sip_request **reqp, struct sip *sip, bool stateful,
goto out;

req->reqp = reqp;
req->srcport = sip_dialog_srcport(dlg);
err = sip_request_send(req, sip, sip_dialog_route(dlg));

out:
Expand Down
13 changes: 4 additions & 9 deletions src/sipreg/reg.c
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ static int send_handler(enum sip_transp tp, struct sa *src,
int err;

(void)contp;
(void)dst;

reg->tp = tp;
if (reg->srcport && tp != SIP_TRANSP_UDP)
Expand All @@ -313,14 +314,6 @@ static int send_handler(enum sip_transp tp, struct sa *src,
err |= mbuf_printf(mb, ";reg-id=%d", reg->regid);

err |= mbuf_printf(mb, "\r\n");

if (reg->srcport) {
struct sip_conncfg cfg;
memset(&cfg, 0, sizeof(cfg));
cfg.srcport = reg->srcport;
err = sip_conncfg_set(reg->sip, dst, &cfg);
}

return err;
}

Expand Down Expand Up @@ -618,10 +611,12 @@ int sipreg_set_fbregint(struct sipreg *reg, uint32_t fbregint)
*/
void sipreg_set_srcport(struct sipreg *reg, uint16_t srcport)
{
if (!reg)
if (!reg || !reg->dlg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is setting the srcport only relevant for dlg? Or is there a use-case without it too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently not. We could add a setter void sip_request_set_srcport(...).

For now I couldn't find any call to plain sip_request() in re or baresip. And no call to sip_request_alloc() followed by sip_request_send().

For sipreg I think this is a correct condition, because it always has a dialog.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw that sip_request_alloc() is not in the API. So a setter sip_request_set_srcport() doesn't make sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a bit misleading, I think the check !reg->dlg is not needed since it's not relevant for setting reg->srcport = srcport. As a caller of public sipreg_set_srcport I assume that srcport is set if I provide a valid sipreg object.

sip_dialog_set_srcport has a extra check for valid dlg, so this should fine.

return;

reg->srcport = srcport;

sip_dialog_set_srcport(reg->dlg, srcport);
}


Expand Down