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

Add support for CC2530/1 USB dongles for sniffing. #94

Merged
merged 2 commits into from
Jul 2, 2017

Conversation

Scytmo
Copy link
Contributor

@Scytmo Scytmo commented Jun 20, 2017

This adds support for packet sniffing for USB dongles based
on the TI CC2530 and CC2531 chips.

Note that the FCS received from these dongles is in TI CC24XX
format, so for wireshark to display the FCS information correctly,
the "TI CC24XX FCS format" in Preferences->Protocols->IEEE802.15.4
must be selected.

This adds support for packet sniffing for USB dongles based
on the TI CC2530 and CC2531 chips.

Note that the FCS received from these dongles is in TI CC24XX
format, so for wireshark to display the FCS information correctly,
the "TI CC24XX FCS format" in Preferences->Protocols->IEEE802.15.4
must be selected.
@Scytmo
Copy link
Contributor Author

Scytmo commented Jun 20, 2017

Hi. I know there are other packet sniffers for these dongles, but I am getting used to using zbwireshark and zbdump for packet sniffing. This commit adds support for these two dongles (at least, the Chinese clone examples that I have) for packet sniffing only. Not sure whether you're interested in this, but I thought I'd submit this pull request in case.

@rmspeers
Copy link
Collaborator

You beat me to this -- I was about to commit a similar one when I had a chance to test. I will take a look at this one now.



def close(self):
pass
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my version I was checking to see if the device was in capture mode, and if so halting that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll add that.

@rtype: List
@return: List of 3 strings identifying device.
'''
return [self.name, "CC253x", ""]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get a serial number off the device?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of the devices that I own have iSerial = 0. I'm not sure if that's typical. I could add a check and use it if it isn't 0 (but have no way of testing it)...?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I didn't have a chance to check mine when reviewing this.
What about a version number of the firmware, does it give us that?
If not, let's leave it out for now and just put a TODO in the code to figure out how to get a unique ID, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see anything else in the USB descriptor that would uniquely identify the device. Also, the Windows based TI Packet Sniffer tool doesn't report a unique device ID for either of my two dongles, which would suggest there isn't a "get device ID" USB transaction (or, that the Windows tool isn't using it - which will make it difficult to discover through reverse engineering). I've added a TODO.

close().
@rtype: None
'''
self.dev.ctrl_transfer(CC253x.USB_DIR_OUT, CC253x.USB_XFER_STOP)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the time we check to see if __stream_open is true first, as other devices have had edge cases there. If this one is safe, that's OK not to check it.
Can we check the result of ctrl_transfer before setting __stream_open to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check for __stream_open == True should be added, yes.
Regarding checking the results of ctrl_transfer, I'm not sure what the recovery for a failure would be, either for the driver, or for the application (we don't return a success/failure value from that function)...?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I see your point -- we'd have to throw an exception which probably isn't great either. Maybe just put a TODO in the comments and I'll look at how to rework error handling at a later time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've added a TODO.

if channel >= 11 or channel <= 26:
self._channel = channel
else:
raise Exception('Invalid channel')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other implementations I think we change the channel here -- so that this can happen mid capture, etc -- without the developer using KillerBee to need to know that you need to stop, change the channel, and restart. Could we do the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but this needs to be dependent on whether the capture is running. On my CC2530 dongle, the channel setting fails if the radio isn't powered up. I'll add that.

self._data_ep = CC253x.USB_CC2531_DATA_EP
self._channel = None
self.dev = dev
self.name = "<unknown>"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we probably don't need this as it is set on L 61?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@Scytmo
Copy link
Contributor Author

Scytmo commented Jun 22, 2017

Thanks for your comments. I've got a commit ready to push, subject to your feedback on a couple of the comments (the serial number, and checking the return from ctrl_transfer in sniffer_off).

Updated after code review:
 - Removed redundant code.
 - Check state of capture in close() and sniffer_off().
 - Perform channel change during capture.
@Scytmo
Copy link
Contributor Author

Scytmo commented Jun 25, 2017

I've pushed another commit, which I believe addresses all of your review comments.

@rmspeers rmspeers merged commit b1d6094 into riverloopsec:master Jul 2, 2017
@Scytmo
Copy link
Contributor Author

Scytmo commented Jul 3, 2017

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants