Merge #513

513: Fix daemon missing group r=Saviq a=townsend2010 I combined a few fixes in this as separate commits since they all went hand in hand and solve a couple of issues together. Fixes #456, fixes #460 Co-authored-by: Chris Townsend <[email protected]> Co-authored-by: Michał Sawicz <[email protected]>
canonical · Dec 11, 2018 · da3f6f4 · da3f6f4
2 parents 5a960ae + 8c00a1d
commit da3f6f4
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 10 deletions.
diff --git a/include/multipass/cli/command.h b/include/multipass/cli/command.h
@@ -21,8 +21,13 @@
 #include <multipass/callable_traits.h>
 #include <multipass/cli/return_codes.h>
 #include <multipass/rpc/multipass.grpc.pb.h>
+#include <multipass/utils.h>
 
+#include <QLocalSocket>
 #include <QString>
+
+#include <fmt/format.h>
+
 #include <grpc++/grpc++.h>
 
 namespace multipass
@@ -81,8 +86,31 @@ class Command
         {
             return on_success(reply);
         }
-
-        return on_failure(status);
+        else
+        {
+            auto socket_address{context.peer()};
+            const auto tokens = multipass::utils::split(context.peer(), ":");
+            if (tokens[0] == "unix")
+            {
+                socket_address = tokens[1];
+                QLocalSocket multipassd_socket;
+                multipassd_socket.connectToServer(QString::fromStdString(socket_address));
+                if (!multipassd_socket.waitForConnected() &&
+                    multipassd_socket.error() == QLocalSocket::SocketAccessError)
+                {
+                    grpc::Status denied_status{
+                        grpc::StatusCode::PERMISSION_DENIED, "multipass socket access denied",
+                        fmt::format("Please check that you have read/write permissions to '{}'", socket_address)};
+                    return on_failure(denied_status);
+                }
+            }
+
+            grpc::Status access_error_status{
+                grpc::StatusCode::NOT_FOUND, "cannot connect to the multipass socket",
+                fmt::format("Please ensure multipassd is running and '{}' is accessible", socket_address)};
+
+            return on_failure(access_error_status);
+        }
     }
 
     template <typename RpcFunc, typename Request, typename SuccessCallable, typename FailureCallable>

diff --git a/src/client/cmd/CMakeLists.txt b/src/client/cmd/CMakeLists.txt
@@ -44,4 +44,5 @@ target_link_libraries(commands
   ssh_client
   rpc
   Qt5::Core
+  Qt5::Network
   yaml)
diff --git a/src/client/cmd/common_cli.cpp b/src/client/cmd/common_cli.cpp
@@ -95,7 +95,10 @@ std::string cmd::instance_action_message_for(const mp::InstanceNames& instance_n
 mp::ReturnCode cmd::standard_failure_handler_for(const std::string& command, std::ostream& cerr,
                                                  const grpc::Status& status, const std::string& error_details)
 {
-    fmt::print(cerr, "{} failed: {}\n{}", command, status.error_message(), error_details);
+    fmt::print(cerr, "{} failed: {}\n{}", command, status.error_message(),
+               !error_details.empty()
+                   ? fmt::format("{}\n", error_details)
+                   : !status.error_details().empty() ? fmt::format("{}\n", status.error_details()) : "");
 
     return return_code_for(status.error_code());
 }

diff --git a/src/client/cmd/launch.cpp b/src/client/cmd/launch.cpp
@@ -246,15 +246,15 @@ mp::ReturnCode cmd::Launch::request_launch()
         {
             if (error == LaunchError::INVALID_DISK_SIZE)
             {
-                error_details = fmt::format("Invalid disk size value supplied: {}\n", request.disk_space());
+                error_details = fmt::format("Invalid disk size value supplied: {}", request.disk_space());
             }
             else if (error == LaunchError::INVALID_MEM_SIZE)
             {
-                error_details = fmt::format("Invalid memory size value supplied: {}\n", request.mem_size());
+                error_details = fmt::format("Invalid memory size value supplied: {}", request.mem_size());
             }
             else if (error == LaunchError::INVALID_HOSTNAME)
             {
-                cerr << "Invalid instance name supplied: " << request.instance_name() << "\n";
+                error_details = fmt::format("Invalid instance name supplied: {}", request.instance_name());
             }
         }
 

diff --git a/src/daemon/daemon_main.cpp b/src/daemon/daemon_main.cpp
@@ -49,6 +49,8 @@ namespace mpp = multipass::platform;
 
 namespace
 {
+const std::vector<std::string> supported_socket_groups{"sudo", "adm"};
+
 void set_server_permissions(const std::string& server_address)
 {
     auto tokens = mp::utils::split(server_address, ":");
@@ -59,12 +61,16 @@ void set_server_permissions(const std::string& server_address)
     if (server_name != "unix")
         return;
 
-    auto group = getgrnam("sudo");
-    if (!group)
-        throw std::runtime_error("Could not determine group id for 'sudo'.");
+    struct group* group{nullptr};
+    for (const auto socket_group : supported_socket_groups)
+    {
+        group = getgrnam(socket_group.c_str());
+        if (group)
+            break;
+    }
 
     const auto socket_path = tokens[1];
-    if (chown(socket_path.c_str(), 0, group->gr_gid) == -1)
+    if (group && chown(socket_path.c_str(), 0, group->gr_gid) == -1)
         throw std::runtime_error("Could not set ownership of the multipass socket.");
 
     if (chmod(socket_path.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) == -1)