Attention is currently required from: laforge. wbokslag has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-tetra/+/33943 ) Change subject: Added first steps towards tetra crypto support ...................................................................... Patch Set 4: (9 comments) Patchset: PS4: Thanks for the remarks, I simplified some stuff, and am waiting for your opinion on merging tetra_crypto_state into tetra_mac_state while keeping tetra_crypto_database a global. File src/crypto/tetra_crypto.h: https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/a653f2ce_7e63b875 PS3, Line 47: mcc ETSI EN 300 392-1 V1.4.1 Clause 7.2.5 states: The MNC is coded in binary using 14 bits. The range of MNC shall be limited to four decimal digits i.e. the maximum value is 9 999 decimal or 270F in hexadecimal format. This does not in any way seem to be a character based representation so I think we're good here. https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/01b4abdd_13b0be75 PS3, Line 54: int is_gssi_range; : int start; : int end; Since I'm not really using address ranges I've decided to remove them from this patch. We can re-introduce those when needed. https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/82e93cb4_6ba25440 PS3, Line 61: mnc You're right, I changed them to uint32_t in the network info struct https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/7c6b69cd_98946b5a PS3, Line 71: int num_keys; Done https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/07b52ca7_952ac2f5 PS3, Line 93: const char *tetra_get_key_type_name(int key_type); Done File src/crypto/tetra_crypto.c: https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/9b060a51_c0b8d002 PS3, Line 14: struct tet I introduce two new global variables. How would you feel about merging the tetra_crypto_state into the tetra_mac_state? Since the MAC handles encryption, it would not be out of place. The second question then remains the tetra_crypto_database which serves as a database that stores user-supplied network specifications and key material. I think this neither fits in the tetra_mac_state, nor does it seem appropriate to pass it around through all the functions that are involved. To me, it seems okay to make the tetra_crypto_database a global since it is reasonable to assume one would want to use it across multiple tetra_mac_states. Agree? https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/92efdc1d_85f4d289 PS3, Line 82: calloc Done https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/b5ee42d5_5893be8a PS3, Line 96: k->mcc, k->mnc, k->key_type); I'll leave it as-is for now -- To view, visit https://gerrit.osmocom.org/c/osmo-tetra/+/33943 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-tetra Gerrit-Branch: master Gerrit-Change-Id: I63bc712630ae5dbaa049c129d456f7aef5bda863 Gerrit-Change-Number: 33943 Gerrit-PatchSet: 4 Gerrit-Owner: wbokslag <w.bokslag(a)midnightblue.nl> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 27 Jul 2023 16:16:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment