Skip to content
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

Combine options outside of Z3 #49

Closed

Conversation

vitorenesduarte
Copy link

@vitorenesduarte vitorenesduarte commented Nov 16, 2022

This PR is the first step towards #43.

It only focus on the following options for now:

  • max_session_ttl
  • recording_mode
  • source_ip

The idea is to gather feedback on the general approach implemented here before implementing the remaining options.

From option inequalities to a simple struct

Before this PR, options were specified using inequalities. Instead now we have:

    p = Policy(
        name="access",
        options=Options(
            max_session_ttl=Duration.new(hours=10),
            recording_mode=RecordingMode.STRICT,
            source_ip=SourceIp.PINNED,
        ),
        allow=Rules(
            AccessNode(
                (User.traits["team"] == ("admins",))
            ),
        ),
    )

With predictable option combining (#43) there's no need to say max_session_ttl < 10h to try to imply that the min will be chosen in case two of these are combined.

Combining two of these inequalities using logical AND also doesn't seem to make sense.
If we say max_session_ttl < 10 AND max_session_ttl < 3, this is equivalent to max_session_ttl < 3
When Z3 solves this (see the solver proposed in #27) it will output max_session_ttl == 0 as that's a valid solution.

In general, saying max_session_ttl < 3 could be interpreted as "max_session_ttl can have any value smaller than 3", but this is not exactly what we want it to mean.

Option combining

Option combining is implemented outside of Z3 as it's very easy to do so (e.g. just call min).

export output

As options are not defined using inequalities, the export command now outputs them as a list.

predicate export examples/access.py outputs the following:

kind: policy
metadata:
  name: access
spec:
  allow:
    access_node: (((access_node.login == user.name) && (!(user.name == "root"))) ||
      equals(user.traits["team"], ["admins"]))
  deny:
    access_node: (((access_node.login == "mike") || (access_node.login == "jester"))
      || (node.labels["env"] == "prod"))
  options:
    max_session_ttl: 36000000000000
    recording_mode: strict
    source_ip: pinned
version: v1

Other changes

While working on this I made some general improvements:

  • ran linter (fixed issues and formatted the code)
  • added linter to CI (hit ignore-missing-import option does not work on false positive "Library stubs not installed for X" python/mypy#10632, which took many iterations to solve)
  • simplified the README so that it points to the examples folder and doesn't contain outdated information
  • ran poetry update which updated poetry.lock
  • removed some unused functions
  • added types to some functions
  • added a TeleportSolverResult that encapsulates the outcomes of the solver
  • removed LtDuration as it's no longer needed
  • simplified build_predicate

@vitorenesduarte vitorenesduarte requested review from klizhentas and xacrimon and removed request for klizhentas November 16, 2022 15:13
@vitorenesduarte vitorenesduarte self-assigned this Nov 16, 2022
Base automatically changed from vitor/access-node-action to sasha/python November 16, 2022 17:13
@vitorenesduarte vitorenesduarte force-pushed the vitor/options-combining-outside-z3 branch from 0f2b215 to 77ebc8d Compare November 16, 2022 17:17
Copy link
Collaborator

@klizhentas klizhentas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitorenesduarte

  • please use the Z3 solver instead of custom solver here. Using Z3 solver makes it impossible to create proper query. You can build your own solver using max strategy so 0 won't be a solution. You have to find a maximum value that satisfies both equations.

  • please use inequalities as they demonstrate the intent much better and you can query - with more granularity, for example, you can say, is it possible to get a certificate less than 10 minutes? With inequalities and Z3 solver the answer will be correct, with your proposed solution it is not possible.

  • Write a test example with the case above to demonstrate the benefit.

@xacrimon
Copy link

Close for now

@xacrimon xacrimon closed this Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants