CaringCaribou / caringcaribou

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

Interface Agnostic Handling of Multiple Interface Devices (Remove SocketCAN specific interface flag '-i') #52

Closed bhass1 closed 5 years ago

bhass1 commented 5 years ago

Currently the cc.py module takes a '-i' argument which is meant to specify the SocketCAN interface (e.g. can0 or vcan0). However, CaringCaribou uses python-can as its CAN abstraction layer enabling support for a variety of device interfaces and drivers.

I propose removing the '-i' flag altogether and let python-can handle whether can0, vcan0, or whatever interface along with its settings is used.

A help message pointing to the python-can docs (https://python-can.readthedocs.io/en/3.1.1/configuration.html) should be included when "cc.py -h" is ran.

kasperkarlsson commented 5 years ago

See response to PR in https://github.com/CaringCaribou/caringcaribou/pull/53#issuecomment-481210607

In general, it is a good idea to discuss things in an issue before sending a PR. I am currently not convinced that completely removing the possibility to specify an interface is the best way to handle this.

bhass1 commented 5 years ago

Fair point about handling setups with multiple interfaces. That is not my use-case so I hadn't thought about that.

At the root of this issue is a question of how much interface configuration should be handled by Caring Caribou versus python-can. My standard procedure has been to create a ~/can.conf file per python-can recommendation and everything "just-works" with a single interface.

In the meantime, a less intrusive change would be to modify the documentation to make it more clear that -i forces SocketCAN to be used with the specified interface.

kasperkarlsson commented 5 years ago

Agreed. Let's keep this issue open for brainstorming/suggestions - it would be great to find an interface type agnostic way of handling this!

kasperkarlsson commented 5 years ago

After going through the python-can documentation, I found that the interface type specifier is now optional. Today I was (finally) able to test this out in a few different live environments, which all seem to work just fine.

@bhass1 It would be great if you could try this out too - it's in the branch https://github.com/CaringCaribou/caringcaribou/tree/interface-agnostic. The code changes were introduced in commit https://github.com/CaringCaribou/caringcaribou/commit/38b7a8fccf64e840c989ae2e726680e04ddedc3f

If no new problems are found, this will be merged to master shortly.

Epsilon314 commented 5 years ago

When trying out with flag -i vcan0 a NotImplementedError was raised and it seems that interface = None is passed to python-can instead of 'socketcan'.

After reading that issue, i tried to set default interface for python-can to 'socketcan' in ~/can.conf file and it works then. Also, instead of configing python-can from file, changing line 216 in can_actions.py to self.bus = can.Bus(DEFAULT_INTERFACE,bustype="socketcan") works either.

Is it true that the second parameter in original self.bus = can.Bus(DEFAULT_INTERFACE,"socketcan") is not parsed and python-can actually uses file config?

kasperkarlsson commented 5 years ago

@Epsilon314 Yes. When bustype wasn't present in kwargs, older versions of python-can would fallback to reading the configuration file. In more recent versions, I believe the same thing is controlled through the channel kwarg for can.interface.Bus.

I am closing this issue now - feel free to open a new one if you encounter some unexpected behavior.