improved error handling

do not print stack trace in case of user errors, fixes #1453
immediate fallback after failed activation, even with --force, before main process exit, fixes #1584

5 files changed, 189 insertions, 127 deletions

diff --git a/awall-cli b/awall-cli
index 9f515eb..40b3a13 100755
--- a/awall-cli
+++ b/awall-cli
@@ -2,7 +2,7 @@
 
 --[[
 Alpine Wall
-Copyright (C) 2012 Kaarle Ritvanen
+Copyright (C) 2012-2013 Kaarle Ritvanen
 Licensed under the terms of GPL2
 ]]--
 
@@ -63,7 +63,7 @@ Dump variable and zone definitions:
     Verbosity level is an integer in range 0-5 and defaults to 0.
 
 ]])
-   os.exit()
+   os.exit(1)
 end
 
 params = {}
@@ -107,144 +107,180 @@ if not util.contains({'translate', 'activate', 'fallback', 'flush',
 		     mode) then help() end
 
 
-require 'awall'
+require 'awall.uerror'
 
-policyset = awall.PolicySet.new(params.i, params.I)
+if not awall.uerror.call(
+   function()
+      
+      require 'awall'
 
-if mode == 'list' then
-   util.printtabular(policyset:list())
-   os.exit()
-end
+      policyset = awall.PolicySet.new(params.i, params.I)
 
-if util.contains({'disable', 'enable'}, mode) then
-   if opind > #arg then help() end
-   repeat
-      policyset[mode](policyset, arg[opind])
-      opind = opind + 1
-   until opind > #arg
-   os.exit()
-end
+      if mode == 'list' then
+	 util.printtabular(policyset:list())
+	 os.exit()
+      end
 
+      if util.contains({'disable', 'enable'}, mode) then
+	 if opind > #arg then help() end
+	 repeat
+	    policyset[mode](policyset, arg[opind])
+	    opind = opind + 1
+	 until opind > #arg
+	 os.exit()
+      end
 
-input = policyset:load()
 
-if mode == 'dump' then level = 0 + (arg[opind] or 0) end
+      input = policyset:load()
 
-if mode ~= 'dump' or level > 3 then
-   awall.loadmodules(basedir)
-   config = awall.Config.new(input)
-end
+      if mode == 'dump' then level = 0 + (arg[opind] or 0) end
 
+      if mode ~= 'dump' or level > 3 then
+	 awall.loadmodules(basedir)
+	 config = awall.Config.new(input)
+      end
 
-require 'awall.iptables'
 
-if mode == 'dump' then
-   require 'json'
-   expinput = input:expand()
+      require 'awall.iptables'
 
-   function capitalize(cls)
-      return string.upper(string.sub(cls, 1, 1))..string.sub(cls, 2, -1)
-   end
+      if mode == 'dump' then
+	 require 'json'
+	 expinput = input:expand()
+
+	 function capitalize(cls)
+	    return string.upper(string.sub(cls, 1, 1))..string.sub(cls, 2, -1)
+	 end
 
-   for cls, objs in pairs(input.data) do
-      if level > 2 or (level == 2 and cls ~= 'service') or util.contains({'variable',
-									  'zone'},
-									 cls) then
-	 if level == 0 then print(capitalize(cls)..'s:') end
+	 for cls, objs in pairs(input.data) do
+	    if level > 2 or (level == 2 and cls ~= 'service') or util.contains(
+	       {'variable', 'zone'},
+	       cls
+	    ) then
+	       if level == 0 then print(capitalize(cls)..'s:') end
 	 
-	 items = {}
-	 for k, v in pairs(objs) do
-	    exp = expinput[cls][k]
-	    expj = json.encode(exp)
-	    src = input.source[cls][k]
-
-	    if level == 0 then table.insert(items, {k, expj, src})
-
-	    else
-	       data = {{capitalize(cls)..' '..k, json.encode(v)},
-		       {'('..src..')',
-			util.compare(exp, v) and '' or '-> '..expj}}
-
-	       if level > 3 then
-		  obj = config.objects[cls][k]
-		  if type(obj) == 'table' and obj.info then
-		     util.extend(data, obj:info())
+	       items = {}
+	       for k, v in pairs(objs) do
+		  exp = expinput[cls][k]
+		  expj = json.encode(exp)
+		  src = input.source[cls][k]
+
+		  if level == 0 then table.insert(items, {k, expj, src})
+
+		  else
+		     data = {
+			{capitalize(cls)..' '..k, json.encode(v)},
+			{
+			   '('..src..')',
+			   util.compare(exp, v) and '' or '-> '..expj
+			}
+		     }
+
+		     if level > 3 then
+			obj = config.objects[cls][k]
+			if type(obj) == 'table' and obj.info then
+			   util.extend(data, obj:info())
+			end
+		     end
+	       
+		     table.insert(items, {k, data})
 		  end
 	       end
+	       table.sort(items, function(a, b) return a[1] < b[1] end)
+
+	       if level == 0 then util.printtabular(items)
+	       else
+		  util.printtabulars(
+		     util.map(items, function(x) return x[2] end)
+		  )
+		  print()
+	       end
+	    end
+	 end
 
-	       table.insert(items, {k, data})
+	 if level > 4 then config:print() end
+
+      elseif mode == 'translate' then
+	 if verify then config:test() end
+	 config:dump(outputdir)
+   
+      elseif mode == 'activate' then
+
+	 awall.iptables.backup()
+
+	 if not force then
+	    signal.signal(
+	       'SIGCHLD',
+	       function()
+		  if pid and lpc.wait(pid, 1) then os.exit(2) end
+	       end
+	    )
+	    for i, sig in ipairs({'INT', 'TERM'}) do
+	       signal.signal(
+		  'SIG'..sig,
+		  function()
+		     interrupted = true
+		     io.stdin:close()
+		  end
+	       )
 	    end
+
+	    require 'lpc'
+	    pid, stdio, stdout = lpc.run(arg[0], 'fallback')
+	    stdio:close()
+	    stdout:close()
 	 end
-	 table.sort(items, function(a, b) return a[1] < b[1] end)
 
-	 if level == 0 then util.printtabular(items)
-	 else
-	    util.printtabulars(util.map(items,
-					function(x) return x[2] end))
-	    print()
+	 function kill()
+	    signal.signal('SIGCHLD', 'default')
+	    signal.kill(pid, 'SIGTERM')
+	    lpc.wait(pid)
 	 end
-      end
-   end
 
-   if level > 4 then config:print() end
+	 function revert()
+	    awall.iptables.revert()
+	    os.exit(1)
+	 end
 
-elseif mode == 'translate' then
-   if verify then config:test() end
-   config:dump(outputdir)
-   
-elseif mode == 'activate' then
-
-   if not force then
-      awall.iptables.backup()
-
-      signal.signal('SIGCHLD',
-		    function()
-		       if pid and lpc.wait(pid, 1) then os.exit(2) end
-		    end)
-      for i, sig in ipairs({'INT', 'TERM'}) do
-	 signal.signal('SIG'..sig, function()
-				      interrupted = true
-				      io.stdin:close()
-				   end)
-      end
+	 if awall.uerror.call(config.activate, config) then
 
-      require 'lpc'
-      pid, stdio, stdout = lpc.run(arg[0], 'fallback')
-      stdio:close()
-      stdout:close()
-   end
-   
-   config:activate()
+	    if not force then
+	       io.stderr:write('New firewall configuration activated\n')
+	       io.stderr:write('Press RETURN to commit changes permanently: ')
+	       interrupted = not io.read()
 
-   if not force then
-      io.stderr:write('New firewall configuration activated\n')
-      io.stderr:write('Press RETURN to commit changes permanently: ')
-      interrupted = not io.read()
+	       kill()
 
-      signal.signal('SIGCHLD', 'default')
-      signal.kill(pid, 'SIGTERM')
-      lpc.wait(pid)
-   end
+	       if interrupted then
+		  io.stderr:write(
+		     '\nActivation canceled, reverting to the old configuration\n'
+		  )
+		  revert()
+	       end
+	    end
 
-   if interrupted then
-      io.stderr:write('\nActivation canceled, reverting to the old configuration\n')
-      awall.iptables.revert()
+	    config:dump()
 
-   else config:dump() end
+	 else
+	    if not force then kill() end
+	    revert()
+	 end
 
 
-elseif mode == 'fallback' then
+      elseif mode == 'fallback' then
+   
+	 for i, sig in ipairs({'HUP', 'PIPE'}) do
+	    signal.signal('SIG'..sig, function() end)
+	 end
 
-   for i, sig in ipairs({'HUP', 'PIPE'}) do
-      signal.signal('SIG'..sig, function() end)
-   end
+	 require 'lsleep'
+	 lsleep.sleep(10)
 
-   require 'lsleep'
-   lsleep.sleep(10)
+	 io.stderr:write('\nTimeout, reverting to the old configuration\n')
+	 awall.iptables.revert()
 
-   io.stderr:write('\nTimeout, reverting to the old configuration\n')
-   awall.iptables.revert()
+      elseif mode == 'flush' then awall.iptables.flush()
 
-elseif mode == 'flush' then awall.iptables.flush()
+      else assert(false) end
 
-else assert(false) end
+   end
+) then os.exit(1) end
diff --git a/awall/iptables.lua b/awall/iptables.lua
index 02536e8..32b59b2 100644
--- a/awall/iptables.lua
+++ b/awall/iptables.lua
@@ -1,6 +1,6 @@
 --[[
 Iptables file dumper for Alpine Wall
-Copyright (C) 2012 Kaarle Ritvanen
+Copyright (C) 2012-2013 Kaarle Ritvanen
 Licensed under the terms of GPL2
 ]]--
 
@@ -10,6 +10,7 @@ module(..., package.seeall)
 require 'lpc'
 
 require 'awall.object'
+require 'awall.uerror'
 require 'awall.util'
 
 local class = awall.object.class
@@ -70,7 +71,7 @@ function BaseIPTables:restore(test)
       end
    end
 
-   if disabled then error('Firewall not enabled in kernel') end
+   if disabled then awall.uerror.raise('Firewall not enabled in kernel') end
 end
 
 function BaseIPTables:activate()
diff --git a/awall/model.lua b/awall/model.lua
index c904a53..1746c7a 100644
--- a/awall/model.lua
+++ b/awall/model.lua
@@ -12,6 +12,7 @@ require 'awall.host'
 require 'awall.iptables'
 require 'awall.object'
 require 'awall.optfrag'
+require 'awall.uerror'
 require 'awall.util'
 
 local util = awall.util
@@ -43,7 +44,9 @@ function ConfigObject:create(cls, params)
    return cls.morph(params, self.context, self.location)
 end
 
-function ConfigObject:error(msg) error(self.location..': '..msg) end
+function ConfigObject:error(msg)
+   awall.uerror.raise(self.location..': '..msg)
+end
 
 function ConfigObject:warning(msg)
    io.stderr:write(self.location..': '..msg..'\n')
diff --git a/awall/policy.lua b/awall/policy.lua
index 2ecfdc2..396a3a6 100644
--- a/awall/policy.lua
+++ b/awall/policy.lua
@@ -12,9 +12,8 @@ require 'lpc'
 
 require 'awall.dependency'
 require 'awall.object'
-require 'awall.util'
-
-local util = awall.util
+local raise = require('awall.uerror').raise
+local util = require('awall.util')
 
 
 local PolicyConfig = awall.object.class()
@@ -37,18 +36,18 @@ function PolicyConfig:expand()
 	 local si, ei, name = string.find(value, pattern)
 	 
 	 if util.contains(visited, name) then
-	    error('Circular variable definition: '..name)
+	    raise('Circular variable definition: '..name)
 	 end
 	 table.insert(visited, name)
 	 
 	 local var = self.data.variable[name]
-	 if not var then error('Invalid variable reference: '..name) end
+	 if not var then raise('Invalid variable reference: '..name) end
 	 
 	 if si == 1 and ei == string.len(value) then value = var
 	 elseif util.contains({'number', 'string'}, type(var)) then
 	    value = string.sub(value, 1, si - 1)..var..string.sub(value, ei + 1, -1)
 	 else
-	    error('Attempted to concatenate complex variable: '..name)
+	    raise('Attempted to concatenate complex variable: '..name)
 	 end
       end
 
@@ -63,7 +62,7 @@ end
 
 local function open(name, dirs)
    if not string.match(name, '^[%w-]+$') then
-      error('Invalid characters in policy name: '..name)
+      raise('Invalid characters in policy name: '..name)
    end
    for i, dir in ipairs(dirs) do
       local path = dir..'/'..name..'.json'
@@ -90,7 +89,7 @@ local function list(dirs)
 	 local si, ei, name = string.find(fname, '^([%w-]+)%.json$')
 	 if name then
 	    if util.contains(allnames, name) then
-	       error('Duplicate policy name: '..name)
+	       raise('Duplicate policy name: '..name)
 	    end
 	    table.insert(allnames, name)
 
@@ -126,7 +125,7 @@ function PolicySet:loadJSON(name, fname)
    else
       file, fname = open(name, self.importdirs)
    end
-   if not file then error('Unable to read policy file '..fname) end
+   if not file then raise('Unable to read policy file '..fname) end
 
    local data = ''
    for line in file:lines() do data = data..line end
@@ -134,7 +133,7 @@ function PolicySet:loadJSON(name, fname)
 
    local success, res = pcall(json.decode, data)
    if success then return res end
-   error(res..' while parsing '..fname)
+   raise(res..' while parsing '..fname)
 end
 
 
@@ -157,7 +156,7 @@ function PolicySet:load()
 
    local order = awall.dependency.order(policies)
    if type(order) ~= 'table' then
-      error('Circular ordering directives: '..order)
+      raise('Circular ordering directives: '..order)
    end
 
 
@@ -196,16 +195,16 @@ end
 function PolicySet:findsymlink(name)
    local symlink = find(name, {self.confdir})
    if symlink and lfs.symlinkattributes(symlink).mode ~= 'link' then
-      error('Not an optional policy: '..name)
+      raise('Not an optional policy: '..name)
    end
    return symlink
 end
 
 function PolicySet:enable(name)
-   if self:findsymlink(name) then error('Policy already enabled: '..name)
+   if self:findsymlink(name) then raise('Policy already enabled: '..name)
    else
       local target = find(name, self.importdirs)
-      if not target then error('Policy not found: '..name) end
+      if not target then raise('Policy not found: '..name) end
       if string.sub(target, 1, 1) ~= '/' then
 	 target = lfs.currentdir()..'/'..target
       end
@@ -219,7 +218,7 @@ end
 
 function PolicySet:disable(name)
    local symlink = self:findsymlink(name)
-   if not symlink then error('Policy not enabled: '..name) end
+   if not symlink then raise('Policy not enabled: '..name) end
    assert(os.remove(symlink))
 end
 
diff --git a/awall/uerror.lua b/awall/uerror.lua
new file mode 100644
index 0000000..a3df48c
--- /dev/null
+++ b/awall/uerror.lua
@@ -0,0 +1,23 @@
+--[[
+User error handling for Alpine Wall
+Copyright (C) 2012-2013 Kaarle Ritvanen
+Licensed under the terms of GPL2
+]]--
+
+module(..., package.seeall)
+
+local prefix = 'awall user error: '
+
+function raise(msg) error(prefix..msg) end
+
+function call(f, ...)
+   return xpcall(
+      function() f(unpack(arg)) end,
+      function(msg)
+	 local si, ei = string.find(msg, prefix, 1, true)
+   	 if si then msg = 'awall: '..string.sub(msg, ei + 1, -1) end
+   	 io.stderr:write(msg..'\n')
+   	 if not si then io.stderr:write(debug.traceback()..'\n') end
+      end
+   )
+end