I'm just now looking through osmo-bsc libfilter, and I would have expected a kind of top-down behavior like for firewalls:
imsi-deny .* imsi-allow 12345.* imsi-deny 1234567.* imsi-allow 123456789.* imsi-deny .*[13579]
i.e. start out denying everything. Except allow IMSIs starting with 12345. Except deny those that continue with 67. Unless they continue with 6789, allow those. Finally only service even-numbered IMSIs here.
That would be quite powerful.
Instead, what I see is that first, we go through the entire access list regarding only the allow-imsi entries. If any one of them matches, break the loop and return acceptance *right away*.
If no accept rule matched, go through the list again, and see if any deny rule matches. If there is one, break and return failure right away.
This was for the "local" per-msc rules, we also have a "global" rule set on the bsc level, which follows after that. For that, we currently actually completely ignore all allow rules, and only return error if any deny rule matches.
In the end, if nothing matched, return acceptance.
A code comment says that for the "global" rules, we want to deny first and accept second, so precisely reverse from the "local" level, but as a user I would find that rather confusing. See auth_imsi() in bsc_msg_filter.c.
So in short, to get the intended behavior outlined above, you are forced to cram all rules into a single regex, sort of like
imsi-allow 12345(<not>67|6789).*[02468] imsi-deny .*
which I don't actually know how to express as you can see... or maybe
imsi-allow 12345[^6].*[02468] imsi-allow 123456[^7].*[02468] imsi-deny .*
My questions are:
- Is it implemented this way for performance considerations?
- Are we not going to change this, even assuming that we regard changes being useful, because we don't want to break with previous configs?
- When looking in auth_imsi(), the access list names are: auth_imsi() name | comments | osmo-bsc passes bsc_lst "local" vty 'msc' level access rules nat_lst "global" vty 'bsc' level access rules Needless to say that "local", "global", "nat", "bsc" and "msc" are quite hard to follow. From the osmo-bsc filter code's perspective, there may be multiple MSCs configured, and per-msc config could be seen as more specific ("local") than bsc level, which applies to all MSCs ("global")... Was that the intention or is the local/global actually swapped by accident? (I notice that this feature seems to be intended for osmo-bsc-nat, and there's also a concept of a super-global blacklist that is always NULL in osmo-bsc.)
Here's a patch to add imsi-allow evaluation on the "global" level; other than the comment says, I use the same order of 'allow' first and 'deny' second to be consistent: https://gerrit.osmocom.org/4750
Otherwise I believe this could benefit from some refactoring, but probably we don't have time/funding to care about that, do we...
~N
On 9. Nov 2017, at 12:40, Neels Hofmeyr nhofmeyr@sysmocom.de wrote:
My questions are:
- Is it implemented this way for performance considerations?
The performance consideration doesn't have anything to do with the order of matches. Not to use longest matching rule might be considered a performance consideration (both at runtime and implementation).
Because there is no longest match a rule set (think of it in terms of first and then second rule) can not work.
imsi-deny "all" imsi-allow "neels"
In hindsight what would have been better is to define a policy "deny/allow" per access-list and then people can build opt-in or opt-out
(allow every one but neels):
access-list foo default allow access-list foo imsi-deny "neels"
(deny everone one but neels): access-list foo default deny access-list foo imsi-allow "neels"
Usecases where something like this. In this group only XYZ country IMSIs are allowed. But if you operate this as a testcell allow your own IMSI as well. Or for test cells.. No local IMSIs but allow my local IMSI.
- Are we not going to change this, even assuming that we regard changes being
useful, because we don't want to break with previous configs?
I think any existing user of osmo-bsc already will already have difficulties to upgrade, changing the access lists might not make a big difference?
- When looking in auth_imsi(), the access list names are: auth_imsi() name | comments | osmo-bsc passes bsc_lst "local" vty 'msc' level access rules nat_lst "global" vty 'bsc' level access rules
Needless to say that "local", "global", "nat", "bsc" and "msc" are quite hard to follow.
Sorry about that. I haven't looked at the code but it started in the NAT and then was generalized to be used in the BSC as well.
From the osmo-bsc filter code's perspective, there may be multiple MSCs configured, and per-msc config could be seen as more specific ("local") than bsc level, which applies to all MSCs ("global")... Was that the intention or is the local/global actually swapped by accident? (I notice that this feature seems to be intended for osmo-bsc-nat, and there's also a concept of a super-global blacklist that is always NULL in osmo-bsc.)
super-global? Hehe. In terms of terminology and system. Upstream for the BSC is a MSC (or bsc-nat that emulates the A-link of a MSC) and the upstream for the bsc-nat is a real MSC (or another nat). Local depends where you are so just saying "BSC" might not be that clear either?
holger
On Thu, Nov 09, 2017 at 02:39:51PM +0800, Holger Freyther wrote:
- Are we not going to change this, even assuming that we regard changes being
useful, because we don't want to break with previous configs?
I think any existing user of osmo-bsc already will already have difficulties to upgrade, changing the access lists might not make a big difference?
I guess if we implement a change, it should be a VTY switch that is off by default.
Meaning that we might as well wait until someone needs this and is willing to fund that development. Thinking about it again, the current state of the code is actually powerful enough for filtering IMSIs.
~N
-
Holger Freyther -
Neels Hofmeyr