-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[shell] Add otcli pass-through commands for direct control over the OpenThread interface. #2371
Conversation
examples/shell/cmd_otcli.cpp
Outdated
otCliConsoleInputLine(buff, buff_ptr - buff); | ||
#else | ||
streamer_printf(streamer_get(), "Exec: %s\n\r", buff); | ||
system(buff); |
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.
This is vulnerable to shell escapes. Do we intend to give the CHIP shell control to run arbitrary system shell commands?
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.
The alternative is to replicate the socket APIs used by ot-ctl
itself. Perhaps those could be made available as a linkable library, but I don't think they are in that form currently.
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.
@mspang @rwalker-apple, updated to use fork()/exec()
.
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.
possible to move to fork()/exec()
(or equivalent) instead of system()
to reduce stringops and close the shell escape hole?
Yep, that's what I had in mind too. |
43033e6
to
2f8ae91
Compare
|
||
int pid, status; | ||
uid_t euid = geteuid(); | ||
char ctl_command[] = "/usr/local/sbin/ot-ctl"; |
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.
This will probably needs more configurability in the future. Could use execvp().
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.
I switched to execvp()
, but when running sudo, it still has trouble finding simply ot-cli
, even if I add export PATH=$PATH:/usr/local/sbin
to /root/.bashrc
.
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.
That's a sudo policy biting:
sudo grep secure_path /etc/sudoers
Defaults secure_path=/usr/sbin:/usr/bin:/sbin:/bin
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.
Maybe add a TODO for now to add some way to override (could just be a command line argument).
Size increase report for "nrfconnect-example-build"
Full report output
|
Size increase report for "esp32-example-build"
Full report output
|
Size increase report for "gn_nrf-example-build"
Full report output
|
Size increase report for "gn_linux-example-build"
Full report output
|
Size increase report for "gn_efr32-example-build"
Full report output
|
Problem
For testing and development scenarios, a shell app user may want to directly access the OpenThread cli in order to configure the interface.
Summary of Changes
Add an otcli pass-through option -- both for Linux and embedded (direct openthread C API calls).
fixes #1694