Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unintended HUGE refactoring of automotive scanners. #3054

Closed
wants to merge 1 commit into from

Conversation

polybassa
Copy link
Contributor

This PR:

  • changes variable names of automotive utilities to be more compliant
    with python
  • adds typing for various automotive utilities
  • refactors the core of automotive scanners which involve:
    • Ecu objects
    • EcuState objects
    • EcuAnsweringMachines
    • AutomotiveTestCases
    • AutomotiveTestCaseExecutors
  • cleans up UDS Scanner, OBD Scanner and GMLAN Scanner

Sorry that this PR became that big. I hope it is merge-able in some way.

@gpotter2 gpotter2 added the cleanup Performs some code clean-up label Jan 14, 2021
@polybassa polybassa force-pushed the scanner_refactoring branch 2 times, most recently from faadb5b to b2fd823 Compare January 17, 2021 09:38
@guedou
Copy link
Member

guedou commented Jan 19, 2021

I think that it is impossible to review as it is as it introduces many changes that are different. According to the PR description, there are several different changes. Do you think that submitting them in different PR will make the whole changes easier to rewview?

@polybassa
Copy link
Contributor Author

Unfortunately, most changes are inter-connected with each other.

classes

Here is a little overview.

Besides renaming, I introduced breaking cleanups.

The core of this scanners are the EcuState objects. This is an object, which stores the state of an Ecu in a generic way.
In the UDS and GMLAN protocol, I've add knowledge about state changing packets. This means, I can determine, if a packet
changed the state of an Ecu.
The Ecu object relies on that EcuState to serve it's utility functions. Also the EcuAnsweringMachine uses this mechanisms of the EcuState combined with the possibility to determine state changes based on transmitted packets. This allows an EcuAnsweringMachine to mimic the behavior of a real Engine Control Unit.
To configure an EcuAnsweringMachine, I need EcuResponse objects, which allow me to describe the response behavior of an EcuAnsweringMachine based on its current EcuState.

And then... I have Scanners... :-(
Since I now have a new way to describe the internal EcuState, also the scanners were cleaned up an refactored to become much more generic by the use of less code.
A Scanner aka AutomotiveTestCaseExecutor preserves a system state Graph of EcuState objects. An AutomotiveTestCaseExecutor maintains the connection to an Ecu under test and is in charge of manipulating an Ecu in a way, that all internal AutomotiveTestCases are executed for each available EcuState in the system state Graph. AutomotiveTestCases are in charge of performing tests on a target in a certain EcuState. A test cases sends request, in my case UDS, GMLAN or OBD, to a target, and evaluates the responses. As soon as a test case determines a state change of the EcuState of the real Ecu under test, (again from the functions implemented in the protocol), it will insert a new identified EcuState into the system state Graph of the AutomotiveTestCaseExecutor.

After some tests on the old implementation, I discovered, that the implementation of the EcuState object was not flexible enough.. so I had to change this object, which ended up in a huge chain of further changes. On that way of changes, I also heavily cleaned up the code an introduced typing. This is, how I ended up with this unfortunately huge PR.

I already removed unrelated changes to that core in #3036, #3035, #3034, #3033.

Since every thing is bound to this EcuState object, in some way, it's very hard to split this PR up. I will double check, if I can remove some minor changes which are not entangled with EcuState.

@codecov
Copy link

codecov bot commented Jan 19, 2021

Codecov Report

Merging #3054 (f0dc5c0) into master (7e161f1) will decrease coverage by 0.01%.
The diff coverage is 95.55%.

@@            Coverage Diff             @@
##           master    #3054      +/-   ##
==========================================
- Coverage   86.29%   86.28%   -0.02%     
==========================================
  Files         281      282       +1     
  Lines       63184    63225      +41     
==========================================
+ Hits        54522    54551      +29     
- Misses       8662     8674      +12     
Impacted Files Coverage Δ
scapy/contrib/automotive/scanner/profiler.py 94.59% <94.59%> (ø)
scapy/contrib/automotive/scanner/executor.py 83.97% <100.00%> (-0.94%) ⬇️
scapy/contrib/automotive/scanner/graph.py 70.42% <100.00%> (-0.60%) ⬇️
scapy/contrib/automotive/gm/gmlan_scanner.py 86.08% <0.00%> (-0.79%) ⬇️
scapy/arch/windows/__init__.py 67.73% <0.00%> (-0.57%) ⬇️
scapy/sendrecv.py 80.61% <0.00%> (-0.31%) ⬇️
scapy/layers/ntp.py 84.59% <0.00%> (-0.28%) ⬇️
scapy/fields.py 91.32% <0.00%> (-0.06%) ⬇️
... and 1 more

@polybassa polybassa force-pushed the scanner_refactoring branch 2 times, most recently from 4da6ddf to 921e1f6 Compare January 19, 2021 15:27
@polybassa polybassa closed this Jan 19, 2021
@polybassa polybassa reopened this Jan 19, 2021
@polybassa polybassa changed the title Unintended HUGH refactoring of automotive scanners. Unintended HUGE refactoring of automotive scanners. Jan 19, 2021
@polybassa polybassa marked this pull request as draft January 25, 2021 08:09
@polybassa polybassa mentioned this pull request Jan 25, 2021
@polybassa
Copy link
Contributor Author

@guedou I've started a series of smaller PRs to make this one somehow merge-able.

This PR:
  - changes variable names of automotive utilities to be more compliant
    with python
  - adds typing for various automotive utilities
  - refactors the core of automotive scanners which involve:
	- Ecu objects
	- EcuState objects
	- EcuAnsweringMachines
	- AutomotiveTestCases
	- AutomotiveTestCaseExecutors
  - cleans up UDS Scanner, OBD Scanner and GMLAN Scanner

Cleanups

Only use type imports from scapy.compat

Fix typing for NamedTuple

add sorted to EcuState __eq__ and __contains__

add default function

add debug flag to raise exception

add scan_range to UDS_SA Enumerator

fix bug

remove unused variable

test EcuStateComparison

improve EcuState comparision

move Profiler and Graph to independent files

remove dump to json

fix unit tests since add_edge function changed

Change internal transition functions to not use any lambdas

more cleanups

additions

remove unused variable

remove unused variable

be super sure to make a configuration

add function to create ECU clone from scanner

extend test

standalone graph tests

add documentation

fix tox

improve transition_function argument storage

minor cleanups

cleanup ecu_sim test

minor fix

update post and pre execute

test fixes

fix tests

fix test

add render function

add typing of render

Apply feedback

Fix flake8 errors

Apply feedback

Refactoring

Simplify RC and RDBI enumerators

Add Selective Enumerator for RDBI and RC

Pass current state to _get_initial_requests

Fix UDS_RCPR.answers

add cleanup to tests

update typing

fix after rebase

cleanup

add pickle test of transition function

disable pickle on python2

Also treat NR code 0x7f as serviceNotSupported

Remove old comment

Better logging for serviceNotSupported reactions

try to fix unit test

change blacklist creation

remove NamedTuple

fixes

revert changes

add root to uds_utils test

more info

update

Allow scan_range to be modified in UDS_RCStartEnumerator

minmal doc cleanup

cleanup transition functions

update render function

add weights to graph

fix unit test ecusim with old data

update

update

update uds protocol RoutineControl

update

ServiceEnumerator should ignore exit_if_service_not_supported
Remove list generation in DSCEnumerator

Refactoring + fix Flake8

Set better expansion_width (253)

Fix comment

modify ecu state modifier

refactor ecu state modifications

cleanup logging of protocols

add api docs

minor fixes

two more states

update

update

update uds ecu states

update

add test cases in enumerator.uts

update evaluate response

update

improve extension method

update

test supported_responses

fix mypy

fix types of timestamp

add directory for scanner code

split enumerator in different files

add contrib comment

fix BMW enumerator

fix BMW enumerator

add testcase positive&negative results

adapt enumerator.uts to change

fix enumerator.uts

fix typing

start with documentation of testcase

complete documentation of AutomotiveTestCaseABC

Renaming

update scanner unit tests

More unit tests for individual scanner parts

fix mypy

fix docs

fix docs

fix test

fix test

remove BrokenPipeError

fix test

further cleanups

further cleanups

more tests

fix codespell

fix test

fix test

fix flake

more unit tests

cleanup

update

fix defaults

bugfix

enable exception to find bug

try bugfix

update periodicSenderThread

update

update

update unit tests

update

add connectionError

fix tox

add more verbose tpsender debug

minor cleanup

add BrokenPipeError to ignored errors during scan

fix HSFZ scan

minor fix

fix mypy

alphabetic sort of mypy enable contrib

update

close socket on Connection Error

rename HSFZ Sockets

fix mypy

fix unit test for obdscanner

refactoring of get_table_entry

fix flake and mypy

fix docs

fix build

fix mypy

disable test

fix tests

fix tests

minor cleanup

update gmlan scanner

debug closed socket error

fix tox

fix tox

update gmlan scanner

update

update

start with unit test implementation

Update retry_packet handling

update unit tests

fix tox and code

add candump

cleanup pcaps and candump logs

more unit tests

update configuration unit tests

improve tests-speed a lot

refactoring of configuration

fix test

debug socket

debug socket

more unit tests

try to fix bug

more unit tests and bugfix

update tests

more and faster tests

debug error

fix tox

fix stmin bug

fix uds unit test

include isotp stmin fix

update tests

update tests

update tests

fix test

debug test

fix tox

try fix

refactor RMBA enumerator

apply feedback

apply feedback

minor fix

minor fix in HSFZ

test propagation of exception

update

add error handling on closed sockets

update error handling

minor exception handling fix

stability update

add pickle test

fix test

debug test

fix test

fix test

improve test

improve test

speedup gmlan test

minor cleanup

update

update

update

minor changes

minor changes

remove long test from pypy

remove unused file

add unstable socket to increase test coverage

fix tox

fix tox

increase timeout of import.uts for macOSX

remove delay_state_change configuration

provide kwargs type information and documentation

fix test

remove old file

add more kwargs checks and improve execution time

update

update

fix obdscanner
@polybassa polybassa closed this Dec 18, 2021
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
…weringMachine. Including typing (secdev#3099)

* Split secdev#3054: Update of Ecu, EcuResponse, EcuSession and EcuAnsweringMachine. Including typing

* Update documentation

* fix obdscanner tests

* Update documentation

* update test

* update test and docs

* make api-doc more nice

* update codespell

* major cleanup

* fix code spell

* fix docs build

* apply feedback

* cleanup

* fix typing

* fix typing
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
* Split secdev#3054: Add base classes for a new and clean implementation of automotive scanners and enumerators
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
…classes (secdev#3189)

* Split of secdev#3054: Implementation of Enumerator and Executor base classes
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 12, 2022
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 14, 2022
…weringMachine. Including typing (secdev#3099)

* Split secdev#3054: Update of Ecu, EcuResponse, EcuSession and EcuAnsweringMachine. Including typing

* Update documentation

* fix obdscanner tests

* Update documentation

* update test

* update test and docs

* make api-doc more nice

* update codespell

* major cleanup

* fix code spell

* fix docs build

* apply feedback

* cleanup

* fix typing

* fix typing
bzalkilani pushed a commit to bzalkilani/scapy that referenced this pull request Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants