Attention is currently required from: dexter, laforge, lynxis lazus, pespin. Hoernchen has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/42996?usp=email ) Change subject: Remote Card Procedure Framework ...................................................................... Patch Set 5: (19 comments) Patchset: PS5: so after a first quick look apparently the right now the untrusted card holder can: 1) fake every response and "drive" the rsp server due to no r-mac with scp02 / not implemented in pysim 2) read all scp02 responses (no r-enc in the spec) 3) alwayys read 00 class commands like verify adm 4) enumerate valid iccid/eids since these are client supplied and the error response differs 5) return sw != 9000 and the resulting crash leaks keys to the openobserve log 6) flood openobserve despite having a rate limiter, which will still forward... File contrib/rcp/rcp_client.py: https://gerrit.osmocom.org/c/pysim/+/42996/comment/4dad4954_7320540d?usp=ema... : PS5, Line 107: log.info(str(self) + " -- %s", rx_json['rcpc_instr']['print']) module/server-controlled print string logged raw (terminal escape injection) https://gerrit.osmocom.org/c/pysim/+/42996/comment/5db5253d_a9e3dc15?usp=ema... : PS5, Line 156: client = RcpcCltConnHdlr(sl, websocket, SERVER_TIMEOUT, json_validator) does this properly enforce wss:// and reject ws://? File contrib/rcp/rcp_module_utils.py: https://gerrit.osmocom.org/c/pysim/+/42996/comment/fd65d929_46e7b403?usp=ema... : PS5, Line 234: SrvSyncConnHdlr.__init__(self, *args, *kwargs) **kwargs ? File contrib/rcp/rcp_server.py: https://gerrit.osmocom.org/c/pysim/+/42996/comment/73968555_21179b78?usp=ema... : PS5, Line 77: option_parser.add_argument("--rcpc-request-limit", help="number of RCP Client requests per minute", missing type=int and therefore crash way way below. https://gerrit.osmocom.org/c/pysim/+/42996/comment/ed5b0344_1509fe2e?usp=ema... : PS5, Line 141: raise ValueError("command %s not found in command description %s" % (cmd_name, str(self.cmd_descr))) undefined cmd_name https://gerrit.osmocom.org/c/pysim/+/42996/comment/e566b6fc_53b467e9?usp=ema... : PS5, Line 219: if module_runtime_state.name == module_name: this matches the last suitable module so empty list -> UnboundLocalError on module_name https://gerrit.osmocom.org/c/pysim/+/42996/comment/15334064_a4ed95f6?usp=ema... : PS5, Line 397: await self._reset() cant we at least check that the reset atr equals the client supplied atr instead of discarding it? https://gerrit.osmocom.org/c/pysim/+/42996/comment/87386899_a9867da0?usp=ema... : PS5, Line 411: # Forward messages between RCP Module Command Server and RCP Client until the procedure ends. client imeout is per recv but this loop has no timeout at all? sounds like a problem... https://gerrit.osmocom.org/c/pysim/+/42996/comment/61288cac_04f1b4d5?usp=ema... : PS5, Line 479: Remove RCPM from internal list when the connection is closed (and the handler is deleted) the del depends on garbage collection timing which means this might live longer than intended... https://gerrit.osmocom.org/c/pysim/+/42996/comment/22100faf_f7127c8f?usp=ema... : PS5, Line 570: rc = requests.post(self.url, auth=self.service_auth, data=report_json) synchronous post with no timeout called from the async handler below sounds pretty bad because this stalls everything? https://gerrit.osmocom.org/c/pysim/+/42996/comment/64a5195a_5c0f1d86?usp=ema... : PS5, Line 586: if rate_limiter.limit(websocket.remote_address[0]): closes socket but does not return, falls through to create a recorder and post, which completely defeats the rate limit? https://gerrit.osmocom.org/c/pysim/+/42996/comment/ea297aa4_9e8ac192?usp=ema... : PS5, Line 607: await websocket.close(code=1011) # Internal Error if the iccid is not in the db this raises and returns the error -> lookup oracle. better to lookup garbage so it proceeds like a proper case and the client learns absolutely nothing. File contrib/rcp/rcp_utils.py: https://gerrit.osmocom.org/c/pysim/+/42996/comment/64515bbd_075fb235?usp=ema... : PS5, Line 76: raise ValueError("invalid type in command argument specification: %s" % arg['spec']['type']) undefined arg https://gerrit.osmocom.org/c/pysim/+/42996/comment/14ec7839_70922a7d?usp=ema... : PS5, Line 89: def load_server_cert(what: str, filename: str) -> SSLContext: no verify_mode/CA ? File contrib/rcp/rcpm_to_rcps_schema.json: https://gerrit.osmocom.org/c/pysim/+/42996/comment/d6aa8dfc_6b70bc74?usp=ema... : PS5, Line 68: "get_keys": { no restrictions on columns? File contrib/rcp/usage_example/rcp_module.py: https://gerrit.osmocom.org/c/pysim/+/42996/comment/3d437208_52ff8d8e?usp=ema... : PS5, Line 106: security_level = 3 very important to not pick unencrypted using this framework, ever... https://gerrit.osmocom.org/c/pysim/+/42996/comment/adedf6f3_e0cf3ae9?usp=ema... : PS5, Line 110: scp = SCP02(card_keys=kset) scp02 was deprecated in 2019 for a reason, which is the padding oracle. this whole rpcs is literally the example from the padding oracle paper, with the example "smartphone" replaced by this. another problem here for this is that scp02 supports 1) no r-enc 2) r-mac is optional (we should check this for our cards, GP get data tag 66) 3) pysim currently does not support scp02 r-mac, needs fixing. without r-mac a malicious client can basically return and therefore steer our server with any kind of garbage and a chosen status word without ever actually forwarding our commands to the card and we can't check anything. File docs/rcpf.rst: https://gerrit.osmocom.org/c/pysim/+/42996/comment/e504909c_e5bf9d11?usp=ema... : PS5, Line 74: `RCP Server` and the `RCP Modules`, all links use SSL/TLS encrypted channels. i guess here should be a big note with the network isolation requirements, not just implicitly by stating that the client is from the big bad internet. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/42996?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I8500a98492328ed5b3eb4bb177af2a4a8e481f80 Gerrit-Change-Number: 42996 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-CC: Hoernchen <ewild@sysmocom.de> Gerrit-CC: lynxis lazus <lynxis@fe80.eu> Gerrit-Attention: laforge <laforge@osmocom.org> Gerrit-Attention: pespin <pespin@sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis@fe80.eu> Gerrit-Attention: dexter <pmaier@sysmocom.de> Gerrit-Comment-Date: Tue, 13 Oct 2026 03:35:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No