Initial working implementation for security_seed (Issue #12)

[bhass1]

Added new security_seed functionality to UDS module. This function asks the UDS server for a Security Access seed repeatedly and records every seed received. It can be used to analyze the randomness of the UDS server's Security Access seed. Supported options include different "Security Access levels", different "Diagnostic Sessions", and performing different types of "ECU Resets" between seed requests. Results are printed to stdout.

[bhass1]

Addresses Issue #12

[kasperkarlsson]

Thank you very much for this contribution!

There are some issues which need to be resolved before this can be merged.

General:

• PEP 8 compliance; see e.g. https://pep8.org/ or https://www.python.org/dev/peps/pep-0008/ - some IDEs (e.g. PyCharm Community) even have built-in support for checking this.

ISO-14229-1: It would be preferable if changes here were kept to a minimum.

• I do not see the need for the is_valid_session function, which just checks a function argument in the UDS module. Maybe it could be skipped altogether?
• The comment change to after CAN-TP layer has been removed is redundant, as ISO-14229-1 works on another layer than the transport protocol.
• The diagnostic_session_control function looks great - keep as is! :+1:
• The default value None introduced for security_access_request_seed argument data_record is a correction, seeing as this is an optional parameter according to the specification. Great find! :+1:

UDS

• The retry logic in __security_seed_wrapper feels quite arbitrary. Since this is in a loop, just remove it and make sure to verify that a response was received before handling it (otherwise it will be retried in the next iteration anyway). A single retry followed by forced handling may end up failing anyway.
• Hard-coded sleep time after HARD_RESET: timing may vary depending on target ECU and reset type. I would suggest taking the sleep duration as an optional command line argument to cc.py uds security_seed instead and apply it regardless of reset type, with some sensible default value (e.g. 0.1 seconds).
• Typo: "secuirty"
• The print_negative_response help function showing the meaning of an NFC is great :+1:

I am looking forward to accepting this PR! If you would like assistance to take care of the issues mentioned above, just let me know :smiley:

[bhass1]

Sorry for the force push, I messed up my rebase earlier! :weary:

I haven't been able to test with a real CAN dongle yet, but I did some sanity checks and ran it through flake8... I will try to test with my two vehicles in the next week. :car: :car:

Thanks for the detailed feedback before!! It should all be addressed with the latest commits. Please let me know if you see any issues or want more changes.

[kasperkarlsson]

Great! I will review these new commits shortly :smiley:

[bhass1]

Tested on my vehicles. It works as intended, but I have to admit the user interface for the "sad" path is not the greatest. I'd like to make it more apparent for users when seeds aren't being collected. It'd also be nice to build in a flag for fuzzing various security access levels until valid seeds start popping out...

Nonetheless I was still able to use it to find some weak implementations in my car tonight, so it works.

[kasperkarlsson]

Thank you very much for your contribution, @bhass1! :smiley_cat: