Tests:
- Make a privileged method throw and exception after spewing output to stdout
and stderr. The exception caught on the service daemon contains the expected
stdout and stderr messages.
- Sending SIGTERM to privileged daemon shuts down the daemon.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Fixes: #2551.
Tests:
- In a VM visit the storage page. Without the patch, an exception is logged when
there is no space to expand the partition. With the patch, the exception is not
logged.
- Raise an exception in the storage.usage_info() method and notice that the
exception is logged when visiting the Storage app page.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- These situation occur when server encounters an error when trying to formulate
a response. All exceptions during execution of actions are caught and reported
properly. However, server may encounter errors during processing of exception
raised in an action. Or may die abruptly. This special error will make
identifying such situations easier.
Tests:
- Add a 'return' after _read_request() in
privileged_daemon.py:RequestHandler:handle(). This will trigger this error.
Starting FreedomBox service will show these errors as 'ConnectionError: Server
returned empty response'. Similarly running 'freedombox-cmd --no-args plinth
is_package_manager_busy' will show the same error.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- A local storage object must exist globally shared by all threads. Then
object.__dict__ is the thread specific storage. Absent this, when multiple
actions run in parallel, one will erase the thread local object of another.
Tests:
- When an error is raised in a privileged method, then the HTML error shown
contains stdout and stderr of the involved processes.
- Running functional tests on a lot of apps does not show this error anymore.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This make it easy to find issues when looking at either main service logs or
privileged daemon logs.
Tests:
- Raise an exception in one of the privileged actions. Notice that the exception
is printed along with module name, action_name, stdout, stderr and traceback.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This change means that when invalid module or action name is provided, the log
message is not printed. However, this is acceptable as those cases are rare in
production and are logged properly on the client side.
Tests:
- Run diagnostics for an app and notice that arguments are printed in privileged
daemon's journald logs.
- Remove a password from bepasty app and notice that the password argument is
not logged.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- Older privileged daemon before 25.10 did not return the stdout/stderr
properties as part of an exception. During upgrade, there is a 5 minute time
window (longer if the privileged daemon is continuously used) when privileged
daemon is the old version and the service is the newer version. During this time
any exception in the privileged task will cause this problem.
- Our goal is not to always provide backward compatibility to old version of
privileged daemon as the web interface and privileged daemon are expected to be
upgraded at the same time. However, this one is easy and is complementary to a
separate fix that addresses the core problem.
Tests:
- Perform an operation that raises an Exception in a privileged method. The
error is properly shown as an HTML message but without stdout and stderr.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
Tests:
- Mounting/unmounting of remote SSH repositories works.
- Creating repo, creating/deleting/list archives work.
- If a privileged method raises an exception after outputting to stdout (using
action_utils.run) then stdout is shown in the HTML UI message.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
Tests:
- If there is a syntax error in communication with privileged server. 'stdout'
and 'stderr' keys are present in 'exception' dictionary of the reply.
- If there is a error in the privileged method in communication with privileged
server. 'stdout' and 'stderr' keys are present in 'exception' dictionary of the
reply. The values are filled with output of the command that have been run.
- If a privileged method uses action_utils.run, then raising an exception in the
method shows proper stdout and stderr in the UI HTML message.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
Tests:
- /usr/share/plinth/actions/actions is not installed.
- Code check works on plinth directory and container script only
- Provisioning a container does not add sudo configuration for actions. 'fbx'
user can perform 'sudo' operations.
- Make install does not install actions based sudo configuration. Admin users
can perform sudo operations.
- Exporting backup archive works. Validating a transmission directory works.
Some of the privileged operations works.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
- Regression: downloading does not work with sudo based action anymore. However,
sudo based actions are to be removed in later patches.
Tests:
- Downloading tar backup archive works. Untar works. Downloading gives upto
10MiB/s speed.
- If API is not called with _raw_output=True, then special exception is raised.
- Downloading tar file from command line using nc also works.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
- Used after migration gitweb and storage calls to using
action_utils.run_as_user.
Tests:
- Gitweb operations and directory validations works when privileged daemon is
running or not running.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
Without the change FileNotFound exception is raised.
Tests:
- Send request using 'nc' to privileged daemon that has invalid 'module'
parameter. SyntaxError exception is raised.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
- Instead of running the command using sudo. If the server is not reachable, run
the privileged command using sudo.
Tests:
- Typical privileged calls are made to server as evidenced by the network emoji
icon in the log.
- Some actions such as creating gitweb repository or downloading a backup
archive happen via sudo instead of privileged daemon.
- When a call is made to privileged daemon the log message is show just like a
sudo call.
- If the daemon is not running and can't be started, the calls are made to sudo.
- If the daemon is rejects connections, then calls are automatically made to
sudo.
- When cloning a gitweb repository, the operation is immediately returned and
task runs in background. Other tasks as waited upon until they are finished.
Introducing a sleep in privileged method leads to increased page load time.
- When server sends non-JSON response, a decode error is printed and exception
is raised.
- When a typical privileged call is made, the return value as expected.
- When a typical privileged call raises exception, a nice HTML exception is
shown in the UI. stdout/stderr outputs are not shown. Error is also logged on
the console as expected but without stdout/stderr.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
Tests:
- When a call is made to privileged daemon the log shows network emoji instead
of #.
- Log for unimplemented calls such as downloading backup images still shows # as
they not sent to privileged daemon.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
Tests:
- When privileged daemon receives a non-JSON request, a proper error structure
is returned with SyntaxError.
- When privileged daemon receives a request without 'module', 'action', 'args'
or 'kwargs' parameters, a proper error structure is returned with TypeError.
- When privileged daemon receives a request for invalid 'module' or 'action', a
proper error structure is returned with SyntaxError.
- When an exception is thrown in a privileged method, the error is properly
returned in error structure and caller is shown all the proper details.
- Valid return values are sent when a privileged call is made.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
- Refactor validation of fields in the JSON object.
- Throw distinct errors for missing field and wrong type.
Signed-off-by: Joseph Nuthalapati <njoseph@riseup.net>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>
- So that we write decorators that can handle errors as needed by backups app.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- When subprocess.call() fails and one of the arguments is a Path-like object,
the exception also contains a Path-like object. The default JSON encoder can't
handle this and will lead to failure when encoding the exception altogether
resulting in a generic exception.
Tests:
- Add an invalid .zim file to kiwix. It fails and shows a default error
exception. Without this patch, it fails.
- Functional tests for kiwix pass.
- Backups app can list archives. This is a result returned from a privileged
method.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
This is to ensure that secret parameter which must likely be marked as secret
are not marked as secret. The partially mitigates the biggest disadvantage of
printing all the parameters by default and marking exception, that is,
forgetting to mark.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- When privileged action is called, it is logged without arguments currently.
Extend this to log all arguments but excluding the parameters of type
secret_str.
- When error is raised, all arguments are being logged currently. Extend this to
exclude the parameters of type secret_str.
Tests:
- Privileged actions with secret strings log messages with '****' instead of
secret string.
- When an error is raised in a privileged action, an exception is logged. In the
exception message, the method and parameters are printed. Parameters that are
secret strings are shown as '****'.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- Method parameters marked with secret_str will not be logged.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
It is already available separately and now printed nicely. In cases where the
exception is caused outside of the action method, continue to print stderr.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
return_value is not available during exception handling.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Tests:
- All tests in patch series have been done with this patch applied
- Unit tests pass
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Currently privileged actions use stdout for returning the results. If any of the
sub-processes accidentally output to stdout, decoding errors occur. Prevent this
by opening a pipe to the privileged action and returning the output in that
pipe.
Tests:
- Run unit tests
- Functional tests for other apps pass
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- Currently, privileged actions are not allowed under top-level plinth module.
They are only allowed under each app module. Allow privileged actions under
plinth module.
- Currently, privileged actions are not allowed under a sub-module of
'privileged' package. They are allowed only in 'privileged' module. Allow
sub-modules under 'privileged' package.
Tests:
- Email app functional tests pass
- Functional tests for apps using package and service privileged methods pass
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- This can be improved later by using a IPC mechanism other than stdin/stdout.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Any privileged action (a method) can be marked as such with the new decorator. A
call to the method will be serialized into a sudo call (or later into a D-Bus
call). The method arguments are turned to JSON and method is called as
superuser. Arguments are de-serialized and are verified for type before the
actual call as superuser. Return values are serialized and returned where they
are de-serialized. Exceptions are also serialized and de-serialized.
The method must have be strictly typed and should not have keyword-only
arguments. Currently supported types are int, float, str, dict/Dict, list/List
and Optional.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- Use '#' vs '$' to indicate root vs. user message.
- Remove '-n' argument to sudo as it is always present.
- Remove env arguments to sudo as they are only present during debug.
- Remove full path to the action as they are already thoroughly checked.
- Print the message as a shell command with escapes instead of as python list.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- Don't set bufsize to 1 while streaming backup download. This is only effective
with text streams with universal_newline flag set. An actual buffer size of 1
is very inefficient and plain wrong. Leave the python default of
io.DEFAULT_BUFFER_SIZE.
- Minor simplification to argument passing.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
- some variable names, indentation and documentation changes
- removed unused backups action
- changed name of upload session variable to 'fbx-backups-upload-path'
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
async is a reserved keyword in Python 3.7. It can no longer be used as method
parameter. Change the name so that we are ready for Python 3.7.
See: https://www.python.org/dev/peps/pep-0492/#deprecation-plans
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
