search

pylessard / python-udsoncan

Python implementation of UDS (ISO-14229) standard.
MIT License
564 stars 195 forks source link

Security access SAPR response not correctly interpreted #214

Closed belliriccardo closed 5 months ago

belliriccardo commented 5 months ago

In short, if the client requests security access for a level that is already unlocked, when requesting the seed with seed = client.request_seed(...) the ecu will (correctly) respond with 0x67 0x00 0x00 0x00 0x00 (as in Security Access Positive Response and an empty seed; citing from the standard: [...] If a server supports security, but the requested security level is already unlocked when a SecurityAccess ‘requestSeed’ message is received, that server shall respond with a SecurityAccess ‘requestSeed’ positive response message service with a seed value equal to zero (0). ). However, this is not interpreted correctly inside SecurityAccess.interpret_response(...), as it raises an exception:

...
2024-02-20 11:11:45 [INFO] UdsClient: SecurityAccess<0x27> - Requesting seed to unlock security access level 0x01
2024-02-20 11:11:45 [DEBUG] Connection: Sending 6 bytes : [270100010203]
2024-02-20 11:11:45 [DEBUG] Connection: Received 5 bytes : [6700000000]
2024-02-20 11:11:45 [INFO] UdsClient: Received positive response for service SecurityAccess (0x27) from server.
2024-02-20 11:11:45 [ERROR] UdsClient: [UnexpectedResponseException] : SecurityAccess service execution returned a valid response but unexpected. Details : Response subfunction received from server (0x00) does not match the requested subfunction (0x01)
Server sent an invalid payload : b'g\x00\x00\x00\x00'
2024-02-20 11:13:10 [INFO] Connection: Connection closed

Is this an actual issue or am I missing something here? There might also be some problems with the uds server implementation, but I haven't found any yet (of relevance). Any insight is welcome!

pylessard commented 5 months ago

I think you are missing the fact that the 2nd byte must be the level echo even if the seed is empty. I think the lib is right and the server is wrong here.

You get: 6700000000 You should have: 670100000000

You can see that if you call unlock_security_access, this case is handled here: https://github.com/pylessard/python-udsoncan/blob/master/udsoncan/client.py#L345-L348

belliriccardo commented 5 months ago

Hi @pylessard, I've checked and that is indeed the case, there is a firmware bug on the ecu that doesn't format the response correctly. Also doesn't help that I've been "manually" using the request_seed, the call to the security function, the send_key, interpreting the response etc etc... if I had used unlock_security_access from the start I would've maybe catched this earlier and I wouldn't have wasted your time! Teached me to not reinvent the wheel.. thanks again, closing this issue

pylessard commented 5 months ago

You're not wasting my time. I've spent months writing this code with the sole purpose of helping find bugs in ECUs. Objective achieved here, I'm happy

You would have gotten a similar error with unlock_security_access.

Have a good day!