Simplify the sudo server interface

Also bring it more in line with how the other APIs work in the AM.

Change-Id: I5ec1b4c3313c90808411ca374fac1552142c4372
Reviewed-by: Dominik Holland <dominik.holland@pelagicore.com>

8 files changed, 61 insertions, 85 deletions

diff --git a/examples/custom-appman/custom-appman.cpp b/examples/custom-appman/custom-appman.cpp
index 38b96a25..decccd08 100644
--- a/examples/custom-appman/custom-appman.cpp
+++ b/examples/custom-appman/custom-appman.cpp
@@ -69,20 +69,16 @@ Q_DECL_EXPORT int main(int argc, char *argv[])
 
     Package::ensureCorrectLocale();
 
-    QString error;
-    QStringList warnings;
-    if (Q_UNLIKELY(!forkSudoServer(DropPrivilegesPermanently, &error, &warnings))) {
-        qCCritical(LogSystem) << "ERROR:" << qPrintable(error);
-        return 2;
-    }
-
     try {
+        QStringList deploymentWarnings;
+        Sudo::forkServer(Sudo::DropPrivilegesPermanently, &deploymentWarnings);
+
         Main a(argc, argv);
 
         DefaultConfiguration cfg;
         cfg.parse();
 
-        a.setup(&cfg, &warnings);
+        a.setup(&cfg, deploymentWarnings);
         a.loadQml(cfg.loadDummyData());
         a.showWindow(cfg.fullscreen() && !cfg.noFullscreen());
 
diff --git a/src/installer-lib/sudo.cpp b/src/installer-lib/sudo.cpp
index 09c4623f..16be2e0d 100644
--- a/src/installer-lib/sudo.cpp
+++ b/src/installer-lib/sudo.cpp
@@ -126,7 +126,7 @@ QT_END_NAMESPACE_AM
 
 QT_BEGIN_NAMESPACE_AM
 
-bool forkSudoServer(SudoDropPrivileges dropPrivileges, QString *errorString, QStringList *warnings)
+void Sudo::forkServer(DropPrivileges dropPrivileges, QStringList *warnings)
 {
     bool canSudo = false;
     int loopControlFd = -1;
@@ -141,26 +141,21 @@ bool forkSudoServer(SudoDropPrivileges dropPrivileges, QString *errorString, QSt
     if (warnings && canSudo && (loopControlFd < 0))
         *warnings << qSL("Could not open /dev/loop-control, which is needed by the installer for SD-Card installations");
 #else
-    Q_UNUSED(errorString)
+    Q_UNUSED(warnings)
     Q_UNUSED(dropPrivileges)
 #endif
 
     if (!canSudo) {
-        SudoServer::initialize(-1, loopControlFd);
-        SudoClient::initialize(-1, SudoServer::instance());
+        SudoServer::createInstance(-1, loopControlFd);
+        SudoClient::createInstance(-1, SudoServer::instance());
         if (warnings) {
             *warnings << qSL("For the installer to work correctly, the executable needs to be run either as root via sudo or SUID (preferred)");
             *warnings << qSL("(using fallback implementation - you might experience permission errors on installer operations)");
         }
-        return true;
+        return;
     }
 
 #if defined(Q_OS_LINUX)
-    static QString dummy;
-    if (!errorString)
-        errorString = &dummy;
-    *errorString = QString();
-
     gid_t realGid = getgid();
     uid_t sudoUid = qEnvironmentVariableIntValue("SUDO_UID");
 
@@ -173,17 +168,13 @@ bool forkSudoServer(SudoDropPrivileges dropPrivileges, QString *errorString, QSt
         realUid = sudoUid;
         realGid = qEnvironmentVariableIntValue("SUDO_GID");
 
-        if (setresgid(realGid, 0, 0) || setresuid(realUid, 0, 0)) {
-            *errorString = QString::fromLatin1("could not set real user or group ID : %1").arg(QString::fromLocal8Bit(strerror(errno)));
-            return false;
-        }
+        if (setresgid(realGid, 0, 0) || setresuid(realUid, 0, 0))
+            throw Exception(errno, "Could not set real user or group ID");
     }
 
     int socketFds[2];
-    if (EINTR_LOOP(socketpair(AF_UNIX, SOCK_DGRAM, 0, socketFds)) != 0) {
-        *errorString = QString::fromLatin1("could not create a pair of sockets: %1").arg(QString::fromLocal8Bit(strerror(errno)));
-        return false;
-    }
+    if (EINTR_LOOP(socketpair(AF_UNIX, SOCK_DGRAM, 0, socketFds)) != 0)
+        throw Exception(errno, "Could not create a pair of sockets");
 
     // We need to make the gcda files generated by the root process writable by the normal user.
     // There is no way to detect a compilation with -ftest-coverage, but we can check for gcov
@@ -196,8 +187,7 @@ bool forkSudoServer(SudoDropPrivileges dropPrivileges, QString *errorString, QSt
 
     pid_t pid = fork();
     if (pid < 0) {
-        *errorString = qL1S("could not fork process");
-        return false;
+        throw Exception(errno, "Could not fork process");
     } else if (pid == 0) {
         // child
         close(0);
@@ -241,13 +231,9 @@ bool forkSudoServer(SudoDropPrivileges dropPrivileges, QString *errorString, QSt
         if (!capSetOk)
             qCCritical(LogSystem) << "could not drop privileges in the SudoServer process -- continuing with full root privileges";
 
-        if (SudoServer::initialize(socketFds[0], loopControlFd)) {
-            ProcessTitle::setTitle("%s", "sudo helper");
-            SudoServer::instance()->run();
-        } else {
-            qCCritical(LogSystem) << "could not initialize the SudoClient";
-            kill(getppid(), 9); //TODO: test - probably does not work?
-        }
+        SudoServer::createInstance(socketFds[0], loopControlFd);
+        ProcessTitle::setTitle("%s", "sudo helper");
+        SudoServer::instance()->run();
     }
     // parent
     close(loopControlFd);
@@ -256,34 +242,24 @@ bool forkSudoServer(SudoDropPrivileges dropPrivileges, QString *errorString, QSt
     if (realUmask)
         umask(realUmask);
 
-    if (!SudoClient::initialize(socketFds[1])) {
-        *errorString = qL1S("could not initialize the SudoClient");
-        kill(pid, 9);
-        return false;
-    }
+    SudoClient::createInstance(socketFds[1]);
 
     if (realUid != effectiveUid) {
         // drop all root privileges
         if (dropPrivileges == DropPrivilegesPermanently) {
             if (setresgid(realGid, realGid, realGid) || setresuid(realUid, realUid, realUid)) {
-                *errorString = QString::fromLatin1("could not set real user or group ID : %1").arg(QString::fromLocal8Bit(strerror(errno)));
-                kill(pid, 9);
-                return false;
+                kill(pid, SIGKILL);
+                throw Exception(errno, "Could not set real user or group ID");
             }
         } else {
             qCCritical(LogSystem) << "\nSudo was instructed to NOT drop root privileges permanently.\nThis is dangerous and should only be used in auto-tests!\n";
             if (setresgid(realGid, realGid, 0) || setresuid(realUid, realUid, 0)) {
-                *errorString = QString::fromLatin1("could not set real user or group ID : %1").arg(QString::fromLocal8Bit(strerror(errno)));
                 kill(pid, 9);
-                return false;
+                throw Exception(errno, "Could not set real user or group ID");
             }
         }
     }
     ::atexit([]() { SudoClient::instance()->stopServer(); });
-
-    return true;
-#else // Q_OS_LINUX
-    return false;
 #endif
 }
 
@@ -341,13 +317,13 @@ SudoClient::SudoClient(int socketFd)
     : m_socket(socketFd)
 { }
 
-bool SudoClient::initialize(int socketFd, SudoServer *shortCircuit)
+SudoClient *SudoClient::createInstance(int socketFd, SudoServer *shortCircuit)
 {
-    if (s_instance)
-        return false;
-    s_instance = new SudoClient(socketFd);
-    s_instance->m_shortCircuit = shortCircuit;
-    return true;
+    if (!s_instance) {
+        s_instance = new SudoClient(socketFd);
+        s_instance->m_shortCircuit = shortCircuit;
+    }
+    return s_instance;
 }
 
 
@@ -445,12 +421,11 @@ SudoServer::SudoServer(int socketFd, int loopControlFd)
     , m_loopControl(loopControlFd)
 { }
 
-bool SudoServer::initialize(int socketFd, int loopControlFd)
+SudoServer *SudoServer::createInstance(int socketFd, int loopControlFd)
 {
-    if (s_instance)
-        return false;
-    s_instance = new SudoServer(socketFd, loopControlFd);
-    return true;
+    if (!s_instance)
+        s_instance = new SudoServer(socketFd, loopControlFd);
+    return s_instance;
 }
 
 void SudoServer::run()
diff --git a/src/installer-lib/sudo.h b/src/installer-lib/sudo.h
index 40517439..44e7968c 100644
--- a/src/installer-lib/sudo.h
+++ b/src/installer-lib/sudo.h
@@ -58,12 +58,16 @@ typedef uint gid_t;
 
 QT_BEGIN_NAMESPACE_AM
 
-enum SudoDropPrivileges {
-    DropPrivilegesPermanently,
-    DropPrivilegesRegainable, // only use this for auto-tests
-};
+class Sudo
+{
+public:
+    enum DropPrivileges {
+        DropPrivilegesPermanently,
+        DropPrivilegesRegainable, // only use this for auto-tests
+    };
 
-bool forkSudoServer(SudoDropPrivileges dropPrivileges, QString *errorString, QStringList *warnings = nullptr);
+    static void forkServer(DropPrivileges dropPrivileges, QStringList *warnings = nullptr) Q_DECL_NOEXCEPT_EXPR(false);
+};
 
 class SudoInterface
 {
@@ -96,7 +100,7 @@ class SudoServer;
 class SudoClient : public SudoInterface
 {
 public:
-    static bool initialize(int socketFd, SudoServer *shortCircuit = 0);
+    static SudoClient *createInstance(int socketFd, SudoServer *shortCircuit = 0);
 
     static SudoClient *instance();
 
@@ -130,7 +134,7 @@ private:
 class SudoServer : public SudoInterface
 {
 public:
-    static bool initialize(int socketFd, int loopControlFd);
+    static SudoServer *createInstance(int socketFd, int loopControlFd);
 
     static SudoServer *instance();
 
diff --git a/src/main-lib/main.cpp b/src/main-lib/main.cpp
index fb85cb14..856e7e45 100644
--- a/src/main-lib/main.cpp
+++ b/src/main-lib/main.cpp
@@ -194,7 +194,7 @@ Main::~Main()
     The caller has to make sure that cfg will be available even after this function returns:
     we will access the cfg object from delayed init functions via lambdas!
 */
-void Main::setup(const DefaultConfiguration *cfg, QStringList *deploymentWarnings) Q_DECL_NOEXCEPT_EXPR(false)
+void Main::setup(const DefaultConfiguration *cfg, const QStringList &deploymentWarnings) Q_DECL_NOEXCEPT_EXPR(false)
 {
     // basics that are needed in multiple setup functions below
     m_noSecurity = cfg->noSecurity();
@@ -207,11 +207,8 @@ void Main::setup(const DefaultConfiguration *cfg, QStringList *deploymentWarning
     Logging::registerUnregisteredDltContexts();
 
     // dump accumulated warnings, now that logging rules are set
-    if (deploymentWarnings) {
-        for (const QString &warning : *deploymentWarnings)
-            qCWarning(LogDeployment).noquote() << warning;
-        deploymentWarnings->clear();
-    }
+    for (const QString &warning : deploymentWarnings)
+        qCWarning(LogDeployment).noquote() << warning;
 
     setupOpenGL(cfg->openGLConfiguration());
 
diff --git a/src/main-lib/main.h b/src/main-lib/main.h
index 0d8919f6..e80a0411 100644
--- a/src/main-lib/main.h
+++ b/src/main-lib/main.h
@@ -95,7 +95,7 @@ public:
 
     bool isSingleProcessMode() const;
 
-    void setup(const DefaultConfiguration *cfg, QStringList *deploymentWarnings = nullptr) Q_DECL_NOEXCEPT_EXPR(false);
+    void setup(const DefaultConfiguration *cfg, const QStringList &deploymentWarnings = QStringList()) Q_DECL_NOEXCEPT_EXPR(false);
     void loadQml(bool loadDummyData) Q_DECL_NOEXCEPT_EXPR(false);
     void showWindow(bool showFullscreen);
 
diff --git a/src/tools/appman/appman.cpp b/src/tools/appman/appman.cpp
index 6ad954ab..4b4da3f5 100644
--- a/src/tools/appman/appman.cpp
+++ b/src/tools/appman/appman.cpp
@@ -77,17 +77,15 @@ Q_DECL_EXPORT int main(int argc, char *argv[])
 
 #if !defined(AM_DISABLE_INSTALLER)
     Package::ensureCorrectLocale();
-
-    QString error;
-    QStringList warnings;
-    if (Q_UNLIKELY(!forkSudoServer(DropPrivilegesPermanently, &error, &warnings))) {
-        qCCritical(LogSystem) << "ERROR:" << qPrintable(error);
-        return 2;
-    }
-    StartupTimer::instance()->checkpoint("after sudo server fork");
 #endif
 
     try {
+        QStringList deploymentWarnings;
+#if !defined(AM_DISABLE_INSTALLER)
+        Sudo::forkServer(Sudo::DropPrivilegesPermanently, &deploymentWarnings);
+        StartupTimer::instance()->checkpoint("after sudo server fork");
+#endif
+
         Main a(argc, argv);
 
 #if defined(AM_TESTRUNNER)
@@ -107,7 +105,7 @@ Q_DECL_EXPORT int main(int argc, char *argv[])
 #if defined(AM_TESTRUNNER)
         TestRunner::initialize(cfg.testRunnerArguments());
 #endif
-        a.setup(&cfg, &warnings);
+        a.setup(&cfg, deploymentWarnings);
 #if defined(AM_TESTRUNNER)
         a.qmlEngine()->rootContext()->setContextProperty("buildConfig", cfg.buildConfig());
 #endif
diff --git a/tests/applicationinstaller/tst_applicationinstaller.cpp b/tests/applicationinstaller/tst_applicationinstaller.cpp
index 78b2f559..02984cab 100644
--- a/tests/applicationinstaller/tst_applicationinstaller.cpp
+++ b/tests/applicationinstaller/tst_applicationinstaller.cpp
@@ -942,7 +942,10 @@ int main(int argc, char **argv)
 {
     Package::ensureCorrectLocale();
 
-    startedSudoServer = forkSudoServer(DropPrivilegesPermanently, &sudoServerError);
+    try {
+        Sudo::forkServer(Sudo::DropPrivilegesPermanently);
+        startedSudoServer = true;
+    } catch (...) { }
 
     QCoreApplication a(argc, argv);
     tstApplicationInstaller = new tst_ApplicationInstaller(&a);
diff --git a/tests/sudo/tst_sudo.cpp b/tests/sudo/tst_sudo.cpp
index 66741104..fa859b43 100644
--- a/tests/sudo/tst_sudo.cpp
+++ b/tests/sudo/tst_sudo.cpp
@@ -499,7 +499,10 @@ static tst_Sudo *tstSudo = nullptr;
 
 int main(int argc, char **argv)
 {
-    startedSudoServer = forkSudoServer(DropPrivilegesRegainable, &sudoServerError);
+    try {
+        Sudo::forkServer(Sudo::DropPrivilegesRegainable);
+        startedSudoServer = true;
+    } catch (...) { }
 
     QCoreApplication a(argc, argv);
     tstSudo = new tst_Sudo(&a);