Update API for SIP. Allow setting outbound number.

[dennwc]

• Add update API for SIP Trunks and Dispatch Rules.
• Allow setting custom outbound number in CreateSIPParticipant.
• Relax Inbound Trunk validation: allow setting zero numbers for wildcard Trunk.
• Fix typo in SIPUri: port should be an integer.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: c0c5445756c2f1b8121ba6f9e11a694612440255

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR

``` Some errors occurred when validating the changesets config: The package or glob expression "github.com/livekit/protocol" specified in the `fixed` option does not match any package in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch. ```

[dennwc]

@biglittlebigben Re: wildcard trunks, initially they were allowed in OSS, but disallowed in Cloud. But actually, Cloud doesn't need to know about inbound numbers upfront either. So they will now work consistently in both LK versions.

[biglittlebigben]

@biglittlebigben Re: wildcard trunks, initially they were allowed in OSS, but disallowed in Cloud. But actually, Cloud doesn't need to know about inbound numbers upfront either. So they will now work consistently in both LK versions.

Do we have a mechanism anywhere (in cloud) that prevents creating trunks that will allow any call in from anywhere?

[dennwc]

The only mechanism as this Validate method. Now we started using it in CLI and it prevents creating inbound trunks for OSS as well.

But otherwise, Cloud relies on SIP URI to tell which project the call belongs to. It doesn't need the number for a successful call dispatch.

[biglittlebigben]

The only mechanism as this Validate method. Now we started using it in CLI and it prevents creating inbound trunks for OSS as well.

But otherwise, Cloud relies on SIP URI to tell which project the call belongs to. It doesn't need the number for a successful call dispatch.

If I understand the code above properly, Validate on the trunk makes sure the headers are valid, and that the rule is not nil on DispatchRuleInfo.

Do we have a way to prevent customers to get surprise bills, or spam calls because they created an inbound trunk with no access control at all?

[dennwc]

First, for the context, we are talking about more sophisticated spam. Regular spam bots do not know the expected format for the LK SIP URI, so they will be filtered out immediately. The ones that remain have knowledge about LK's URI format. And I assume they know the correct project ID.

With that knowledge, number filtering won't do much - an attacker can fake it as well. So you are right, in the long term, we may want to require either CIDR filter or authentication on the trunks. But that would need to be a different protocol change with a proper notice.

[biglittlebigben]

We have heard of users who think got caught by such an abuse scenario. My experience with doing a lot of spam bot/infra abuse mitigation is that as soon as a bot maker notices you exist, they will cater the requests to the weaknesses of your access control.

One can argue that this Validate function is not the right place to do such validation, and we should have cloud specific, stricter enforcement, as OSS users may have a good reason for wildcard trunks/dispatches on private networks. I however don't think we can ship a relaxation of these rules on cloud.

[dennwc]

OK, then I propose the following: require that either of 3 fields is set: numbers, auth_* or CIDR. This is a step toward better trunk security.

We can then send a notice that going forward, we will require one of two fields: auth_* or CIDR. Number filter doesn't give meaningful protection.

[biglittlebigben]

OK, then I propose the following: require that either of 3 fields is set: numbers, auth_* or CIDR. This is a step toward better trunk security.

We can then send a notice that going forward, we will require one of two fields: auth_* or CIDR. Number filter doesn't give meaningful protection.

Sure! Folks who really want a wildcard trunk can use a 0.0.0.0/0 CIDR, but at least they opted into the trouble they're getting themselves into.

[dennwc]

Done, added trunk validation as discussed.

[dennwc]

Added a separate oneof message for per-field update, plus the helpers to apply these updates. PTAL.

[biglittlebigben]

This PR still has both add/remove/set semantics, both at the top level (oneof for the payload type) and at the individual fields add/remove/set fields. So far, the pattern has been to only have add/remove_ semantics for updates. The set semantics can be convenient, but I'm not sure they warrant the extra API complexity. Curious about what other folks think.

[dennwc]

This still needs a bit more work on the Cloud side. I split parts unrelated to update API into https://github.com/livekit/protocol/pull/890 and https://github.com/livekit/protocol/pull/891 to not block them.