osmo-bsc access lists semantics are confusing

[Neels Hofmeyr]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20171109/b780d641/attachment.bin>