-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sesearch: CIL output #134
sesearch: CIL output #134
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,50 +78,77 @@ cdef class Conditional(PolicyObject): | |
return other in self.booleans | ||
|
||
def __str__(self): | ||
# sepol representation is in postfix notation. This code | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and in other places, where there isn't a statement, it should be handled using format(), also with a default of kernel output. Please support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking more about your example output, this is not meant to output a full conditional block, so it should not be rendering a full booleanif/(true/(false block in the cil output. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That looks nice, thanks for the pointer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The purpose and use case of the CIL output flag (which I should have stated in the patch description) is that when the output is set to CIL, I can copy and use the text mechanically without modification in other CIL policies. It's also possible to compare the original CIL policy and the output of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is the intent: to succinctly return the results. Not the intent: copy the result verbatim to a policy source file. If I have 50 conditional results it would be a mess. The kernel language was chosen for rendering because SETools predates CIL. SETools did not change to CIL rendering because it is an intermediate language that most don't even see. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't mean to criticize KPL or the choices made. These days I prefer to operate closer to metal with CIL. But since CIL output is a new feature, we could choose to be more verbose, to be more accurate and to be fully compatible with the CIL syntax. If the user wants a compact representation, the default KPL output should be great for that purpose. Perhaps there could be even several output variants, like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since CIL is an intermediate language, I don't feel the need to extensively support it as you describe. |
||
# converts it to infix notation. Parentheses are added | ||
# to ensure correct expressions, though they may end up | ||
# being overused. Set previous operator at start to the | ||
# highest precedence (NOT) so if there is a single binary | ||
# operator, no parentheses are output | ||
stack = [] | ||
prev_op_precedence = 5 | ||
|
||
for expr_node in self.expression(): | ||
if isinstance(expr_node, Boolean): | ||
# append the boolean name | ||
stack.append(str(expr_node)) | ||
elif expr_node.unary: | ||
operand = stack.pop() | ||
operator = str(expr_node) | ||
op_precedence = expr_node.precedence | ||
|
||
# NOT is the highest precedence, so only need | ||
# parentheses if the operand is a subexpression | ||
if isinstance(operand, list): | ||
subexpr = [operator, "(", operand, ")"] | ||
if self.policy.gen_cil: | ||
# sepol representation is in postfix notation. This code | ||
# converts it to prefix notation used by CIL. Parentheses | ||
# are always needed. | ||
stack = [] | ||
|
||
for expr_node in self.expression(): | ||
if isinstance(expr_node, Boolean): | ||
# append the boolean name | ||
stack.append(str(expr_node)) | ||
elif expr_node.unary: | ||
operand = stack.pop() | ||
operator = str(expr_node) | ||
|
||
subexpr = ["(", operator, " ", operand, ")"] | ||
|
||
stack.append(subexpr) | ||
else: | ||
subexpr = [operator, operand] | ||
operand1 = stack.pop() | ||
operand2 = stack.pop() | ||
operator = str(expr_node) | ||
|
||
stack.append(subexpr) | ||
prev_op_precedence = op_precedence | ||
else: | ||
operand1 = stack.pop() | ||
operand2 = stack.pop() | ||
operator = str(expr_node) | ||
op_precedence = expr_node.precedence | ||
subexpr = ["(", operator, " ", operand1, " ", operand2, ")"] | ||
|
||
stack.append(subexpr) | ||
return ''.join(flatten_list(stack)) | ||
|
||
if prev_op_precedence > op_precedence: | ||
# if previous operator is of higher precedence | ||
# no parentheses are needed. | ||
subexpr = [operand1, operator, operand2] | ||
else: | ||
# This code converts sepol to infix notation. Parentheses are | ||
# added to ensure correct expressions, though they may end up | ||
# being overused. Set previous operator at start to the | ||
# highest precedence (NOT) so if there is a single binary | ||
# operator, no parentheses are output | ||
stack = [] | ||
prev_op_precedence = 5 | ||
|
||
for expr_node in self.expression(): | ||
if isinstance(expr_node, Boolean): | ||
# append the boolean name | ||
stack.append(str(expr_node)) | ||
elif expr_node.unary: | ||
operand = stack.pop() | ||
operator = str(expr_node) | ||
op_precedence = expr_node.precedence | ||
|
||
# NOT is the highest precedence, so only need | ||
# parentheses if the operand is a subexpression | ||
if isinstance(operand, list): | ||
subexpr = [operator, "(", operand, ")"] | ||
else: | ||
subexpr = [operator, operand] | ||
|
||
stack.append(subexpr) | ||
prev_op_precedence = op_precedence | ||
else: | ||
subexpr = ["(", operand1, operator, operand2, ")"] | ||
operand1 = stack.pop() | ||
operand2 = stack.pop() | ||
operator = str(expr_node) | ||
op_precedence = expr_node.precedence | ||
|
||
if prev_op_precedence > op_precedence: | ||
# if previous operator is of higher precedence | ||
# no parentheses are needed. | ||
subexpr = [operand1, operator, operand2] | ||
else: | ||
subexpr = ["(", operand1, operator, operand2, ")"] | ||
|
||
stack.append(subexpr) | ||
prev_op_precedence = op_precedence | ||
stack.append(subexpr) | ||
prev_op_precedence = op_precedence | ||
|
||
return ' '.join(flatten_list(stack)) | ||
return ' '.join(flatten_list(stack)) | ||
|
||
def __hash__(self): | ||
return hash(self.key) | ||
|
@@ -222,6 +249,7 @@ cdef class ConditionalOperator(PolicyObject): | |
|
||
cdef: | ||
str text | ||
str cil_text | ||
readonly int precedence | ||
# T/F the operator is unary | ||
readonly bint unary | ||
|
@@ -231,12 +259,12 @@ cdef class ConditionalOperator(PolicyObject): | |
readonly object evaluate | ||
|
||
_cond_expr_val_to_details = { | ||
sepol.COND_NOT: ("!", 5, lambda x: not x), | ||
sepol.COND_OR: ("||", 1, lambda x, y: x or y), | ||
sepol.COND_AND: ("&&", 3, lambda x, y: x and y), | ||
sepol.COND_XOR: ("^", 2, lambda x, y: x ^ y), | ||
sepol.COND_EQ: ("==", 4, lambda x, y: x == y), | ||
sepol.COND_NEQ: ("!=", 4, lambda x, y: x != y)} | ||
sepol.COND_NOT: ("!", "not", 5, lambda x: not x), | ||
sepol.COND_OR: ("||", "or", 1, lambda x, y: x or y), | ||
sepol.COND_AND: ("&&", "and", 3, lambda x, y: x and y), | ||
sepol.COND_XOR: ("^", "xor", 2, lambda x, y: x ^ y), | ||
sepol.COND_EQ: ("==", "eq", 4, lambda x, y: x == y), | ||
sepol.COND_NEQ: ("!=", "ne", 4, lambda x, y: x != y)} | ||
|
||
@staticmethod | ||
cdef inline ConditionalOperator factory(SELinuxPolicy policy, sepol.cond_expr_t *symbol): | ||
|
@@ -245,11 +273,14 @@ cdef class ConditionalOperator(PolicyObject): | |
op.policy = policy | ||
op.key = <uintptr_t>symbol | ||
op.unary = symbol.expr_type == sepol.COND_NOT | ||
op.text, op.precedence, op.evaluate = op._cond_expr_val_to_details[symbol.expr_type] | ||
op.text, op.cil_text, op.precedence, op.evaluate = op._cond_expr_val_to_details[symbol.expr_type] | ||
return op | ||
|
||
def __str__(self): | ||
return self.text | ||
if self.policy.gen_cil: | ||
return self.cil_text | ||
else: | ||
return self.text | ||
|
||
def statement(self): | ||
raise NoStatement | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,10 @@ cdef class MLSRule(PolicyRule): | |
yield self | ||
|
||
def statement(self): | ||
return f"{self.ruletype} {self.source} {self.target}:{self.tclass} {self.default};" | ||
if self.policy.gen_cil: | ||
return f"(rangetransition {self.source} {self.target} {self.tclass} {self.default})" | ||
else: | ||
return f"{self.ruletype} {self.source} {self.target}:{self.tclass} {self.default};" | ||
Comment on lines
-70
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other places in the change should have separate methods for each policy lang, unless they are simple one-liners like this. |
||
|
||
|
||
# | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CIL is not a property of the policy. It should be a keyword parameter to the statement() method of the individual classes, with a default of the kernel (current) output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually my first version used a similar method, the flag was stored as
PolicyObject::gen_cil
, set in sesearch near output stage. The problem I had with that approach was that a lot of effort was needed to make sure the flag was copied to all objects and that gets ugly. I never found out why the output in some case wasn't CIL. Also,statement()
method is not used directly in a lot of places but__str__()
instead. Perhaps just my poor Python skills are showing.I'd even argue that while CIL output is indeed not a property of a policy, it's pretty much a global flag. There should never be a need for mixing CIL and kernel output.
But I'll try the
format()
method next, perhaps the format option gets passed around more easily,There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I hand you a
policy.33
with no context whatsoever and ask you if it's source files were CIL, you can't answer because it's not a property of the policy.