Change in osmo-sgsn[master]: ACL: integrate sanitize check into sgsn_acl_* functions

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Max gerrit-no-reply at lists.osmocom.org
Thu Dec 13 10:49:18 UTC 2018


Max has posted comments on this change. ( https://gerrit.osmocom.org/12227 )

Change subject: ACL: integrate sanitize check into sgsn_acl_* functions
......................................................................


Patch Set 5:

> Patch Set 5:
> I did, maybe it was not clear enough: "I think sanitizing the imsi should be done by caller of sgsn_acl_* based on where the information come from (from the wire or from known sanitized source)."
> 

Hmm, I was sure I've addressed it already. Anyway, the "where the information come from" is not applicable because it only comes from a single source which is not trusted so we always sanitize it.

> If you know your data is sane there's no need to re-sanitize it.

That's not our case.

> You should expect the caller of a data struct to provide sane data instead of internally sanitizing it and storing different data from what was provided.

Sorry, you've lost me with "caller of a data struct" - what do you mean by that?

> It's responsibility of the caller (vty code for instance) to make sure parse of human input is correctly parsed and sanitized.

I disagree, and the code I've looked over seems to disagree as well.
For example, in osmo_bsc_vty.c:
* osmo_mcc_from_str() sanitize data internally
* gsm_parse_reg() regexp compiled and result checked outside of vty

In general, I don't see any non-trivial checks done inside vty which I think is the right thing. What would be the advantage of having this check in separate file instead of static function in the same file? We can also move it to libosmocore but I don't see any benefits from keeping it in vty. Do you?

> You can do checks inside the data structure if you want (I wouldn't), but I'd avoid changing content of the data being handled in there.

You mean inside function?

The rest would be addressed in a next revision.


-- 
To view, visit https://gerrit.osmocom.org/12227
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3dff108148683b107e9edac430a0475283580e9
Gerrit-Change-Number: 12227
Gerrit-PatchSet: 5
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-CC: Stefan Sperling <stsp at stsp.name>
Gerrit-Comment-Date: Thu, 13 Dec 2018 10:49:18 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181213/8314d60a/attachment.htm>


More information about the gerrit-log mailing list