Skip to content

Firewall leak detector makes bad assumptions about PacketFu parsing

I’ve seen this:

calling as amnesia: host -T torproject.org 208.67.222.222
call returned: [0, "Using domain server:\nName: 208.67.222.222\nAddress: 208.67.222.222#53\nAliases: \n\ntorproject.org has address 86.59.30.40\ntorproject.org has address 38.229.72.16\ntorproject.org has address 82.195.75.101\ntorproject.org has address 138.201.14.197\ntorproject.org has IPv6 address 2001:41b8:202:deb:213:21ff:fe20:1426\ntorproject.org has IPv6 address 2001:858:2:2:aabb:0:563b:1e28\ntorproject.org has IPv6 address 2a01:4f8:172:1b46:0:abba:5:1\ntorproject.org has IPv6 address 2620:0:6b0:b:1a1a:0:26e5:4810\ntorproject.org mail is handled by 10 eugeni.torproject.org.\n", ""]
    When I do a TCP DNS lookup of "torproject.org"                                        # features/step_definitions/firewall_leaks.rb:19
    Then the firewall leak detector has detected leaks                                    # features/step_definitions/firewall_leaks.rb:1
      <Test::Unit::AssertionFailedError> exception expected but was
      <NoMethodError(<undefined method `ip_saddr' for nil:NilClass>)>. (Test::Unit::AssertionFailedError)
      ./features/step_definitions/firewall_leaks.rb:2:in `/^the firewall leak detector has detected leaks$/'
      features/tor_enforcement.feature:33:in `Then the firewall leak detector has detected leaks'

This implies that the various PacketFu .parse methods can return nil despite .can_parse? returning true. This feels like a bug in PacketFu.

Sadly the way this error happened prevented a .pcap artifact to be saved. Also:

Then(/^the firewall leak detector has detected leaks$/) do
  assert_raise(Test::Unit::AssertionFailedError) do
    step 'all Internet traffic has only flowed through Tor'
  end
end

That step calls assert_all_connections and given how we have the opposite expectation about the assertion (assert_raise) the artifact saving occurs in the opposite of what we want here.

Also: probably we’ll end up throwing in some asserts in the pcap_connections_helper (e.g. for the nil issue above) so we should probably introduce another exception to throw when assert_all_connections fails, and a subclass of Test::Unit::AssertionFailedError would be good enough to make it possible to distinguish.

Parent Task: #10288

Original created by @anonym on 11508 (Redmine)

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information