From 4c0719d95da31f7043b1c0f3dbd331dd9ada2bba Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 13:44:06 +0200 Subject: [PATCH 01/18] [machineid] Start porting to C++ --- src/modules/machineid/CMakeLists.txt | 9 +++++ src/modules/machineid/MachineIdJob.cpp | 55 ++++++++++++++++++++++++++ src/modules/machineid/MachineIdJob.h | 48 ++++++++++++++++++++++ src/modules/machineid/module.desc | 5 --- 4 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 src/modules/machineid/CMakeLists.txt create mode 100644 src/modules/machineid/MachineIdJob.cpp create mode 100644 src/modules/machineid/MachineIdJob.h delete mode 100644 src/modules/machineid/module.desc diff --git a/src/modules/machineid/CMakeLists.txt b/src/modules/machineid/CMakeLists.txt new file mode 100644 index 000000000..aa82d672f --- /dev/null +++ b/src/modules/machineid/CMakeLists.txt @@ -0,0 +1,9 @@ +calamares_add_plugin( machineid + TYPE job + EXPORT_MACRO PLUGINDLLEXPORT_PRO + SOURCES + MachineIdJob.cpp + LINK_PRIVATE_LIBRARIES + calamares + SHARED_LIB +) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp new file mode 100644 index 000000000..b604c645b --- /dev/null +++ b/src/modules/machineid/MachineIdJob.cpp @@ -0,0 +1,55 @@ +/* === This file is part of Calamares - === + * + * Copyright 2014, Kevin Kofler + * Copyright 2016, Philip Müller + * Copyright 2017, Alf Gaida + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "MachineIdJob.h" + +#include "utils/CalamaresUtilsSystem.h" +#include "utils/Logger.h" + +MachineIdJob::MachineIdJob( QObject* parent ) + : Calamares::CppJob( parent ) +{ +} + + +MachineIdJob::~MachineIdJob() {} + + +QString +MachineIdJob::prettyName() const +{ + return tr( "Generate machine-id." ); +} + + +Calamares::JobResult +MachineIdJob::exec() +{ + return Calamares::JobResult::ok(); +} + + +void +MachineIdJob::setConfigurationMap( const QVariantMap& configurationMap ) +{ +} + +CALAMARES_PLUGIN_FACTORY_DEFINITION( MachineIdJobFactory, registerPlugin< MachineIdJob >(); ) diff --git a/src/modules/machineid/MachineIdJob.h b/src/modules/machineid/MachineIdJob.h new file mode 100644 index 000000000..7fb705201 --- /dev/null +++ b/src/modules/machineid/MachineIdJob.h @@ -0,0 +1,48 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef MACHINEIDJOB_H +#define MACHINEIDJOB_H + +#include +#include + +#include + +#include + +#include + +class PLUGINDLLEXPORT MachineIdJob : public Calamares::CppJob +{ + Q_OBJECT + +public: + explicit MachineIdJob( QObject* parent = nullptr ); + virtual ~MachineIdJob() override; + + QString prettyName() const override; + + Calamares::JobResult exec() override; + + void setConfigurationMap( const QVariantMap& configurationMap ) override; +}; + +CALAMARES_PLUGIN_FACTORY_DECLARATION( MachineIdJobFactory ) + +#endif // DUMMYCPPJOB_H diff --git a/src/modules/machineid/module.desc b/src/modules/machineid/module.desc deleted file mode 100644 index 8d42b64f5..000000000 --- a/src/modules/machineid/module.desc +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: "job" -name: "machineid" -interface: "python" -script: "main.py" From 3ae5a3db76158770522dec835a1e312e8129e40b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 14:13:01 +0200 Subject: [PATCH 02/18] [machineid] Improve documentation on this module - document module's purpose - document existing configurations - deprecate "symlink" and introduce more-accurate "dbus-symlink" - add new configurations for upcoming entropy file --- src/modules/machineid/machineid.conf | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/modules/machineid/machineid.conf b/src/modules/machineid/machineid.conf index 263687263..97bd10a06 100644 --- a/src/modules/machineid/machineid.conf +++ b/src/modules/machineid/machineid.conf @@ -1,8 +1,24 @@ +# Machine-ID and other random data on the target system. +# +# This module can create a number of "random" things on the target: +# - a systemd machine-id file (hence the name of the Calamares module) +# with a random UUID. +# - a dbus machine-id file (or, optionally, link to the one from systemd) +# - an entropy file +# --- # Whether to create /etc/machine-id for systemd. systemd: true + # Whether to create /var/lib/dbus/machine-id for D-Bus. dbus: true # Whether /var/lib/dbus/machine-id should be a symlink to /etc/machine-id # (ignored if dbus is false, or if there is no /etc/machine-id to point to). +dbus-symlink: true +# this is a deprecated form of *dbus-symlink* symlink: true + +# Whether to create an entropy file +entropy: false +# Whether to copy entropy from the host +entropy-copy: false From be27b448181ad5fe0be1a552cd6583289636941e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 14:36:21 +0200 Subject: [PATCH 03/18] [machineid] Read configuration map into instance vars --- src/modules/machineid/MachineIdJob.cpp | 22 +++++++++++++++++++++- src/modules/machineid/MachineIdJob.h | 9 +++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index b604c645b..3fd539e80 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -23,6 +23,7 @@ #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" +#include "utils/Variant.h" MachineIdJob::MachineIdJob( QObject* parent ) : Calamares::CppJob( parent ) @@ -48,8 +49,27 @@ MachineIdJob::exec() void -MachineIdJob::setConfigurationMap( const QVariantMap& configurationMap ) +MachineIdJob::setConfigurationMap( const QVariantMap& map ) { + m_systemd = CalamaresUtils::getBool( map, "systemd", false ); + + m_dbus = CalamaresUtils::getBool( map, "dbus", false ); + if ( map.contains( "dbus-symlink" ) ) + { + m_dbus_symlink = CalamaresUtils::getBool( map, "dbus-symlink", false ); + } + else if ( map.contains( "symlink" ) ) + { + m_dbus_symlink = CalamaresUtils::getBool( map, "symlink", false ); + cWarning() << "MachineId: configuration setting *symlink* is deprecated, use *dbus-symlink*."; + } + // else it's still false from the constructor + + // ignore it, though, if dbus is false + m_dbus_symlink = m_dbus && m_dbus_symlink; + + m_entropy = CalamaresUtils::getBool( map, "entropy", false ); + m_entropy_copy = CalamaresUtils::getBool( map, "entropy-copy", false ); } CALAMARES_PLUGIN_FACTORY_DEFINITION( MachineIdJobFactory, registerPlugin< MachineIdJob >(); ) diff --git a/src/modules/machineid/MachineIdJob.h b/src/modules/machineid/MachineIdJob.h index 7fb705201..d775d9007 100644 --- a/src/modules/machineid/MachineIdJob.h +++ b/src/modules/machineid/MachineIdJob.h @@ -41,6 +41,15 @@ public: Calamares::JobResult exec() override; void setConfigurationMap( const QVariantMap& configurationMap ) override; + +private: + bool m_systemd = false; ///< write systemd's files + + bool m_dbus = false; ///< write dbus files + bool m_dbus_symlink = false; ///< .. or just symlink to systemd + + bool m_entropy = false; ///< write an entropy file + bool m_entropy_copy = false; ///< copy from host system }; CALAMARES_PLUGIN_FACTORY_DECLARATION( MachineIdJobFactory ) From 8c3146a1cd7ea3d6918d29c67904a9704073f72e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 15:46:54 +0200 Subject: [PATCH 04/18] [machineid] Start implementing module logic - remove existing files for each kind of random-generation that is enabled. There's a helper function for the case that Cala is no longer setuid and needs help to remove those files from the target (e.g. a setuid helper). --- src/modules/machineid/MachineIdJob.cpp | 44 ++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index 3fd539e80..b8fc3334a 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -25,6 +25,11 @@ #include "utils/Logger.h" #include "utils/Variant.h" +#include "GlobalStorage.h" +#include "JobQueue.h" + +#include + MachineIdJob::MachineIdJob( QObject* parent ) : Calamares::CppJob( parent ) { @@ -40,10 +45,49 @@ MachineIdJob::prettyName() const return tr( "Generate machine-id." ); } +// might need to use a helper to remove the file +static void +removeFile( const QString& fileName ) +{ + QFile::remove( fileName ); +} Calamares::JobResult MachineIdJob::exec() { + QString root; + + Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); + if ( gs && gs->contains( "rootMountPoint" ) ) + { + root = gs->value( "rootMountPoint" ).toString(); + } + else + { + cWarning() << "No *rootMountPoint* defined."; + return Calamares::JobResult::internalError( tr( "Configuration Error" ), + tr( "No root mount point is set for MachineId." ), + Calamares::JobResult::InvalidConfiguration ); + } + + QString target_systemd_machineid_file = root + QStringLiteral( "/etc/machine-id" ); + QString target_dbus_machineid_file = root + QStringLiteral( "/var/lib/dbus/machine-id" ); + QString target_entropy_file = root + QStringLiteral( "/var/lib/urandom/random-seed" ); + + // Clear existing files + if ( m_entropy ) + { + removeFile( target_entropy_file ); + } + if ( m_dbus ) + { + removeFile( target_dbus_machineid_file ); + } + if ( m_systemd ) + { + removeFile( target_systemd_machineid_file ); + } + return Calamares::JobResult::ok(); } From 3a8d543c72969c7fda7cdd1cdda2f17948a1e68e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 15:58:02 +0200 Subject: [PATCH 05/18] [libcalamares] Document operator bool() of JobResult - this could be named isValid() instead, but basically the idea is that this code makes sense: JobResult r = do_thing(); if ( !r ) { /* Error happened! */ return r; } /* Carry on .. */ --- src/libcalamares/Job.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libcalamares/Job.h b/src/libcalamares/Job.h index 3eaa251ef..bddd0277c 100644 --- a/src/libcalamares/Job.h +++ b/src/libcalamares/Job.h @@ -46,11 +46,16 @@ public: InvalidConfiguration = 2 }; + // Can't copy, but you can keep a temporary JobResult( const JobResult& rhs ) = delete; JobResult( JobResult&& rhs ); virtual ~JobResult() {} + /** @brief Is this JobResult a success? + * + * Equivalent to errorCode() == 0, might be named isValid(). + */ virtual operator bool() const; virtual QString message() const; From 9cbfd200a1ee08a59610245b8f8b0764bf43427e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 16:01:10 +0200 Subject: [PATCH 06/18] [machineid] Keep the paths unsullied - keep the rootMountPoint and the path-with-random-data separate instead of concatenating them at the beginning. Then we can use the "clean" names also within the host system. --- src/modules/machineid/MachineIdJob.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index b8fc3334a..0edf8a4da 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -47,9 +47,9 @@ MachineIdJob::prettyName() const // might need to use a helper to remove the file static void -removeFile( const QString& fileName ) +removeFile( const QString& rootMountPoint, const QString& fileName ) { - QFile::remove( fileName ); + QFile::remove( rootMountPoint + fileName ); } Calamares::JobResult @@ -70,22 +70,22 @@ MachineIdJob::exec() Calamares::JobResult::InvalidConfiguration ); } - QString target_systemd_machineid_file = root + QStringLiteral( "/etc/machine-id" ); - QString target_dbus_machineid_file = root + QStringLiteral( "/var/lib/dbus/machine-id" ); - QString target_entropy_file = root + QStringLiteral( "/var/lib/urandom/random-seed" ); + QString target_systemd_machineid_file = QStringLiteral( "/etc/machine-id" ); + QString target_dbus_machineid_file = QStringLiteral( "/var/lib/dbus/machine-id" ); + QString target_entropy_file = QStringLiteral( "/var/lib/urandom/random-seed" ); // Clear existing files if ( m_entropy ) { - removeFile( target_entropy_file ); + removeFile( root, target_entropy_file ); } if ( m_dbus ) { - removeFile( target_dbus_machineid_file ); + removeFile( root, target_dbus_machineid_file ); } if ( m_systemd ) { - removeFile( target_systemd_machineid_file ); + removeFile( root, target_systemd_machineid_file ); } return Calamares::JobResult::ok(); From c8229733b0b71844c5c0daff7c4349a2f039e0c7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 16:05:03 +0200 Subject: [PATCH 07/18] [libcalamares] Document the pseudo-constructors for JobResult --- src/libcalamares/Job.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/Job.h b/src/libcalamares/Job.h index bddd0277c..3a68297ca 100644 --- a/src/libcalamares/Job.h +++ b/src/libcalamares/Job.h @@ -68,9 +68,16 @@ public: /// @brief an "ok status" result static JobResult ok(); - /// @brief an "error" result resulting from the execution of the job + /** @brief an "error" result resulting from the execution of the job + * + * The error code is set to GenericError. + */ static JobResult error( const QString& message, const QString& details = QString() ); - /// @brief an "internal error" meaning the job itself has a problem (usually for python) + /** @brief an "internal error" meaning the job itself has a problem (usually for python) + * + * Pass in a suitable error code; using 0 (which would normally mean "ok") instead + * gives you a GenericError code. + */ static JobResult internalError( const QString&, const QString& details, int errorCode ); protected: From 50bb8cde57f2f21fb05d2dff6f5f981844b0620d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 16:26:41 +0200 Subject: [PATCH 08/18] [machineid] Stubs for entropy, systemd, dbus randomness --- src/modules/machineid/MachineIdJob.cpp | 60 ++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index 0edf8a4da..00149793c 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -52,6 +52,37 @@ removeFile( const QString& rootMountPoint, const QString& fileName ) QFile::remove( rootMountPoint + fileName ); } +/// @brief How to generate entropy (bool-like) +enum class EntropyGeneration +{ + New, + CopyFromHost +}; +/// @brief How to create the DBus machine-id (bool-like) +enum class DBusGeneration +{ + New, + SymlinkFromSystemD +}; + +static Calamares::JobResult +createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, const QString& fileName ) +{ + return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); +} + +static Calamares::JobResult +createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ) +{ + return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); +} + +static Calamares::JobResult +createDBusMachineId( DBusGeneration kind, const QString& rootMountPoint, const QString& fileName ) +{ + return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); +} + Calamares::JobResult MachineIdJob::exec() { @@ -88,6 +119,35 @@ MachineIdJob::exec() removeFile( root, target_systemd_machineid_file ); } + //Create new files + if ( m_entropy ) + { + auto r = createEntropy( + m_entropy_copy ? EntropyGeneration::CopyFromHost : EntropyGeneration::New, root, target_entropy_file ); + if ( !r ) + { + return r; + } + } + if ( m_systemd ) + { + auto r = createSystemdMachineId( root, target_systemd_machineid_file ); + if ( !r ) + { + return r; + } + } + if ( m_dbus ) + { + auto r = createDBusMachineId( m_dbus_symlink ? DBusGeneration::SymlinkFromSystemD : DBusGeneration::New, + root, + target_dbus_machineid_file ); + if ( !r ) + { + return r; + } + } + return Calamares::JobResult::ok(); } From 8352a793e1664fd36c6903ba5ec07e469390d36a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 1 Oct 2019 17:00:50 +0200 Subject: [PATCH 09/18] [machineid] Implement entropy-file creation - read-urandom or copy-existing-file are implemented - fairly chatty on failure - needs tests (probably the implementation should be moved to a separate file and unit-tested) --- src/modules/machineid/MachineIdJob.cpp | 101 ++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index 00149793c..37df5ef34 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -52,12 +52,29 @@ removeFile( const QString& rootMountPoint, const QString& fileName ) QFile::remove( rootMountPoint + fileName ); } +/// @brief Copy @p fileName from host into target system at @p rootMountPoint +static Calamares::JobResult +copyFile( const QString& rootMountPoint, const QString& fileName ) +{ + QFile f( fileName ); + if ( !f.exists() ) + { + return Calamares::JobResult::error( QObject::tr( "File not found" ), fileName ); + } + if ( !f.copy( rootMountPoint + fileName ) ) + { + return Calamares::JobResult::error( QObject::tr( "File not found" ), rootMountPoint + fileName ); + } + return Calamares::JobResult::ok(); +} + /// @brief How to generate entropy (bool-like) enum class EntropyGeneration { New, CopyFromHost }; + /// @brief How to create the DBus machine-id (bool-like) enum class DBusGeneration { @@ -65,10 +82,92 @@ enum class DBusGeneration SymlinkFromSystemD }; +static int +getUrandomPoolSize() +{ + QFile f( "/proc/sys/kernel/random/poolsize" ); + constexpr const int minimumPoolSize = 512; + int poolSize = minimumPoolSize; + + if ( f.exists() && f.open( QIODevice::ReadOnly | QIODevice::Text ) ) + { + QByteArray v = f.read( 16 ); + if ( v.length() > 2 ) + { + bool ok = false; + poolSize = v.toInt( &ok ); + if ( !ok ) + { + poolSize = minimumPoolSize; + } + } + } + return poolSize >= minimumPoolSize ? poolSize : minimumPoolSize; +} + +static Calamares::JobResult +createNewEntropy( int poolSize, const QString& rootMountPoint, const QString& fileName ) +{ + QFile urandom( "/dev/urandom" ); + if ( urandom.exists() && urandom.open( QIODevice::ReadOnly ) ) + { + QByteArray data = urandom.read( poolSize ); + urandom.close(); + + QFile entropyFile( rootMountPoint + fileName ); + if ( entropyFile.exists() ) + { + cWarning() << "Entropy file" << ( rootMountPoint + fileName ) << "already exists."; + return Calamares::JobResult::ok(); // .. anyway + } + if ( !entropyFile.open( QIODevice::WriteOnly ) ) + { + return Calamares::JobResult::error( + QObject::tr( "File not found" ), + QObject::tr( "Could not create new random file
%1
." ).arg( fileName ) ); + } + entropyFile.write( data ); + entropyFile.close(); + if ( entropyFile.size() < data.length() ) + { + cWarning() << "Entropy file is" << entropyFile.size() << "bytes, random data was" << data.length(); + } + if ( data.length() < poolSize ) + { + cWarning() << "Entropy data is" << data.length() << "bytes, rather than poolSize" << poolSize; + } + } + return Calamares::JobResult::error( + QObject::tr( "File not found" ), + QObject::tr( "Could not read random file
%1
." ).arg( QStringLiteral( "/dev/urandom" ) ) ); +} + + static Calamares::JobResult createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, const QString& fileName ) { - return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); + if ( kind == EntropyGeneration::CopyFromHost ) + { + if ( QFile::exists( fileName ) ) + { + auto r = copyFile( rootMountPoint, fileName ); + if ( r ) + { + return r; + } + else + { + cWarning() << "Could not copy" << fileName << "for entropy, generating new."; + } + } + else + { + cWarning() << "Host system entropy does not exist at" << fileName; + } + } + + int poolSize = getUrandomPoolSize(); + return createNewEntropy( poolSize, rootMountPoint, fileName ); } static Calamares::JobResult From afe7dfbcf2432a356a2f13d0e7ecf93b02f32e2e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 2 Oct 2019 12:37:31 +0200 Subject: [PATCH 10/18] [machineid] Fix include guard comment --- src/modules/machineid/MachineIdJob.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/machineid/MachineIdJob.h b/src/modules/machineid/MachineIdJob.h index d775d9007..5d44b2017 100644 --- a/src/modules/machineid/MachineIdJob.h +++ b/src/modules/machineid/MachineIdJob.h @@ -54,4 +54,4 @@ private: CALAMARES_PLUGIN_FACTORY_DECLARATION( MachineIdJobFactory ) -#endif // DUMMYCPPJOB_H +#endif // MACHINEIDJOB_H From 10e5995144f765540f3e1c7cd56c46b4d77f7148 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 3 Oct 2019 12:32:47 +0200 Subject: [PATCH 11/18] [machineid] Split helper functions into own file --- src/modules/machineid/CMakeLists.txt | 1 + src/modules/machineid/MachineIdJob.cpp | 152 ++---------------------- src/modules/machineid/Workers.cpp | 154 +++++++++++++++++++++++++ src/modules/machineid/Workers.h | 83 +++++++++++++ 4 files changed, 246 insertions(+), 144 deletions(-) create mode 100644 src/modules/machineid/Workers.cpp create mode 100644 src/modules/machineid/Workers.h diff --git a/src/modules/machineid/CMakeLists.txt b/src/modules/machineid/CMakeLists.txt index aa82d672f..a6874fa17 100644 --- a/src/modules/machineid/CMakeLists.txt +++ b/src/modules/machineid/CMakeLists.txt @@ -3,6 +3,7 @@ calamares_add_plugin( machineid EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES MachineIdJob.cpp + Workers.cpp LINK_PRIVATE_LIBRARIES calamares SHARED_LIB diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index 37df5ef34..2f0416c72 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -20,6 +20,7 @@ */ #include "MachineIdJob.h" +#include "Workers.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" @@ -45,143 +46,6 @@ MachineIdJob::prettyName() const return tr( "Generate machine-id." ); } -// might need to use a helper to remove the file -static void -removeFile( const QString& rootMountPoint, const QString& fileName ) -{ - QFile::remove( rootMountPoint + fileName ); -} - -/// @brief Copy @p fileName from host into target system at @p rootMountPoint -static Calamares::JobResult -copyFile( const QString& rootMountPoint, const QString& fileName ) -{ - QFile f( fileName ); - if ( !f.exists() ) - { - return Calamares::JobResult::error( QObject::tr( "File not found" ), fileName ); - } - if ( !f.copy( rootMountPoint + fileName ) ) - { - return Calamares::JobResult::error( QObject::tr( "File not found" ), rootMountPoint + fileName ); - } - return Calamares::JobResult::ok(); -} - -/// @brief How to generate entropy (bool-like) -enum class EntropyGeneration -{ - New, - CopyFromHost -}; - -/// @brief How to create the DBus machine-id (bool-like) -enum class DBusGeneration -{ - New, - SymlinkFromSystemD -}; - -static int -getUrandomPoolSize() -{ - QFile f( "/proc/sys/kernel/random/poolsize" ); - constexpr const int minimumPoolSize = 512; - int poolSize = minimumPoolSize; - - if ( f.exists() && f.open( QIODevice::ReadOnly | QIODevice::Text ) ) - { - QByteArray v = f.read( 16 ); - if ( v.length() > 2 ) - { - bool ok = false; - poolSize = v.toInt( &ok ); - if ( !ok ) - { - poolSize = minimumPoolSize; - } - } - } - return poolSize >= minimumPoolSize ? poolSize : minimumPoolSize; -} - -static Calamares::JobResult -createNewEntropy( int poolSize, const QString& rootMountPoint, const QString& fileName ) -{ - QFile urandom( "/dev/urandom" ); - if ( urandom.exists() && urandom.open( QIODevice::ReadOnly ) ) - { - QByteArray data = urandom.read( poolSize ); - urandom.close(); - - QFile entropyFile( rootMountPoint + fileName ); - if ( entropyFile.exists() ) - { - cWarning() << "Entropy file" << ( rootMountPoint + fileName ) << "already exists."; - return Calamares::JobResult::ok(); // .. anyway - } - if ( !entropyFile.open( QIODevice::WriteOnly ) ) - { - return Calamares::JobResult::error( - QObject::tr( "File not found" ), - QObject::tr( "Could not create new random file
%1
." ).arg( fileName ) ); - } - entropyFile.write( data ); - entropyFile.close(); - if ( entropyFile.size() < data.length() ) - { - cWarning() << "Entropy file is" << entropyFile.size() << "bytes, random data was" << data.length(); - } - if ( data.length() < poolSize ) - { - cWarning() << "Entropy data is" << data.length() << "bytes, rather than poolSize" << poolSize; - } - } - return Calamares::JobResult::error( - QObject::tr( "File not found" ), - QObject::tr( "Could not read random file
%1
." ).arg( QStringLiteral( "/dev/urandom" ) ) ); -} - - -static Calamares::JobResult -createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, const QString& fileName ) -{ - if ( kind == EntropyGeneration::CopyFromHost ) - { - if ( QFile::exists( fileName ) ) - { - auto r = copyFile( rootMountPoint, fileName ); - if ( r ) - { - return r; - } - else - { - cWarning() << "Could not copy" << fileName << "for entropy, generating new."; - } - } - else - { - cWarning() << "Host system entropy does not exist at" << fileName; - } - } - - int poolSize = getUrandomPoolSize(); - return createNewEntropy( poolSize, rootMountPoint, fileName ); -} - -static Calamares::JobResult -createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ) -{ - return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); -} - -static Calamares::JobResult -createDBusMachineId( DBusGeneration kind, const QString& rootMountPoint, const QString& fileName ) -{ - return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); -} - Calamares::JobResult MachineIdJob::exec() { @@ -207,22 +71,22 @@ MachineIdJob::exec() // Clear existing files if ( m_entropy ) { - removeFile( root, target_entropy_file ); + MachineId::removeFile( root, target_entropy_file ); } if ( m_dbus ) { - removeFile( root, target_dbus_machineid_file ); + MachineId::removeFile( root, target_dbus_machineid_file ); } if ( m_systemd ) { - removeFile( root, target_systemd_machineid_file ); + MachineId::removeFile( root, target_systemd_machineid_file ); } //Create new files if ( m_entropy ) { - auto r = createEntropy( - m_entropy_copy ? EntropyGeneration::CopyFromHost : EntropyGeneration::New, root, target_entropy_file ); + auto r = MachineId::createEntropy( + m_entropy_copy ? MachineId::EntropyGeneration::CopyFromHost : MachineId::EntropyGeneration::New, root, target_entropy_file ); if ( !r ) { return r; @@ -230,7 +94,7 @@ MachineIdJob::exec() } if ( m_systemd ) { - auto r = createSystemdMachineId( root, target_systemd_machineid_file ); + auto r = MachineId::createSystemdMachineId( root, target_systemd_machineid_file ); if ( !r ) { return r; @@ -238,7 +102,7 @@ MachineIdJob::exec() } if ( m_dbus ) { - auto r = createDBusMachineId( m_dbus_symlink ? DBusGeneration::SymlinkFromSystemD : DBusGeneration::New, + auto r = MachineId::createDBusMachineId( m_dbus_symlink ? MachineId::DBusGeneration::SymlinkFromSystemD : MachineId::DBusGeneration::New, root, target_dbus_machineid_file ); if ( !r ) diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp new file mode 100644 index 000000000..7b70fab9f --- /dev/null +++ b/src/modules/machineid/Workers.cpp @@ -0,0 +1,154 @@ +/* === This file is part of Calamares - === + * + * Copyright 2014, Kevin Kofler + * Copyright 2016, Philip Müller + * Copyright 2017, Alf Gaida + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "Workers.h" + +#include "utils/CalamaresUtilsSystem.h" +#include "utils/Logger.h" + +#include + +namespace MachineId +{ + +// might need to use a helper to remove the file +void +removeFile( const QString& rootMountPoint, const QString& fileName ) +{ + QFile::remove( rootMountPoint + fileName ); +} + +Calamares::JobResult +copyFile( const QString& rootMountPoint, const QString& fileName ) +{ + QFile f( fileName ); + if ( !f.exists() ) + { + return Calamares::JobResult::error( QObject::tr( "File not found" ), fileName ); + } + if ( !f.copy( rootMountPoint + fileName ) ) + { + return Calamares::JobResult::error( QObject::tr( "File not found" ), rootMountPoint + fileName ); + } + return Calamares::JobResult::ok(); +} + +int +getUrandomPoolSize() +{ + QFile f( "/proc/sys/kernel/random/poolsize" ); + constexpr const int minimumPoolSize = 512; + int poolSize = minimumPoolSize; + + if ( f.exists() && f.open( QIODevice::ReadOnly | QIODevice::Text ) ) + { + QByteArray v = f.read( 16 ); + if ( v.length() > 2 ) + { + bool ok = false; + poolSize = v.toInt( &ok ); + if ( !ok ) + { + poolSize = minimumPoolSize; + } + } + } + return poolSize >= minimumPoolSize ? poolSize : minimumPoolSize; +} + +Calamares::JobResult +createNewEntropy( int poolSize, const QString& rootMountPoint, const QString& fileName ) +{ + QFile urandom( "/dev/urandom" ); + if ( urandom.exists() && urandom.open( QIODevice::ReadOnly ) ) + { + QByteArray data = urandom.read( poolSize ); + urandom.close(); + + QFile entropyFile( rootMountPoint + fileName ); + if ( entropyFile.exists() ) + { + cWarning() << "Entropy file" << ( rootMountPoint + fileName ) << "already exists."; + return Calamares::JobResult::ok(); // .. anyway + } + if ( !entropyFile.open( QIODevice::WriteOnly ) ) + { + return Calamares::JobResult::error( + QObject::tr( "File not found" ), + QObject::tr( "Could not create new random file
%1
." ).arg( fileName ) ); + } + entropyFile.write( data ); + entropyFile.close(); + if ( entropyFile.size() < data.length() ) + { + cWarning() << "Entropy file is" << entropyFile.size() << "bytes, random data was" << data.length(); + } + if ( data.length() < poolSize ) + { + cWarning() << "Entropy data is" << data.length() << "bytes, rather than poolSize" << poolSize; + } + } + return Calamares::JobResult::error( + QObject::tr( "File not found" ), + QObject::tr( "Could not read random file
%1
." ).arg( QStringLiteral( "/dev/urandom" ) ) ); +} + + +Calamares::JobResult +createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, const QString& fileName ) +{ + if ( kind == EntropyGeneration::CopyFromHost ) + { + if ( QFile::exists( fileName ) ) + { + auto r = copyFile( rootMountPoint, fileName ); + if ( r ) + { + return r; + } + else + { + cWarning() << "Could not copy" << fileName << "for entropy, generating new."; + } + } + else + { + cWarning() << "Host system entropy does not exist at" << fileName; + } + } + + int poolSize = getUrandomPoolSize(); + return createNewEntropy( poolSize, rootMountPoint, fileName ); +} + +Calamares::JobResult +createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ) +{ + return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); +} + +Calamares::JobResult +createDBusMachineId( DBusGeneration kind, const QString& rootMountPoint, const QString& fileName ) +{ + return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); +} + +} // namespace diff --git a/src/modules/machineid/Workers.h b/src/modules/machineid/Workers.h new file mode 100644 index 000000000..eaa093fd0 --- /dev/null +++ b/src/modules/machineid/Workers.h @@ -0,0 +1,83 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef WORKERS_H +#define WORKERS_H + +#include "Job.h" + +/// @brief Utility functions for doing the random-data stuff for MachineId +namespace MachineId +{ +/** @brief Utility functions + * + * These probably belong in libcalamares, since they're general utilities + * for moving files around in the target system. + */ + +/// @brief Remove @p fileName from the target system at @p rootMountPoint +void removeFile( const QString& rootMountPoint, const QString& fileName ); + +/// @brief Copy @p fileName from host into target system at @p rootMountPoint +Calamares::JobResult copyFile( const QString& rootMountPoint, const QString& fileName ); + + +/** @brief Entropy functions + * + * The target system may want to pre-seed the entropy pool with a suitable + * chunk of entropy data. During installation we have lots of disk access + * so plenty of entropy -- this is used mostly be Debian. + */ + +/// @brief How to generate entropy (bool-like) +enum class EntropyGeneration +{ + New, + CopyFromHost +}; + +/// @brief Returns a recommended size for the entropy pool (in bytes) +int getUrandomPoolSize(); + +/// @brief Creates a new entropy file @p fileName in the target system at @p rootMountPoint +Calamares::JobResult createNewEntropy( int poolSize, const QString& rootMountPoint, const QString& fileName ); + +/// @brief Create an entropy file @p fileName in the target system at @p rootMountPoint +Calamares::JobResult createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, const QString& fileName ); + + +/** @brief MachineID functions + * + * Creating UUIDs for DBUS and SystemD. + */ + +/// @brief How to create the DBus machine-id (bool-like) +enum class DBusGeneration +{ + New, + SymlinkFromSystemD +}; + +Calamares::JobResult createDBusMachineId( DBusGeneration kind, const QString& rootMountPoint, const QString& fileName ); + +Calamares::JobResult createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ); + + +} + +#endif // WORKERS_H From 2b9e1d6231ce727017c6ea9d6a53e65a315406ae Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 3 Oct 2019 13:32:48 +0200 Subject: [PATCH 12/18] [machineid] Add tests - Testing some of the functionality that's been added just now: - copyfile fails, buggy implementation - poolsize fails, buggy implementation - removefile not tested --- src/modules/machineid/CMakeLists.txt | 14 ++++ src/modules/machineid/MachineIdJob.cpp | 13 ++-- src/modules/machineid/Tests.cpp | 99 ++++++++++++++++++++++++++ src/modules/machineid/Workers.cpp | 2 +- src/modules/machineid/Workers.h | 5 +- 5 files changed, 125 insertions(+), 8 deletions(-) create mode 100644 src/modules/machineid/Tests.cpp diff --git a/src/modules/machineid/CMakeLists.txt b/src/modules/machineid/CMakeLists.txt index a6874fa17..efb6454e8 100644 --- a/src/modules/machineid/CMakeLists.txt +++ b/src/modules/machineid/CMakeLists.txt @@ -8,3 +8,17 @@ calamares_add_plugin( machineid calamares SHARED_LIB ) + +if ( ECM_FOUND AND BUILD_TESTING ) + ecm_add_test( + Tests.cpp + Workers.cpp + TEST_NAME + machineidtest + LINK_LIBRARIES + calamares + Qt5::Core + Qt5::Test + ) + calamares_automoc( machineidtest ) +endif() diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index 2f0416c72..9ab0ef162 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -85,8 +85,10 @@ MachineIdJob::exec() //Create new files if ( m_entropy ) { - auto r = MachineId::createEntropy( - m_entropy_copy ? MachineId::EntropyGeneration::CopyFromHost : MachineId::EntropyGeneration::New, root, target_entropy_file ); + auto r = MachineId::createEntropy( m_entropy_copy ? MachineId::EntropyGeneration::CopyFromHost + : MachineId::EntropyGeneration::New, + root, + target_entropy_file ); if ( !r ) { return r; @@ -102,9 +104,10 @@ MachineIdJob::exec() } if ( m_dbus ) { - auto r = MachineId::createDBusMachineId( m_dbus_symlink ? MachineId::DBusGeneration::SymlinkFromSystemD : MachineId::DBusGeneration::New, - root, - target_dbus_machineid_file ); + auto r = MachineId::createDBusMachineId( m_dbus_symlink ? MachineId::DBusGeneration::SymlinkFromSystemD + : MachineId::DBusGeneration::New, + root, + target_dbus_machineid_file ); if ( !r ) { return r; diff --git a/src/modules/machineid/Tests.cpp b/src/modules/machineid/Tests.cpp new file mode 100644 index 000000000..a2dcc5927 --- /dev/null +++ b/src/modules/machineid/Tests.cpp @@ -0,0 +1,99 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "Workers.h" + +#include "utils/Logger.h" + + +#include +#include +#include + +class MachineIdTests : public QObject +{ + Q_OBJECT +public: + MachineIdTests() {} + virtual ~MachineIdTests() {} + +private Q_SLOTS: + void initTestCase(); + + void testRemoveFile(); + void testCopyFile(); + + void testPoolSize(); +}; + +void +MachineIdTests::initTestCase() +{ + Logger::setupLogLevel( Logger::LOGDEBUG ); +} + +void +MachineIdTests::testCopyFile() +{ + QTemporaryDir d( QDir::tempPath() + QStringLiteral( "/test-XXXXXX" ) ); + cDebug() << "Temporary files as" << QDir::tempPath(); + cDebug() << "Temp dir file at " << d.path(); + QVERIFY( !d.path().isEmpty() ); + + QFile source( d.filePath( "example" ) ); + QVERIFY( !source.exists() ); + source.open( QIODevice::WriteOnly ); + source.write( "Derp" ); + source.close(); + QCOMPARE( source.size(), 4 ); + QVERIFY( source.exists() ); + + // This should fail since "example" isn't standard in our test directory + auto r0 = MachineId::copyFile( d.path(), "example" ); + QVERIFY( !r0 ); + + if ( QFile::exists( "CMakeCache.txt" ) ) + { + auto r1 = MachineId::copyFile( d.path(), "CMakeCache.txt" ); + QVERIFY( r1 ); + QVERIFY( QFile::exists( d.filePath( "CMakeCache.txt" ) ) ); + } +} + +void +MachineIdTests::testRemoveFile() +{ +} + +void +MachineIdTests::testPoolSize() +{ +#ifdef Q_OS_FREEBSD + // It hardly makes sense, but also the /proc entry is missing + QCOMPARE( MachineId::getUrandomPoolSize(), 512 ); +#else + // Based on a sample size of 1, Netrunner + QCOMPARE( MachineId::getUrandomPoolSize(), 4096 ); +#endif +} + + +QTEST_GUILESS_MAIN( MachineIdTests ) + +#include "Tests.moc" +#include "utils/moc-warnings.h" diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp index 7b70fab9f..284605b0f 100644 --- a/src/modules/machineid/Workers.cpp +++ b/src/modules/machineid/Workers.cpp @@ -151,4 +151,4 @@ createDBusMachineId( DBusGeneration kind, const QString& rootMountPoint, const Q return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); } -} // namespace +} // namespace MachineId diff --git a/src/modules/machineid/Workers.h b/src/modules/machineid/Workers.h index eaa093fd0..2247af45a 100644 --- a/src/modules/machineid/Workers.h +++ b/src/modules/machineid/Workers.h @@ -58,7 +58,8 @@ int getUrandomPoolSize(); Calamares::JobResult createNewEntropy( int poolSize, const QString& rootMountPoint, const QString& fileName ); /// @brief Create an entropy file @p fileName in the target system at @p rootMountPoint -Calamares::JobResult createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, const QString& fileName ); +Calamares::JobResult +createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, const QString& fileName ); /** @brief MachineID functions @@ -78,6 +79,6 @@ Calamares::JobResult createDBusMachineId( DBusGeneration kind, const QString& ro Calamares::JobResult createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ); -} +} // namespace MachineId #endif // WORKERS_H From 145855a56faa521989cfebe837dd07c65bf8822b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 4 Oct 2019 13:48:24 +0200 Subject: [PATCH 13/18] [machineid] Implement systemd machine-id creation --- src/modules/machineid/Workers.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp index 284605b0f..3557c5b84 100644 --- a/src/modules/machineid/Workers.cpp +++ b/src/modules/machineid/Workers.cpp @@ -142,7 +142,14 @@ createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, cons Calamares::JobResult createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ) { - return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); + auto cmd = QStringList { QStringLiteral( "systemd-machine-id-setup" ) }; + auto r = CalamaresUtils::System::instance()->targetEnvCommand( cmd ); + if ( r.getExitCode() ) + { + return r.explainProcess( cmd, std::chrono::seconds( 0 ) ); + } + + return Calamares::JobResult::ok(); } Calamares::JobResult From c67ac999def3400ca776b380f5b8f1503647373b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 4 Oct 2019 14:57:05 +0200 Subject: [PATCH 14/18] [machineid] Implement DBUS and systemd machine-ids - refactor running the command into a helper function, to deal with the regular if-command-failed-then-complain pattern. - mark parameters as unused. - move distinction about kind of DBus file up into the MachineIdJob and remove the enum that marked it. --- src/modules/machineid/MachineIdJob.cpp | 20 ++++++++++++++------ src/modules/machineid/Workers.cpp | 26 +++++++++++++++++++++----- src/modules/machineid/Workers.h | 12 +++++------- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index 9ab0ef162..393950ded 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -104,13 +104,21 @@ MachineIdJob::exec() } if ( m_dbus ) { - auto r = MachineId::createDBusMachineId( m_dbus_symlink ? MachineId::DBusGeneration::SymlinkFromSystemD - : MachineId::DBusGeneration::New, - root, - target_dbus_machineid_file ); - if ( !r ) + if ( m_dbus_symlink && QFile::exists( root + target_systemd_machineid_file ) ) { - return r; + auto r = MachineId::createDBusLink( root, target_dbus_machineid_file, target_systemd_machineid_file ); + if ( !r ) + { + return r; + } + } + else + { + auto r = MachineId::createDBusMachineId( root, target_dbus_machineid_file ); + if ( !r ) + { + return r; + } } } diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp index 3557c5b84..e7d5eb03f 100644 --- a/src/modules/machineid/Workers.cpp +++ b/src/modules/machineid/Workers.cpp @@ -139,10 +139,9 @@ createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, cons return createNewEntropy( poolSize, rootMountPoint, fileName ); } -Calamares::JobResult -createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ) +static Calamares::JobResult +runCmd( const QStringList& cmd ) { - auto cmd = QStringList { QStringLiteral( "systemd-machine-id-setup" ) }; auto r = CalamaresUtils::System::instance()->targetEnvCommand( cmd ); if ( r.getExitCode() ) { @@ -153,9 +152,26 @@ createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ) } Calamares::JobResult -createDBusMachineId( DBusGeneration kind, const QString& rootMountPoint, const QString& fileName ) +createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ) +{ + Q_UNUSED( rootMountPoint ) + Q_UNUSED( fileName ) + return runCmd( QStringList { QStringLiteral( "systemd-machine-id-setup" ) } ); +} + +Calamares::JobResult +createDBusMachineId( const QString& rootMountPoint, const QString& fileName ) +{ + Q_UNUSED( rootMountPoint ) + Q_UNUSED( fileName ) + return runCmd( QStringList { QStringLiteral( "dbus-uuidgen" ), QStringLiteral( "--ensure" ) } ); +} + +Calamares::JobResult +createDBusLink( const QString& rootMountPoint, const QString& fileName, const QString& systemdFileName ) { - return Calamares::JobResult::internalError( QObject::tr( "Internal Error" ), QObject::tr( "Not implemented" ), 0 ); + Q_UNUSED( rootMountPoint ); + return runCmd( QStringList { QStringLiteral( "ln" ), QStringLiteral( "-s" ), systemdFileName, fileName } ); } } // namespace MachineId diff --git a/src/modules/machineid/Workers.h b/src/modules/machineid/Workers.h index 2247af45a..5cf6270d9 100644 --- a/src/modules/machineid/Workers.h +++ b/src/modules/machineid/Workers.h @@ -67,14 +67,12 @@ createEntropy( const EntropyGeneration kind, const QString& rootMountPoint, cons * Creating UUIDs for DBUS and SystemD. */ -/// @brief How to create the DBus machine-id (bool-like) -enum class DBusGeneration -{ - New, - SymlinkFromSystemD -}; +/// @brief Create a new DBus UUID file +Calamares::JobResult createDBusMachineId( const QString& rootMountPoint, const QString& fileName ); -Calamares::JobResult createDBusMachineId( DBusGeneration kind, const QString& rootMountPoint, const QString& fileName ); +/// @brief Symlink DBus UUID file to the one from systemd (which must exist already) +Calamares::JobResult +createDBusLink( const QString& rootMountPoint, const QString& fileName, const QString& systemdFileName ); Calamares::JobResult createSystemdMachineId( const QString& rootMountPoint, const QString& fileName ); From dd47201f272797971707961537227d7a2f81414a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 4 Oct 2019 15:05:11 +0200 Subject: [PATCH 15/18] [machineid] Fix tests by appending a / - The code in Workers.cpp assumes that rootMountPoint ends in a / so that it can have filenames appended easily; make the tests fit that assumption, but still need to check that it is so in production. --- src/modules/machineid/Tests.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/modules/machineid/Tests.cpp b/src/modules/machineid/Tests.cpp index a2dcc5927..51b136b2d 100644 --- a/src/modules/machineid/Tests.cpp +++ b/src/modules/machineid/Tests.cpp @@ -63,13 +63,15 @@ MachineIdTests::testCopyFile() QCOMPARE( source.size(), 4 ); QVERIFY( source.exists() ); + QString root = d.path() + '/'; + // This should fail since "example" isn't standard in our test directory - auto r0 = MachineId::copyFile( d.path(), "example" ); + auto r0 = MachineId::copyFile( root, "example" ); QVERIFY( !r0 ); if ( QFile::exists( "CMakeCache.txt" ) ) { - auto r1 = MachineId::copyFile( d.path(), "CMakeCache.txt" ); + auto r1 = MachineId::copyFile( root, "CMakeCache.txt" ); QVERIFY( r1 ); QVERIFY( QFile::exists( d.filePath( "CMakeCache.txt" ) ) ); } From 12107b311361320d54b6e84fb4ca28eddeb9b0fa Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 4 Oct 2019 17:07:48 +0200 Subject: [PATCH 16/18] [machineid] Turn back the change in tests - the *mount* module inserts a rootMountPoint without trailing / into global storage, so we can't assume that here. On the other hand, the paths passed in to the Worker functions are absolute paths -- adjust the tests to follow that. --- src/modules/machineid/Tests.cpp | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/modules/machineid/Tests.cpp b/src/modules/machineid/Tests.cpp index 51b136b2d..cfd96bbdd 100644 --- a/src/modules/machineid/Tests.cpp +++ b/src/modules/machineid/Tests.cpp @@ -50,12 +50,16 @@ MachineIdTests::initTestCase() void MachineIdTests::testCopyFile() { - QTemporaryDir d( QDir::tempPath() + QStringLiteral( "/test-XXXXXX" ) ); + QTemporaryDir tempRoot( QDir::tempPath() + QStringLiteral( "/test-root-XXXXXX" ) ); cDebug() << "Temporary files as" << QDir::tempPath(); - cDebug() << "Temp dir file at " << d.path(); - QVERIFY( !d.path().isEmpty() ); + cDebug() << "Temp dir file at " << tempRoot.path(); + QVERIFY( !tempRoot.path().isEmpty() ); - QFile source( d.filePath( "example" ) ); + // This will pretend to be the host system + QTemporaryDir tempISOdir( QDir::tempPath() + QStringLiteral( "/test-live-XXXXXX" ) ); + QVERIFY( QDir( tempRoot.path() ).mkpath( tempRoot.path() + tempISOdir.path() ) ); + + QFile source( tempRoot.filePath( "example" ) ); QVERIFY( !source.exists() ); source.open( QIODevice::WriteOnly ); source.write( "Derp" ); @@ -63,17 +67,20 @@ MachineIdTests::testCopyFile() QCOMPARE( source.size(), 4 ); QVERIFY( source.exists() ); - QString root = d.path() + '/'; - // This should fail since "example" isn't standard in our test directory - auto r0 = MachineId::copyFile( root, "example" ); + auto r0 = MachineId::copyFile( tempRoot.path(), "example" ); QVERIFY( !r0 ); - if ( QFile::exists( "CMakeCache.txt" ) ) + const QString sampleFile = QStringLiteral( "CMakeCache.txt" ); + if ( QFile::exists( sampleFile ) ) { - auto r1 = MachineId::copyFile( root, "CMakeCache.txt" ); - QVERIFY( r1 ); - QVERIFY( QFile::exists( d.filePath( "CMakeCache.txt" ) ) ); + auto r1 = MachineId::copyFile( tempRoot.path(), sampleFile ); + // Also fail, because it's not an absolute path + QVERIFY( !r1 ); + + QVERIFY( QFile::copy( sampleFile, tempISOdir.path() + '/' + sampleFile ) ); + auto r2 = MachineId::copyFile( tempRoot.path(), tempISOdir.path() + '/' + sampleFile ); + QVERIFY( r2 ); } } From 9e359c98a9f1ef5620fa9a7e8097b99b5c6c0e17 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 4 Oct 2019 17:43:48 +0200 Subject: [PATCH 17/18] [machineid] Refactor workers, demand absolute paths --- src/modules/machineid/Workers.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp index e7d5eb03f..6b55f2489 100644 --- a/src/modules/machineid/Workers.cpp +++ b/src/modules/machineid/Workers.cpp @@ -29,16 +29,34 @@ namespace MachineId { +static inline bool +isAbsolutePath( const QString& fileName ) +{ + return fileName.startsWith( '/' ); +} + // might need to use a helper to remove the file void removeFile( const QString& rootMountPoint, const QString& fileName ) { - QFile::remove( rootMountPoint + fileName ); + if ( isAbsolutePath( fileName ) ) + { + QFile::remove( rootMountPoint + fileName ); + } + // Otherwise, do nothing } Calamares::JobResult copyFile( const QString& rootMountPoint, const QString& fileName ) { + if ( !isAbsolutePath( fileName ) ) + { + return Calamares::JobResult::internalError( + QObject::tr( "File not found" ), + QObject::tr( "Path
%1
must be an absolute path." ).arg( fileName ), + 0 ); + } + QFile f( fileName ); if ( !f.exists() ) { From 642dbf449caa4b9339ee83c1cfe0e91d856b42bb Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 4 Oct 2019 18:05:29 +0200 Subject: [PATCH 18/18] [machineid] Drop trailing \n, toInt() doesn't like it --- src/modules/machineid/Workers.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp index 6b55f2489..e6379f3f1 100644 --- a/src/modules/machineid/Workers.cpp +++ b/src/modules/machineid/Workers.cpp @@ -81,6 +81,10 @@ getUrandomPoolSize() QByteArray v = f.read( 16 ); if ( v.length() > 2 ) { + if ( v.endsWith( '\n' ) ) + { + v.chop(1); + } bool ok = false; poolSize = v.toInt( &ok ); if ( !ok ) @@ -89,7 +93,7 @@ getUrandomPoolSize() } } } - return poolSize >= minimumPoolSize ? poolSize : minimumPoolSize; + return (poolSize >= minimumPoolSize) ? poolSize : minimumPoolSize; } Calamares::JobResult