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

Fix cmd match stringy #12

Closed
wants to merge 3 commits into from
Closed

Fix cmd match stringy #12

wants to merge 3 commits into from

Conversation

RollerMatic
Copy link

@RollerMatic RollerMatic commented Oct 15, 2024

This PR fixes the regex matching algorithm in tacquito's default authorizer by adding boundary conditions around the contents of the match attribute of commands.

@RollerMatic
Copy link
Author

Checks

Tests pass

~/github/tacquito ❯  go test
PASS
ok      github.com/facebookincubator/tacquito   2.048s

Functional testing

before

> ./client -mode authz  --username cisco --cmd "bash /dev/console/file_contains_ls.sh"
2024/10/15 13:16:14 executing static password mode
(*tacquito.AuthorReply)(0xc0000fd9c0)({
 Status: (tacquito.AuthorStatus) AuthorStatusPassAdd,
 Args: (tacquito.Args) ,
 ServerMsg: (tacquito.AuthorServerMsg) (len=22) authorization approved,
 Data: (tacquito.AuthorData)
})

after

expected to deny: command does not match patterns allowed for the user

❯ ./client -mode authz  --username cisco --cmd "bash /dev/console/file_contains_ls.sh"
2024/10/15 12:52:34 executing static password mode
2024/10/15 12:52:35 FAILED: args [service=shell, cmd=bash, cmd-arg=/dev/console/file_contains_ls.sh, cmd-arg=<cr>, ]; expected a [AuthorStatusPassAdd] but got a [AuthorStatusFail]
(*tacquito.AuthorReply)(0xc0000ff3c0)({
 Status: (tacquito.AuthorStatus) AuthorStatusFail,
 Args: (tacquito.Args) ,
 ServerMsg: (tacquito.AuthorServerMsg) (len=14) not authorized,
 Data: (tacquito.AuthorData)
})

expected to pass: "cisco" user can run commands that match regex ls.*

❯ ./client -mode authz  --username cisco --cmd "bash ls /dev/console"
2024/10/15 12:52:45 executing static password mode
(*tacquito.AuthorReply)(0xc00078ccc0)({
 Status: (tacquito.AuthorStatus) AuthorStatusPassAdd,
 Args: (tacquito.Args) ,
 ServerMsg: (tacquito.AuthorServerMsg) ,
 Data: (tacquito.AuthorData)
})

@RollerMatic RollerMatic marked this pull request as ready for review October 15, 2024 20:24
@facebook-github-bot
Copy link
Contributor

@RollerMatic has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@RollerMatic has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@RollerMatic has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@RollerMatic RollerMatic marked this pull request as draft October 15, 2024 21:48
@RollerMatic RollerMatic deleted the fix_cmd_match_stringy branch October 15, 2024 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants