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

Expected command pattern instead of strict and relaxed alignment #46

Closed
lukpueh opened this issue Nov 18, 2016 · 2 comments
Closed

Expected command pattern instead of strict and relaxed alignment #46

lukpueh opened this issue Nov 18, 2016 · 2 comments

Comments

@lukpueh
Copy link
Member

lukpueh commented Nov 18, 2016

Currently we pass command verification upon strict (must be equal) and relaxed (must have an equal prefix) alignment of recorded command and expected command. The following examples show in what cases the alignment passes or fails.

# Left command is the command recorded by toto-run.py
# Right command is the expected command defined in the layout

# Passes:
["vi", "file1", "file2"] ~ ["vi", "file1", "file2"]

# Passes with warning:
["vi", "file1", "file2"] ~  ["vi"]
["make", "install"] ~  ["make"]

# Fails:
["vi", "file1", "file2"] ~ ["make", "install"]
["/usr/bin/vi", "file1"] ~ []
["vi", "file1", "file2"] ~ ["vi", "file2", "file1"]
["/usr/bin/vi", "file1"] ~ ["vi", "file1"]

As one can see in above examples that semantically equal commands sometimes fail whereas syntactically (relaxed) equal commands can pass although they have very different semantics.

@vladimir-v-diaz brought up the idea of using match patterns. We could use regex or a subset of regex like implemented by fnmatch. This would allow either fail or pass and get rid of the relaxed pass.

In any case command alignment does not provide strong security guarantees, since recorded commands can be easily faked using alias, symlinks or the like.

@JustinCappos
Copy link
Collaborator

I think that just recording what was run and having the client software
eventually warn (error?) is perhaps the best policy. Getting too advanced
with this probably isn't worth the time. So, simply doing a strict check
may be best. Yes, we know someone could bypass this, but it prevents many
types of accidental 'foot-shooting'... :)

On Fri, Nov 18, 2016 at 12:42 PM, lukpueh notifications@github.com wrote:

Currently we pass command verification upon strict (must be equal) and
relaxed (must have an equal prefix) alignment of recorded command and
expected command. The following examples show in what cases the alignment
passes or fails.

Left command is the command recorded by toto-run.py# Right command is the expected command defined in the layout

Passes:

["vi", "file1", "file2"] ~ ["vi", "file1", "file2"]

Passes with warning:

["vi", "file1", "file2"] ~ ["vi"]
["make", "install"] ~ ["make"]

Fails:

["vi", "file1", "file2"] ~ ["make", "install"]
["/usr/bin/vi", "file1"] ~ []
["vi", "file1", "file2"] ~ ["vi", "file2", "file1"]
["/usr/bin/vi", "file1"] ~ ["vi", "file1"]

As one can see in above examples that semantically equal commands
sometimes fail whereas syntactically (relaxed) equal commands can pass
although they have very different semantics.

@vladimir-v-diaz https://github.com/vladimir-v-diaz brought up the idea
of using match patterns. We could use regex
https://docs.python.org/2/library/re.html or a subset of regex like
implemented by fnmatch https://docs.python.org/2/library/fnmatch.html.
This would allow either fail or pass and get rid of the relaxed pass.

In any case command alignment does not provide strong security guarantees,
since recorded commands can be easily faked using alias, symlinks or the
like.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#46, or mute the thread
https://github.com/notifications/unsubscribe-auth/AA0XD8MZKGblomNXXmoomAKftmCkXU7rks5q_eOPgaJpZM4K2vhY
.

@lukpueh
Copy link
Member Author

lukpueh commented Nov 21, 2016

Sounds good to me.

lukpueh added a commit that referenced this issue Nov 30, 2016
In a previous commit we added a custom CommandAlignmentFailed
exception and replaced log with print in case of soft fail.
This commit removes the custom exception (1) and changes print back
to log (2) for the following reasons.

(1) We decided use log throughout the library also for user feedback
because it is more powerful and can easily be toggled
programatically, see discussion
#6 (comment)

(2) We decided to never hard fail command alignment because the check
is of informational character rather than a security guarantee. See
#46

The commit also adds unit tests for command alignment
lukpueh added a commit that referenced this issue Dec 1, 2016
In a previous commit we added a custom CommandAlignmentFailed
exception and replaced log with print in case of soft fail.
This commit removes the custom exception (1) and changes print back
to log (2) for the following reasons.

(1) We decided use log throughout the library also for user feedback
because it is more powerful and can easily be toggled
programatically, see discussion
#6 (comment)

(2) We decided to never hard fail command alignment because the check
is of informational character rather than a security guarantee. See
#46

The commit also adds unit tests for command alignment
@lukpueh lukpueh closed this as completed Dec 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants