Attention is currently required from: laforge, pespin, dexter. Christian Amsüss has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/29033 ) Change subject: Add new pySim.ota library, implement SIM OTA crypto ...................................................................... Patch Set 5: (3 comments) File pySim/ota.py: https://gerrit.osmocom.org/c/pysim/+/29033/comment/6ebf4152_00f338c4 PS5, Line 347: apdu += otak.crypt._get_padding(len_cipher, otak.crypt.blocksize) This should also set pad_cnt. Setting PCNT=0 often works in practice because the last intentional command produces output so the padding zeros aren't even read, but still that's probably not intended. I'm not sure what the right behavior is with Gerrit here -- I've prepared a fix in the branch chrysn/for-29033 (but didn't push it to refs/change/29033 as that might create a new patchset rather than a proposed patch set if that's even a thing here). https://gerrit.osmocom.org/c/pysim/+/29033/comment/b3a60db4_7c7cfb9c PS5, Line 416: res = self.SmsResponsePacket.parse(remainder) Do we trust this parsing step enought, to * not raise anything even when run on the encrypted data in the por_shall_be_cipherd case? (If not, it could go into an `else` branch of the next line.) * to be run before the CC is evaluated? (It's not like we're doing *much* processing yet, but I have a weak personal preference to look at as little data from networks as possible before I've verified it's from a known somewhat-trusted peer.) (If both are "yes", please just mark as resolved). https://gerrit.osmocom.org/c/pysim/+/29033/comment/46ee5d21_fe577b61 PS5, Line 443: cc = otak.auth.check_sig(temp_data, res['cc_rc']) Needless assign; check_sig is merely called for the exception it'd raise. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/29033 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I193ff4712c8503279c017b4b1324f0c3d38b9f84 Gerrit-Change-Number: 29033 Gerrit-PatchSet: 5 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Christian Amsüss <chrysn(a)fsfe.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 19 Aug 2022 19:09:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment