Attention is currently required from: neels, pespin.
lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/43480?usp=email )
Change subject: libvlr: allow to use the same key slot for multiple ciphering commands ......................................................................
Patch Set 2:
(7 comments)
File src/libmsc/msc_vty.c:
https://gerrit.osmocom.org/c/osmo-msc/+/43480/comment/0d7b4ce1_3d885af8?usp=... : PS2, Line 536: gsmnet->vlr->cfg.ciph_sec_ctx_max_reuse = atoi(argv[0]);
This double assignment with on top super long lines and indentation looks confusing.
Done
File src/libvlr/vlr_access_req_fsm.c:
https://gerrit.osmocom.org/c/osmo-msc/+/43480/comment/81d17cfd_ad45a908?usp=... : PS2, Line 466: par->key_seq = par->vsub->last_tuple->key_seq;
lots of indirection here, may make sense to add some intermediate ASSERT?
i've added assert. It can't be zero when auth was successful.
File src/libvlr/vlr_auth_fsm.h:
https://gerrit.osmocom.org/c/osmo-msc/+/43480/comment/38b8e62c_0d32829d?usp=... : PS2, Line 43: bool auth_ciph_sec_ctx_is_usable(struct vlr_subscr *vsub, uint8_t key_seq);
const
Done
File src/libvlr/vlr_auth_fsm.c:
https://gerrit.osmocom.org/c/osmo-msc/+/43480/comment/0f6ef3e0_adf57b7f?usp=... : PS2, Line 697: bool auth_ciph_sec_ctx_is_usable(struct vlr_subscr *vsub, uint8_t key_seq)
const vsub. […]
Done
https://gerrit.osmocom.org/c/osmo-msc/+/43480/comment/d35914cf_4e817d73?usp=... : PS2, Line 702: if (!at || key_seq == 0x7)
so "at && key_seq == 0x7" is actually fine then? Or do you better need to split these cases?
(at && key_seq == 0x7) no that's not fine. but one of them is actually a false.
File tests/msc_vlr/msc_vlr_test_authen_reuse.c:
https://gerrit.osmocom.org/c/osmo-msc/+/43480/comment/b0d8a0fb_7b7396c7?usp=... : PS2, Line 40: net->vlr->cfg.auth_tuple_max_reuse_count = net->vlr->cfg.ciph_sec_ctx_max_reuse = set_max_reuse_count;
let's please have these in separate lines!
Done
File tests/msc_vlr/msc_vlr_tests.c:
https://gerrit.osmocom.org/c/osmo-msc/+/43480/comment/f27e3073_183dd6ad?usp=... : PS2, Line 681: net->vlr->cfg.auth_tuple_max_reuse_count = net->vlr->cfg.ciph_sec_ctx_max_reuse = 0;
separate lines!
My idea was to make sure it's coupled as long the MSC don't want to save auth tuple for ciphering.