In dissect_relay_resolve_payload(), what happens if the buffer handed to resolve_address isn't null-terminated? You might want to use tvb_get_ephemeral_string() or something similar instead. Otherwise it looks OK.

Do you have any sample captures you can attach, and has the code been fuzz tested?

A sample dump is available at:
http://roberthogan.net/stuff/dissector/examples/

You can follow the instructions at:

http://roberthogan.net/stuff/dissector/tordissector-README.txt

under section 'CAPTURE AND DISSECT TOR TRAFFIC'.

I haven't been able to fuzz test the decoder yet because there is no fuzz testing client available for Tor. To fuzz the raw packets myself I would have to dump them unencrypted and then reverse perform the onion-layer crypto on each as well as the SSL crypto. I can look into doing that but I may be some time.

Also, I think it might be best to postpone applying this patch until the complementary patch for Tor enters the 0.2.2.x series. I'll return here when that happens.

Let me know about the fuzz testing requirements. I've followed your advise on dissect_relay_resolve_payload().

You can fuzz a Wireshark dissector using tools/fuzz-text.sh and/or editcap plus one or more capture files. See http://wiki.wireshark.org/FuzzTesting for more details.

I asked about attaching a capture here because public captures on bugs.wireshark.org and wiki.wireshark.org are automatically harvested for the fuzz tests on buildbot.wireshark.org.

Created attachment 3021 [details]
sample dump

sorry, didn't mean to be obtuse: I have fuzzed a sample payload using the wireshark tools but since the tor payload is ssl-encrypted this only real tests the existing ssl dissector, since ssl decryption uniformly fails. A true fuzz test will involve fuzzing both the ssl plaintext payload and also the tor plaintext payload which is wrapped in up to three layers of aes encryption. I will look into doing this before requesting check-in.

Created attachment 3032 [details]
recent dump file

Created attachment 3033 [details]
Point to this file in SSl preferences and Tor preferences when dissecting tor-220509.dmp

Created attachment 3034 [details]
Patch against svn as of 220509

Text of updated HOWTO for wireshark tor dissector:

Tor Dissector for Wireshark
---------------------------

This HOWTO describes the steps required to view and analyze Tor traffic in Wireshark.
It should be useful for researchers analyzing the behaviour of various versions of 
the Tor client. This patchset will not allow you to decrypt the traffic of an Onion
Router (i.e. a relay node on the Tor network), only the traffic coming to and from
the Tor client on your machine (i.e. an Onion Proxy). Doing the former would require
a different patch to Tor and apart from being unethical might even be illegal in your
jurisdiction! (You would be snooping on the traffic of other Tor users.)

This README and all required files can be found at:
http://roberthogan.net/stuff/dissector/

If you find that any of the files referenced below do not exist anymore you should 
inspect the URL above for the updated reference.

Contact details: robert at roberthogan.net


BUILD THE NECESSARY TOOLS
-------------------------

1. Patch Wireshark
  This patch is very much a work in progress. The date stamp on the patch will likely
  change over time so please be sure to check the parent directory if the link below
  no longer works for you. Please let me know about any bugs you find.

  - Download the wireshark patch:
    http://roberthogan.net/stuff/dissector/patches/wireshark-tordissector-20090208.diff
  - Download wireshark svn:
    svn checkout http://anonsvn.wireshark.org/wireshark/trunk wireshark
  - Apply the patch:
    cd /location/of/wireshark
    patch -p0 < /location/of/wireshark-tordissector-20090208.diff
  - Build wireshark:
    ./autogen.sh
    ./configure 
    make
    sudo makeinstall

2. Patch Tor
  This is a modified version of a patch by Steven Murdoch. The patch logs the TLS
  master keys used to TLS-encrypt traffic between your copy of Tor and other routers,
  it also logs the AES keys used to encrypt relay cells passed along the circuits
  created by your copy of Tor. Wireshark will use these logs to decrypt the TLS and
  circuit streams it finds in the traffic captures you create later.

  - Download the tor patch:
    http://roberthogan.net/stuff/dissector/patches/tor-dissector.diff
  - Download tor svn:
    svn checkout https://svn.torproject.org/svn/tor/trunk tor
  - Apply the patch:
    cd /location/of/tor
    patch -p0 < /location/of/tor-dissector.diff
  - Build tor (NOTE CONFIGURE PARAMETER BELOW!):
    ./autogen.sh
    ./configure --enable-highly-insecure-key-debugging
    make
    sudo makeinstall

CAPTURE AND DISSECT TOR TRAFFIC
-------------------------------

1. If you are impatient you can view the sample log and traffic capture at
    http://roberthogan.net/stuff/dissector/examples/sampledump/
   and skip to step 4.

2. Start The Traffic Capture
  - sudo /location/of/svnwireshark/tshark -i eth0 -w /location/of/dump/test.dump

3. Start Tor
  - /location/of/svntor/src/or/tor --SafeLogging 0 --Log info > /location/of/dump/torkeys.txt

4. Configure Wireshark.
  - Open wireshark.
  - Click Edit->Preferences.
  - Expand the 'Protocols' line on the left hand side of the preferences dialog.
  - Scroll down or type 'SSL' to go to the SSL preferences section.
  - Paste the full path of the torkeys.txt file mentioned in step 2 above to
    'SSL master keys list'.
      e.g. /location/of/dump/torkeys.txt
    See also: 
      http://roberthogan.net/stuff/dissector/examples/wireshark-ssl-config.png
  - Click 'Apply'
  - Scroll down or type 'Tor' to go to the Tor preferences section.
  - Paste the full path of the torkeys.txt file mentioned in step 2 above to
    'Tor cell keys list'.
      e.g. /location/of/dump/torkeys.txt
    See also:
      http://roberthogan.net/stuff/dissector/examples/wireshark-tor-config.png
  - Click 'OK' to save the settings and close the dialog.

5. View the capture.
  - Open /location/of/dump/test.dump in wireshark.
  - To view Tor data only, type 'tor' in the 'Filter' edit box on the top left and
    click 'Apply'.
  - You can now view the Tor traffic in your capture. For an idea of what you should
    see, take a look at:
    http://roberthogan.net/stuff/dissector/examples/wireshark-tor-dissector-1.png
  - You can filter the view using most of the Tor protocol items. For example,
    to view the destruction of circuits because of Tor protocol violations you can
    type 'tor.destroyreason==0x04' in the 'Filter' edit box. The item name you should
    use for each protocol item is visible on the bottom-left of the status bar when
    you select that item. For example:
      http://roberthogan.net/stuff/dissector/examples/wireshark-tor-dissector-destroyreason.png

ACKNOWLEDGEMENTS
----------------
Steven Murdoch for pointing out to me what I needed to do to get the AES-CTR decryption working.
Nick Mathewson for pointing out a number of flaws with my initial adaptation of Tor's AES-CTR
implementation. 
Any remaining errors are very much my own.

  

Created attachment 3610 [details]
Updated Patch against SVN trunk

Created attachment 3611 [details]
Updated Patch against SVN trunk

The functionality to support this dissector is now in Tor trunk, so would it be possible to give the patch a look over?

Created attachment 3750 [details]
Updated Patch against SVN trunk

Removes the 'if (tree)' check that causes problems with desegmented payloads for dissectors.

Created attachment 3791 [details]
Updated Patch

Updated against SVN.

Created attachment 4293 [details]
Updated Patch

Updated against SVN

I'd like to get bug 3343 (convert SSL preferences to a UAT) committed first.

Created attachment 5008 [details]
Updated Patch

Comment on attachment 5008 [details]
Updated Patch

Some initial comments after a review:

1. Update cell_keys_list to UAT instead of formatted string
2. Has LibCrypt been updated with the necessary support for AES-CTR mode?  Are the packet-ssl-utils.[ch] updates still necessary?
3. Patch contains a bunch of unnecessary files.  It should just be packet-tor.c, packet-ssl-utils.[ch], (if necessary), and the updated makefiles to include packet-tor.c
4. hf_tor_version should be added for the "typo" of the second hf_tor_length, and both could be added as proto_tree_add_item()s
5. Use proper encoding macros for proto_tree_add_item() (fix-encoding-args.pl can help)
6. Cleanup noted memory leaks.