CaringCaribou / caringcaribou

A friendly car security exploration tool for the CAN bus
GNU General Public License v3.0
751 stars 197 forks source link

only show relevant DID reads #97

Closed alexdetrano closed 10 months ago

alexdetrano commented 1 year ago

on a real CAN bus, sometimes other ECUs are also performing DID reads. This commit checks the DID value in the response against the requested DID value. If they do not match, the response is extraneous and is not shown to the user.

kasperkarlsson commented 1 year ago

Hi there! When I run the test suite, I get the following error:

$ cc.py -i vcan0 test

-------------------
CARING CARIBOU v0.4
-------------------

Loading module 'test'

Running tests using CAN interface 'vcan0'

test_create_iso_14229_1 (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_ecu_reset_failure_on_invalid_reset_type (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_ecu_reset_failure_suppress_positive_response (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_ecu_reset_success (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_ecu_reset_success_suppress_positive_response (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_read_data_by_identifier_failure (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_read_data_by_identifier_success (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_read_memory_by_address_failure_on_invalid_length (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_read_memory_by_address_success (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_write_data_by_identifier_failure (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_write_data_by_identifier_success (test_iso_14229_1.DiagnosticsOverIsoTpTestCase) ... ok
test_create_iso_15765_2 (test_iso_15765_2.IsoTpTestCase) ... ok
test_fail_too_long_message (test_iso_15765_2.IsoTpTestCase) ... ok
test_multi_frame_long_message (test_iso_15765_2.IsoTpTestCase) ... ok
test_multi_frame_two_frames (test_iso_15765_2.IsoTpTestCase) ... ok
test_single_frame (test_iso_15765_2.IsoTpTestCase) ... ok
test_dump_dids (test_module_uds.UdsModuleTestCase) ... FAIL
test_ecu_reset_hard_reset_success (test_module_uds.UdsModuleTestCase) ... ok
test_ecu_reset_unsupported_reset_type_failure (test_module_uds.UdsModuleTestCase) ... ok
test_security_access_request_seed_invalid_level (test_module_uds.UdsModuleTestCase) ... ok
test_security_access_request_seed_send_key_success (test_module_uds.UdsModuleTestCase) ... ok
test_service_discovery (test_module_uds.UdsModuleTestCase) ... ok
test_service_discovery_empty_range (test_module_uds.UdsModuleTestCase) ... ok
test_uds_discovery (test_module_uds.UdsModuleTestCase) ... ok
test_uds_discovery_blacklist (test_module_uds.UdsModuleTestCase) ... ok
test_parse_candump_line (test_send.SendFileParserTestCase) ... ok
test_parse_pythoncan_line_v_20 (test_send.SendFileParserTestCase) ... ok
test_parse_pythoncan_line_v_21 (test_send.SendFileParserTestCase) ... ok
test_parse_pythoncan_line_v_21_flags (test_send.SendFileParserTestCase) ... ok
test_parse_pythoncan_line_v_30_channel (test_send.SendFileParserTestCase) ... ok
test_parse_pythoncan_line_v_30_flags (test_send.SendFileParserTestCase) ... ok

======================================================================
FAIL: test_dump_dids (test_module_uds.UdsModuleTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/caringcaribou-0.4-py3.10.egg/caringcaribou/tests/test_module_uds.py", line 198, in test_dump_dids
    self.assertEqual(expected_response_cnt, len(responses))
AssertionError: 2 != 0

----------------------------------------------------------------------
Ran 31 tests in 1.110s

FAILED (failures=1)

It seems like this filtering causes the test to fail (which might be completely reasonable given that the test was not written with such a filter in mind). Do you see any reasonable, simple way of fixing this as part of the PR?

kasperkarlsson commented 10 months ago

Hi again! Just checking in on old PRs, do you have any thoughts on the issue regarding the broken test?

alexdetrano commented 10 months ago

I will have to look into it. Sorry I missed this email!

On Tue, Jan 16, 2024, 6:18 AM Kasper Karlsson @.***> wrote:

Hi again! Just checking in on old PRs, do you have any thoughts on the issue regarding the broken test?

— Reply to this email directly, view it on GitHub https://github.com/CaringCaribou/caringcaribou/pull/97#issuecomment-1893542301, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKE4IM2FRFGZ4VTBKWMB53YOZOWRAVCNFSM6AAAAAA63GHAACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJTGU2DEMZQGE . You are receiving this because you authored the thread.Message ID: @.***>

alexdetrano commented 10 months ago

I see what's happening. The mock ECU is not responding to DIDs according to the UDS spec. If you look at the test response below for reading DID 0001

 vcan0  0000200C   [8]  03 22 00 01 00 00 00 00   '."......'
 vcan0  0000200D   [8]  02 62 72 00 00 00 00 00   '.br.....'

you can see the response is 62 72, in other words 62 <data>

but a real DID response should look like 62 <data_identifier> <data>

so the total CAN frame response should look like

04 62 00 01 72 00 00 00

the problem is in caringcaribou/tests/mock/mock_ecu_uds.py

I'll delete this PR and re-submit with an updated test edu, since this commit will conflict with the latest where we check for the length of the response before indexing.