From 40fbefa06ba9ceba641abf7b80d779cba678a00b Mon Sep 17 00:00:00 2001 From: Brian Candler Date: Fri, 6 May 2011 17:01:50 +0100 Subject: [PATCH] Experiment: try using FFI::Struct for iptables instead of CStruct FFI::Struct handles nested structs and nested arrays much better, and avoids duplicating logic about structure alignment (which it probably does more correctly that CStruct) However it's awkward to use in other ways. e.g. no accessor methods; no proper #inspect; no ntohl for in_addr; no zero-sized arrays at end of struct; no hooks to convert int32 <-> IPAddr as far as I can see. --- lib/linux/iptables.rb | 73 +++++++++++++++++++++------------ lib/linux/iptables4.rb | 91 +++++++++++++++++++++--------------------- 2 files changed, 93 insertions(+), 71 deletions(-) diff --git a/lib/linux/iptables.rb b/lib/linux/iptables.rb index 6867ec2..0af7cf0 100644 --- a/lib/linux/iptables.rb +++ b/lib/linux/iptables.rb @@ -1,11 +1,35 @@ require 'socket' require 'linux/constants' -require 'linux/c_struct' -require 'linux/netlink/message' # just for :dev_name type +require 'ffi' + +# Good things about FFI::Struct: +# - robust pre-existing code +# - good handling of nested structures and nested arrays +# Bad things about FFI::Struct: +# - no Hash initialization: MyStruct.new(:foo=>1, :bar=>2) +# - no accessor methods m.foo = 1 +# - can't do zero size array at end of struct: layout :foo, [Foo, 0] +# - no network-order fields? (in_addr) +# - no decent inspect (fix this below) + +class FFI::Struct + def inspect + res = "#<#{self.class}" + members.zip(values).each do |m,v| + res << " #{m}=#{v.inspect}" + end + res << ">" + end +end + +class FFI::StructLayout::CharArray + def inspect + to_s.inspect + end +end module Linux module Ext - require 'ffi' extend FFI::Library ffi_lib FFI::Library::LIBC attach_function :getsockopt, [:int, :int, :int, :buffer_inout, :buffer_inout], :int @@ -20,10 +44,6 @@ module Linux # libip4tc.c and libip6tc.c) # # filter = Linux::Iptables4.table("filter") - # - # TODO: should we use ffi's structures instead of CStruct? - # We have to use ffi anyway, until ruby getsockopt supports buffer passing. - # http://redmine.ruby-lang.org/issues/4645 class Iptables def self.inherited(subclass) #:nodoc: subclass.instance_variable_set(:@tables, {}) @@ -66,36 +86,39 @@ module Linux def getsockopt(level, optname, buf) buflen = FFI::Buffer.new :socklen_t - if buflen.length == 4 - buflen.put_uint32(0, buf.length) - elsif buflen.length == 8 - buflen.put_uint64(0, buf.length) + if buflen.size == 4 + buflen.put_uint32(0, buf.size) + elsif buflen.size == 8 + buflen.put_uint64(0, buf.size) else - raise "Unexpected buflen length: #{buflen.length}" + raise "Unexpected buflen length: #{buflen.size}" end res = Ext.getsockopt(@socket.fileno, level, optname, buf, buflen) raise "getsockopt error: #{res}" if res < 0 # FIXME: get errno? - buf.get_bytes(0, buflen.length == 4 ? buflen.get_uint32(0) : buflen.get_uint64(0)) + res # unlike Ruby's getsockopt, we return the length, not the buf end def reload - buf = FFI::Buffer.new IPTGetInfo.bytesize - buf.put_string(0, @name) - info = self.class::STRUCT_GETINFO.parse(getsockopt(self.class::TC_IPPROTO, self.class::SO_GET_INFO, buf)) - #warn "valid_hooks=0x%08x, num_entries=%d, size=%d" % [info.valid_hooks, info.num_entries, info.size] + info = IPTGetInfo.new + info[:name] = @name + getsockopt(self.class::TC_IPPROTO, self.class::SO_GET_INFO, info) + #warn "valid_hooks=0x%08x, num_entries=%d, size=%d" % [info[:valid_hooks], info[:num_entries], info[:size]] - buf2 = FFI::Buffer.new(self.class::STRUCT_GET_ENTRIES.bytesize + info.size) - buf2.put_bytes(0, self.class::STRUCT_GET_ENTRIES.new(:name=>@name, :size=>info.size).to_str) + init = self.class::STRUCT_GET_ENTRIES.new + init[:name] = @name + init[:size] = info[:size] + buf2 = FFI::MemoryPointer.new(self.class::STRUCT_GET_ENTRIES_SIZE + info[:size]) + buf2.put_bytes(0, init.pointer.get_bytes(0, self.class::STRUCT_GET_ENTRIES_SIZE)) getsockopt(self.class::TC_IPPROTO, self.class::SO_GET_ENTRIES, buf2) res = [] - ptr = self.class::STRUCT_GET_ENTRIES.bytesize - limit = ptr + info.size - while ptr < limit - res << self.class::STRUCT_ENTRY.parse(buf2.get_bytes(ptr, self.class::STRUCT_ENTRY.bytesize)) - ptr += res.last.next_offset + offset = self.class::STRUCT_GET_ENTRIES_SIZE + limit = offset + info[:size] + while offset < limit + res << self.class::STRUCT_ENTRY.new(buf2 + offset) + offset += res.last[:next_offset] end - raise "Error parsing rules: got #{res.size} instead of #{info.num_entries}" if res.size != info.num_entries + raise "Error parsing rules: got #{res.size} instead of #{info[:num_entries]}" if res.size != info[:num_entries] @rules = res end end diff --git a/lib/linux/iptables4.rb b/lib/linux/iptables4.rb index fa22c10..e4239f7 100644 --- a/lib/linux/iptables4.rb +++ b/lib/linux/iptables4.rb @@ -1,4 +1,5 @@ require 'linux/iptables' +require 'ipaddr' module Linux #- @@ -6,55 +7,51 @@ module Linux #+ # struct ipt_getinfo - class IPTGetInfo < CStruct - field :name, :pattern=>"Z#{IPT_TABLE_MAXNAMELEN}", :default=>EMPTY_STRING - field :valid_hooks, :int - #field :hook_entry, :pattern=>"I#{NF_INET_NUMHOOKS}", :default=>[0]*NF_INET_NUMHOOKS - #field :underflow, :pattern=>"I#{NF_INET_NUMHOOKS}", :default=>[0]*NF_INET_NUMHOOKS - field :hook_entry, :pattern=>"a#{NF_INET_NUMHOOKS*4}", :default=>EMPTY_STRING - field :underflow, :pattern=>"a#{NF_INET_NUMHOOKS*4}", :default=>EMPTY_STRING - field :num_entries, :int - field :size, :int + class IPTGetInfo < FFI::Struct + layout :name, [:char, IPT_TABLE_MAXNAMELEN], + :valid_hooks, :uint, + :hook_entry, [:uint, NF_INET_NUMHOOKS], + :underflow, [:uint, NF_INET_NUMHOOKS], + :num_entries, :uint, + :size, :uint + end + + class IPTIP < FFI::Struct + layout :src, :int32, # FIXME: needs ntohl + :dst, :int32, + :smsk, :int32, + :dmsk, :int32, + :iniface, [:char, IFNAMSIZ], + :outiface, [:char, IFNAMSIZ], + :iniface_mask, [:uchar, IFNAMSIZ], + :outiface_mask, [:uchar, IFNAMSIZ], + :proto, :uint16, + :flags, :uint8, + :invflags, :uint8 + end + + # struct xt_counters (netfilter/x_tables.h) + class XTCounters < FFI::Struct + layout :pcnt, :uint64, + :bcnt, :uint64 + end + + # struct ipt_entry + class IPTEntry < FFI::Struct + layout :ip, IPTIP, + :nfcache, :uint, + :target_offset, :uint16, # size of ipt_entry + matches + :next_offset, :uint16, # size of ipt_entry + matches + target + :comefrom, :uint, + :counters, XTCounters, + :elems, [:uchar, 1] # should be [:uchar, 0] end # struct ipt_get_entries - class IPTGetEntries < CStruct - field :name, :pattern=>"Z#{IPT_TABLE_MAXNAMELEN}", :default=>EMPTY_STRING - field :size, :uint - field :entrytable, :binary, :align=>1.size # struct ipt_entry entrytable[0] - end - - # struct ipt_entry - class IPTEntry < CStruct - #### struct ipt_ip - field :src, :nl # struct in_addr - field :dst, :nl - field :smsk, :nl - field :dmsk, :nl - field :iniface, :dev_name - field :outiface, :dev_name - field :iniface_mask, :dev_name - field :outiface_mask, :dev_name - field :proto, :uint16 - field :flags, :uchar - field :invflags, :uchar - #### - field :nfcache, :uint - field :target_offset, :uint16 # size of ipt_entry + matches - field :next_offset, :uint16 # size of ipt_entry + matches + target - field :comefrom, :uint - ### struct xt_counters - field :packet_count, :uint64, :align => 8 - field :byte_count, :uint64 - ### - field :elems, :binary # matches (if any), then the target - - def after_parse - self.src = src == 0 ? nil : IPAddr.new(src, Socket::AF_INET) - self.dst = dst == 0 ? nil : IPAddr.new(dst, Socket::AF_INET) - self.smsk = smsk == 0 ? nil : IPAddr.new(smsk, Socket::AF_INET) - self.dmsk = dmsk == 0 ? nil : IPAddr.new(dmsk, Socket::AF_INET) - end + class IPTGetEntries < FFI::Struct + layout :name, [:uchar, IPT_TABLE_MAXNAMELEN], + :size, :uint, + :entrytable, [IPTEntry, 1] # should be [IPTEntry, 0] end # Class for handling iptables. Note that this doesn't actually use @@ -72,6 +69,8 @@ module Linux STRUCT_ENTRY = IPTEntry STRUCT_GETINFO = IPTGetInfo STRUCT_GET_ENTRIES = IPTGetEntries + # This is a frig because of [1] instead of [0] above + STRUCT_GET_ENTRIES_SIZE = IPTGetEntries.offset_of(:entrytable) end end