osmo-msc/3da4715f718dc8b695244ef4d93...

198 lines
7.1 KiB
Plaintext

{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "344f85e7_d4a0d87a",
"filename": "/COMMIT_MSG",
"patchSetId": 1
},
"lineNbr": 15,
"author": {
"id": 1000074
},
"writtenOn": "2024-03-26T10:26:25Z",
"side": 1,
"message": "we do the same in osmo-pcu with tbfs btw (ms_merge_and_clear_ms()).",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "ee0aa82e_04e646a3",
"filename": "/COMMIT_MSG",
"patchSetId": 1
},
"lineNbr": 15,
"author": {
"id": 1000005
},
"writtenOn": "2024-03-26T14:13:59Z",
"side": 1,
"message": "interesting, which way around is that? do we also drop the old one there?",
"parentUuid": "344f85e7_d4a0d87a",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "d1718f8a_e55af610",
"filename": "/COMMIT_MSG",
"patchSetId": 1
},
"lineNbr": 15,
"author": {
"id": 1000074
},
"writtenOn": "2024-03-26T14:24:56Z",
"side": 1,
"message": "Yes, definetly drop the old one. In osmo-pcu is even more complex because we first lookup by TBF, and then the assigned MS object, but sometimes the MS object has non-yet-known-imsi. So at some point, when figuring out the IMSI, we figure out we have a duplicated MS object and we need to move gained information from the previous MS object and then drop that MS object plus its related TBFs.",
"parentUuid": "ee0aa82e_04e646a3",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "9ce0e0e7_d2cfa366",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 1
},
"lineNbr": 0,
"author": {
"id": 1000004
},
"writtenOn": "2024-03-26T08:20:03Z",
"side": 1,
"message": "are we sure this code path only gets triggered after authentication [assuming authentication is enabled]? I just want to prevent any attacker spoofing some IMSI and then the MSC/VLR would evict the old (valid) subscriber at the expense of the new (invalid, spoofed) one.",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "4990e4f3_0617c662",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 1
},
"lineNbr": 0,
"author": {
"id": 1000005
},
"writtenOn": "2024-03-26T14:13:59Z",
"side": 1,
"message": "I thought about that, too. The situation is:\n\nThis happens before Auth:\n- the MSC gets a LU request by TMSI,\n- then requires the IMSI\n- the ID response for that triggers this de-duplication.\n- authentication follows.\n- we can copy auth tuples safely.\n- i am not copying state like the security status or FSM states that might skip important validations.\n- we always have to know a subscriber\u0027s IMSI, i.e. the code makes sure that an IMSI is known in order to contact the HLR. This triggers only in a situation where the IMSI is not yet known, i.e. only possible in the short phase before the ID Request, before auth. i.e. not possible to sneakily hijack another IMSI\u0027s security context.\n\nIf authentication is switched off, then this doesn\u0027t hold of course, but nothing else does either.",
"parentUuid": "9ce0e0e7_d2cfa366",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "16cfd6af_fe5db9e1",
"filename": "src/libmsc/paging.c",
"patchSetId": 1
},
"lineNbr": 147,
"author": {
"id": 1000074
},
"writtenOn": "2024-03-26T10:26:25Z",
"side": 1,
"message": "Shouldn\u0027t this happen only \"if discarding_vsub-\u003ecs.is_paging\"?",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "0c177c27_f91a086d",
"filename": "src/libmsc/paging.c",
"patchSetId": 1
},
"lineNbr": 147,
"author": {
"id": 1000010
},
"writtenOn": "2024-03-26T10:41:18Z",
"side": 1,
"message": "We never reach this code if `discarding_vsub-\u003ecs.is_paging` is `false`, see line 129 above.",
"parentUuid": "16cfd6af_fe5db9e1",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "eec2c848_83fe2660",
"filename": "src/libmsc/paging.c",
"patchSetId": 1
},
"lineNbr": 147,
"author": {
"id": 1000074
},
"writtenOn": "2024-03-26T10:53:26Z",
"side": 1,
"message": "Ah indeed it missed that one.",
"parentUuid": "0c177c27_f91a086d",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "7e963ad9_991297fe",
"filename": "src/libmsc/paging.c",
"patchSetId": 1
},
"lineNbr": 147,
"author": {
"id": 1000005
},
"writtenOn": "2024-03-26T14:13:59Z",
"side": 1,
"message": "yes, i wanted to keep that close together, but the use count put should happen in the end, while the if() should happen in the start... i\u0027ll drop a comment there",
"parentUuid": "eec2c848_83fe2660",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "ca39f788_f7e3f283",
"filename": "src/libvlr/vlr.c",
"patchSetId": 1
},
"lineNbr": 597,
"author": {
"id": 1000074
},
"writtenOn": "2024-03-26T10:26:25Z",
"side": 1,
"message": "I\u0027d rpobably move all this code to its own function \"vsub_join()\" or alike.",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "e207ac85_0f270790",
"filename": "src/libvlr/vlr.c",
"patchSetId": 1
},
"lineNbr": 597,
"author": {
"id": 1000005
},
"writtenOn": "2024-03-26T14:13:59Z",
"side": 1,
"message": "ack\n(the paging_request_join() function was supposed to be that own function, but then this code here grew and grew with more aspects to be taken care of...)",
"parentUuid": "ca39f788_f7e3f283",
"revId": "3da4715f718dc8b695244ef4d9376f3724a9bb94",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
}
]
}