minijail: Make denylist compiler return Errno
Make 2 big changes to the denylist compiler. Instead of '0' and '!'
indicating a denylist policy, have an explicit header, '@denylist' in a
policy to indicate a denylist policy. Also, require denylist policies to
specifically define returning a specific Errno for each syscall entry.
Bug: None
Test: Fix tests and run unit tests
Change-Id: Iea2d94b67df8362b7268b08ff8fe8a814b5e0db1
diff --git a/tools/compiler.py b/tools/compiler.py
index a0e5183..e7ab842 100644
--- a/tools/compiler.py
+++ b/tools/compiler.py
@@ -289,7 +289,8 @@
visitor = bpf.FlatteningVisitor(
arch=self._arch, kill_action=kill_action)
if denylist:
- accept_action = kill_action
+ # Default action for a denylist policy is return EPERM
+ accept_action = bpf.ReturnErrno(self._arch.constants['EPERM'])
reject_action = bpf.Allow()
else:
accept_action = bpf.Allow()
diff --git a/tools/parser.py b/tools/parser.py
index b7e5f69..9e8dcd5 100644
--- a/tools/parser.py
+++ b/tools/parser.py
@@ -42,6 +42,7 @@
('DEFAULT', r'@default\b'),
('INCLUDE', r'@include\b'),
('FREQUENCY', r'@frequency\b'),
+ ('DENYLIST', r'@denylist$'),
('PATH', r'(?:\.)?/\S+'),
('NUMERIC_CONSTANT', r'-?0[xX][0-9a-fA-F]+|-?0[Oo][0-7]+|-?[0-9]+'),
('COLON', r':'),
@@ -59,7 +60,6 @@
('BITWISE_OR', r'\|'),
('OP', r'&|\bin\b|==|!=|<=|<|>=|>'),
('EQUAL', r'='),
- ('NOT', r'!'),
('ARGUMENT', r'\barg[0-9]+\b'),
('RETURN', r'\breturn\b'),
('ACTION',
@@ -401,7 +401,7 @@
# action = 'allow' | '1'
# | 'kill-process'
# | 'kill-thread'
- # | 'kill' | '0'
+ # | 'kill'
# | 'trap'
# | 'trace'
# | 'log'
@@ -412,9 +412,9 @@
if not tokens:
self._parser_state.error('missing action')
action_token = tokens.pop(0)
- # The only valid denylist action is 0
+ # denylist policies must specify a return for every line.
if self._denylist:
- if action_token.type != 'NUMERIC_CONSTANT':
+ if action_token.type != 'RETURN':
self._parser_state.error('invalid denylist policy')
if action_token.type == 'ACTION':
@@ -437,13 +437,7 @@
elif action_token.type == 'NUMERIC_CONSTANT':
constant = self._parse_single_constant(action_token)
if constant == 1:
- if self._denylist:
- self._parser_state.error('invalid denylist policy')
return bpf.Allow()
- if constant == 0:
- if not self._denylist:
- self._parser_state.error('invalid allowlist policy')
- return self._kill_action
elif action_token.type == 'RETURN':
if not tokens:
self._parser_state.error('missing return value')
@@ -458,23 +452,16 @@
if not tokens:
self._parser_state.error('missing filter')
if tokens[0].type == 'ARGUMENT':
- if self._denylist:
- self._parser_state.error('invalid denylist policy')
- action = bpf.Allow()
- elif tokens[0].type == 'NOT':
- if not self._denylist:
- self._parser_state.error('invalid allowlist policy')
- tokens.pop(0)
- action = self._kill_action
+ # Only argument expressions can start with an ARGUMENT token.
+ argument_expression = self.parse_argument_expression(tokens)
+ if tokens and tokens[0].type == 'SEMICOLON':
+ tokens.pop(0)
+ action = self.parse_action(tokens)
+ else:
+ action = bpf.Allow()
+ return Filter(argument_expression, action)
else:
return Filter(None, self.parse_action(tokens))
- # Parse an argument expression, either allowlist or denylist.
- # Only argument expressions can start with an ARGUMENT token.
- argument_expression = self.parse_argument_expression(tokens)
- if tokens and tokens[0].type == 'SEMICOLON':
- tokens.pop(0)
- action = self.parse_action(tokens)
- return Filter(argument_expression, action)
# filter = '{' , single-filter , [ { ',' , single-filter } ] , '}'
# | single-filter
@@ -722,6 +709,7 @@
self._parser_states.append(ParserState(filename))
try:
statements = []
+ denylist_header = False
with open(filename) as policy_file:
for tokens in self._parser_state.tokenize(policy_file):
if tokens[0].type == 'INCLUDE':
@@ -735,6 +723,14 @@
elif tokens[0].type == 'DEFAULT':
self._default_action = self._parse_default_statement(
tokens)
+ elif tokens[0].type == 'DENYLIST':
+ tokens.pop()
+ if not self._denylist:
+ self._parser_state.error('policy is denylist, but '
+ 'flag --denylist not '
+ 'passed in.')
+ else:
+ denylist_header = True
else:
statement = self.parse_filter_statement(tokens)
if statement is None:
@@ -746,6 +742,9 @@
if tokens:
self._parser_state.error(
'extra tokens', token=tokens[0])
+ if self._denylist and not denylist_header:
+ self._parser_state.error('policy must contain @denylist flag to'
+ ' be compiled with --denylist flag.')
return statements
finally:
self._parser_states.pop()
diff --git a/tools/parser_unittest.py b/tools/parser_unittest.py
index 0aa17e1..72eee54 100755
--- a/tools/parser_unittest.py
+++ b/tools/parser_unittest.py
@@ -58,14 +58,12 @@
('INCLUDE', '@include'),
('PATH', './minijail.policy'),
])
- """A ! is a valid symbol for tokenizing no matter the policy type."""
self.assertEqual(
[(token.type, token.value) for token in TokenizerTests._tokenize(
- 'read: ! arg0 in ~0xffff || arg0 & (1|2) && arg0 == 0o755; '
+ 'read: arg0 in ~0xffff || arg0 & (1|2) && arg0 == 0o755; '
'return ENOSYS # ignored')], [
('IDENTIFIER', 'read'),
('COLON', ':'),
- ('NOT', '!'),
('ARGUMENT', 'arg0'),
('OP', 'in'),
('BITWISE_COMPLEMENT', '~'),
@@ -396,18 +394,6 @@
with self.assertRaisesRegex(parser.ParseException, 'unclosed brace'):
self.parser.parse_filter(self._tokenize('{ allow'))
- def test_parse_allowlist_with_not(self):
- """Reject a policy with a !"""
- with self.assertRaisesRegex(parser.ParseException,
- 'invalid allowlist policy'):
- self.parser.parse_filter(self._tokenize('! allow'))
-
- def test_parse_allowlist_with_zero(self):
- """Reject a policy with a 0"""
- with self.assertRaisesRegex(parser.ParseException,
- 'invalid allowlist policy'):
- self.parser.parse_filter(self._tokenize('0'))
-
class ParseFilterDenylistTests(unittest.TestCase):
"""Tests for PolicyParser.parse_filter with a denylist policy."""
@@ -423,27 +409,14 @@
return list(self.parser._parser_state.tokenize([line]))[0]
def test_parse_filter(self):
- """Accept only filters prefaced by a ! or just the token 0."""
+ """Accept only filters that return an errno."""
self.assertEqual(
- self.parser.parse_filter(self._tokenize('! arg0 == 0')), [
- parser.Filter([[parser.Atom(0, '==', 0)]], self.kill_action),
- ])
- self.assertEqual(
- self.parser.parse_filter(self._tokenize('0')), [
- parser.Filter(None, self.kill_action),
+ self.parser.parse_filter(self._tokenize('arg0 == 0; return ENOSYS')),
+ [
+ parser.Filter([[parser.Atom(0, '==', 0)]],
+ bpf.ReturnErrno(self.arch.constants['ENOSYS'])),
])
- def test_parse_allowlist_without_not(self):
- """Reject a denylist policy without a !"""
- with self.assertRaisesRegex(parser.ParseException,
- 'invalid denylist policy'):
- self.parser.parse_filter(self._tokenize('allow'))
-
- def test_parse_allowlist_with_zero(self):
- """Reject a denylistpolicy with a 1"""
- with self.assertRaisesRegex(parser.ParseException,
- 'invalid denylist policy'):
- self.parser.parse_filter(self._tokenize('1'))
class ParseFilterStatementTests(unittest.TestCase):
"""Tests for PolicyParser.parse_filter_statement."""
@@ -918,6 +891,18 @@
r'applied')):
self.parser.parse_file(path)
+ def test_parse_allowlist_denylist_header(self):
+ """Reject trying to compile denylist policy file as allowlist."""
+ with self.assertRaisesRegex(parser.ParseException,
+ r'policy is denylist, but flag --denylist '
+ 'not passed in'):
+ path = self._write_file(
+ 'test.policy', """
+ @denylist
+ """)
+ self.parser.parse_file(path)
+
+
class ParseFileDenylistTests(unittest.TestCase):
"""Tests for PolicyParser.parse_file."""
@@ -943,8 +928,9 @@
path = self._write_file(
'test.policy', """
# Comment.
- read: 0
- write: 0
+ @denylist
+ read: return ENOSYS
+ write: return ENOSYS
""")
self.assertEqual(
@@ -956,13 +942,15 @@
syscall=parser.Syscall('read', 0),
frequency=1,
filters=[
- parser.Filter(None, self.kill_action),
+ parser.Filter(None, bpf.ReturnErrno(
+ self.arch.constants['ENOSYS'])),
]),
parser.FilterStatement(
syscall=parser.Syscall('write', 1),
frequency=1,
filters=[
- parser.Filter(None, self.kill_action),
+ parser.Filter(None, bpf.ReturnErrno(
+ self.arch.constants['ENOSYS'])),
]),
]))
@@ -971,8 +959,9 @@
path = self._write_file(
'test.policy', """
# Comment.
- read: 0
- write: ! arg0 == 0
+ @denylist
+ read: return ENOSYS
+ write: arg0 == 0 ; return ENOSYS
""")
self.assertEqual(
@@ -984,17 +973,30 @@
syscall=parser.Syscall('read', 0),
frequency=1,
filters=[
- parser.Filter(None, self.kill_action),
+ parser.Filter(None, bpf.ReturnErrno(
+ self.arch.constants['ENOSYS'])),
]),
parser.FilterStatement(
syscall=parser.Syscall('write', 1),
frequency=1,
filters=[
parser.Filter([[parser.Atom(0, '==', 0)]],
- self.kill_action),
+ bpf.ReturnErrno(self.arch.constants['ENOSYS'])),
parser.Filter(None, bpf.Allow()),
]),
]))
+
+ def test_parse_denylist_no_header(self):
+ """Reject trying to compile denylist policy file as allowlist."""
+ with self.assertRaisesRegex(parser.ParseException,
+ r'policy must contain @denylist flag to be '
+ 'compiled with --denylist flag'):
+ path = self._write_file(
+ 'test.policy', """
+ read: return ENOSYS
+ """)
+ self.parser.parse_file(path)
+
if __name__ == '__main__':
unittest.main()
