From 12661d6da926423c87ce21b2e98e85a1afab54a9 Mon Sep 17 00:00:00 2001 From: ScottH <59572507+sharder996@users.noreply.github.com> Date: Tue, 22 Jul 2025 13:40:54 +0000 Subject: [PATCH 1/4] [ssl-cert-provider] regenerate gRPC server certs on cert mismatch (#4226) The previously generated gRPC server certificate's issuer could be different than the current one in certain circumstances, like a manual installation after a snap installation of Multipass. The patch fixes that by loading both the root and the subordinate cert for the gRPC server and verifying that the root certificate is the issuer. Also added some logs to the certificate regeneration logic. Fixes #4218 MULTI-2100 --- src/cert/ssl_cert_provider.cpp | 84 +++++++- tests/test_ssl_cert_provider.cpp | 356 +++++++++++++++++++++++++++++++ 2 files changed, 434 insertions(+), 6 deletions(-) diff --git a/src/cert/ssl_cert_provider.cpp b/src/cert/ssl_cert_provider.cpp index ccb5302eee..793f942e19 100644 --- a/src/cert/ssl_cert_provider.cpp +++ b/src/cert/ssl_cert_provider.cpp @@ -16,6 +16,7 @@ */ #include +#include #include #include #include @@ -33,9 +34,11 @@ #include namespace mp = multipass; +namespace mpl = mp::logging; namespace { +constexpr auto kLogCategory = "ssl-cert-provider"; // utility function for checking return code or raw pointer from openssl C-apis // TODO: constrain T to int or raw pointer once C++20 concepts is available template @@ -196,6 +199,22 @@ void set_random_serial_number(X509* cert) openssl_check(X509_set_serialNumber(cert, serial), "Failed to set serial number!\n"); } +/** + * Check whether this certificate is the issuer (signer) of the given certificate. + * + * @param [in] signed_cert The certificate to check + * @return True if this certificate signed @p signed_cert; false otherwise. + */ +bool is_issuer_of(X509& issuer, X509& signed_cert) +{ + // Get the public key of this certificate (issuer) + std::unique_ptr pubkey{X509_get_pubkey(&issuer), + &EVP_PKEY_free}; + openssl_check(pubkey.get(), "Failed to get public key from certificate"); + // Verify that signed_cert is issued by this certificate. + return X509_verify(&signed_cert, pubkey.get()) == 1; +} + class X509Cert { public: @@ -336,6 +355,15 @@ class X509Cert std::unique_ptr cert{X509_new(), X509_free}; }; +std::unique_ptr load_cert_from_file(const std::string& path) +{ + std::unique_ptr file{fopen(path.c_str(), "r"), &fclose}; + if (!file) + return {nullptr, X509_free}; + + return {PEM_read_X509(file.get(), nullptr, nullptr, nullptr), X509_free}; +} + mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, const std::string& server_name) { @@ -351,13 +379,52 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, if (std::filesystem::exists(root_cert_path) && QFile::exists(priv_key_path) && QFile::exists(cert_path)) { - // Unlike other daemon files, the root certificate needs to be accessible by everyone - MP_PLATFORM.set_permissions(root_cert_path, - std::filesystem::perms::owner_all | - std::filesystem::perms::group_read | - std::filesystem::perms::others_read); - return {mp::utils::contents_of(cert_path), mp::utils::contents_of(priv_key_path)}; + // Ensure that we can load both certificates + const auto root_cert = load_cert_from_file(root_cert_path.string()); + const auto cert = load_cert_from_file(cert_path.toStdString()); + + if (root_cert && cert) + { + mpl::debug(kLogCategory, + "Certificates for the gRPC server (root: {}, subordinate: {}) are valid " + "X.509 files", + root_cert_path, + cert_path.toStdString()); + + // FIXME: Also check the validity period of the certificates to decide if they need + // to be re-generated + + // Validate root cert is the issuer(signer) of the subordinate certificate + if (is_issuer_of(*root_cert.get(), *cert.get())) + { + mpl::info(kLogCategory, "Re-using existing certificates for the gRPC server"); + + // Unlike other daemon files, the root certificate needs to be accessible by + // everyone + MP_PLATFORM.set_permissions(root_cert_path, + std::filesystem::perms::owner_all | + std::filesystem::perms::group_read | + std::filesystem::perms::others_read); + return {mp::utils::contents_of(cert_path), + mp::utils::contents_of(priv_key_path)}; + } + + mpl::warn(kLogCategory, + "Existing root certificate (`{}`) is not the signer of the gRPC " + "server certificate (`{}`)", + root_cert_path, + cert_path.toStdString()); + } + else + { + mpl::warn(kLogCategory, + "Could not load either of the root (`{}`) or subordinate (`{}`) " + "certificates for the gRPC server", + root_cert_path, + cert_path.toStdString()); + } } + mpl::info(kLogCategory, "Regenerating certificates for the gRPC server"); const auto priv_root_key_path = cert_dir.filePath(prefix + "_root_key.pem"); @@ -380,9 +447,14 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, { if (QFile::exists(priv_key_path) && QFile::exists(cert_path)) { + // FIXME: The client does not respect the log level and this always get printed + // even on `multipass list` + // Re-enable it after fixing. + // mpl::trace(kLogCategory, "Re-using existing certificates for the gRPC client"); return {mp::utils::contents_of(cert_path), mp::utils::contents_of(priv_key_path)}; } + // mpl::trace(kLogCategory, "Regenerating certificates for the gRPC client"); const EVPKey client_cert_key{}; const X509Cert client_cert{client_cert_key, X509Cert::CertType::Client}; client_cert_key.write(priv_key_path); diff --git a/tests/test_ssl_cert_provider.cpp b/tests/test_ssl_cert_provider.cpp index e0a876f216..40f7b578ba 100644 --- a/tests/test_ssl_cert_provider.cpp +++ b/tests/test_ssl_cert_provider.cpp @@ -17,6 +17,7 @@ #include "common.h" #include "file_operations.h" +#include "mock_logger.h" #include "mock_platform.h" #include "temp_dir.h" @@ -32,6 +33,7 @@ struct SSLCertProviderFixture : public testing::Test { mpt::TempDir temp_dir; mp::Path cert_dir{temp_dir.path() + "/test-cert"}; + mpt::MockLogger::Scope logger_scope = mpt::MockLogger::inject(); }; TEST_F(SSLCertProviderFixture, createsCertAndKey) @@ -110,3 +112,357 @@ TEST_F(SSLCertProviderFixture, createsDifferentCertsPerServerName) EXPECT_THAT(pem_cert1, StrNe(pem_cert2)); EXPECT_THAT(pem_key1, StrNe(pem_key2)); } + +TEST_F(SSLCertProviderFixture, reusesExistingValidServerCertificates) +{ + const auto [mock_platform, _] = mpt::MockPlatform::inject(); + // move the multipass_root_cert.pem into the temporary directory so it will be deleted + // automatically later + EXPECT_CALL(*mock_platform, get_root_cert_path()) + .WillRepeatedly( + Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + constexpr auto root_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIBzjCCAXWgAwIBAgIUFSHy1TV98cz/ZOvfMBXOdgH02oYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMzUwNzA4MDg1OTM5WjA9 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRowGAYDVQQDDBFNdWx0 +aXBhc3MgUm9vdCBDQTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABIq/jXfQ+0U3 +DYfNb54xG3EKiBC+SDluEOJyELQkg9kGXP2Yvh8d7tWN99bc63Bju1uAR4tWhGxP +EJt8PbSL88ajUzBRMB0GA1UdDgQWBBSOKSfZjt+7cfRspiOTU2I5a6NmVjAfBgNV +HSMEGDAWgBSOKSfZjt+7cfRspiOTU2I5a6NmVjAPBgNVHRMBAf8EBTADAQH/MAoG +CCqGSM49BAMCA0cAMEQCIChkSDoKa5iZqptHa9Ih7267WSYxx2h0nzOZxopZWUMx +AiAr+aaVzBBXe31uTuGvjiv/KccZHp1Rn/vaCOgbDxFATw== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIB2jCCAYCgAwIBAgIUCl9D+5RERQiuLKYhDXnTHb+z2QYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMjYwNzEwMDg1OTM5WjA1 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRIwEAYDVQQDDAlsb2Nh +bGhvc3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATlZbmi2M8q9SR+Cgd6C/pA +AfuqGqznWizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +o2YwZDAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFPAgeiZPxzoczlQ5 +pJrcHJWugdvYMB8GA1UdIwQYMBaAFI4pJ9mO37tx9GymI5NTYjlro2ZWMAwGA1Ud +EwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIgelfVfOSRmfsEMxxgWuZw6uMQCdFV +BZPeiPY0ZxjUPMcCIQChuXlX+ZuzLHPfv3KzCq11P3Y1dqNF4k7QQOl+Wrtl6w== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_key_contents = + R"cert(-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgGNUltvugUGTeKVu1 +0txykTDHfS2nlRGuRUCEHw5KKJuhRANCAATlZbmi2M8q9SR+Cgd6C/pAAfuqGqzn +WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +-----END PRIVATE KEY-----)cert"; + + const QDir dir{cert_dir}; + const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto cert_path = dir.filePath("localhost.pem"); + const auto key_path = dir.filePath("localhost_key.pem"); + + mpt::make_file_with_content(root_cert_path, root_cert_contents); + mpt::make_file_with_content(cert_path, subordinate_cert_contents); + mpt::make_file_with_content(key_path, subordinate_key_contents); + + logger_scope.mock_logger->expect_log(mpl::Level::debug, "are valid X.509 files"); + logger_scope.mock_logger->expect_log(mpl::Level::info, + "Re-using existing certificates for the gRPC server"); + + mp::SSLCertProvider cert_provider{cert_dir, "localhost"}; + + const auto actual_cert = cert_provider.PEM_certificate(); + const auto actual_key = cert_provider.PEM_signing_key(); + + EXPECT_THAT(actual_cert, StrEq(subordinate_cert_contents)); + EXPECT_THAT(actual_key, StrEq(subordinate_key_contents)); +} + +TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenRootCertIsCorrupt) +{ + const auto [mock_platform, _] = mpt::MockPlatform::inject(); + // move the multipass_root_cert.pem into the temporary directory so it will be deleted + // automatically later + EXPECT_CALL(*mock_platform, get_root_cert_path()) + .WillRepeatedly( + Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + constexpr auto root_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIBzjCCAXWgAwIBAgIUFSHy1TV98cz/ZOvfMBXOdgH02oYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMzUwNzA4MDg1OTM5WjA9 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRowGAYDVQQDDBFNdWx0 +aXBhc3MgUm9vdCBDQTBZMBMGBy49AgEGCCqGSM49AwEHA0IABIq/jXfQ+0U3 +DYfNb54xG3EKiBC+SDluEOJyELQkg9kGXP2Yvh8d7tWN99bc63Bju1uAR4tWhGxP +EJt8PbSL88ajUzBRMB0GA1UdDgQWBBSOKSfZjt+7cfRspiOTU2I5a6NmVjAfBgNV +HSMEGDAWgBSOKSfZjt+7cfRspiOTU2I5a6NmVjAPBgNVHRMBAf8EBTADAQH/MAoG +CCqGSM49BAMCA0cAMEQCIChkSDoKa5iZqptHa9Ih7267WSYxx2h0nzOZxopZWUMx +AiAr+aaVzBBXe31uTuGvjiv/KccZHp1Rn/vaCOgbDxFATw== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIB2jCCAYCgAwIBAgIUCl9D+5RERQiuLKYhDXnTHb+z2QYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMjYwNzEwMDg1OTM5WjA1 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRIwEAYDVQQDDAlsb2Nh +bGhvc3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATlZbmi2M8q9SR+Cgd6C/pA +AfuqGqznWizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +o2YwZDAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFPAgeiZPxzoczlQ5 +pJrcHJWugdvYMB8GA1UdIwQYMBaAFI4pJ9mO37tx9GymI5NTYjlro2ZWMAwGA1Ud +EwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIgelfVfOSRmfsEMxxgWuZw6uMQCdFV +BZPeiPY0ZxjUPMcCIQChuXlX+ZuzLHPfv3KzCq11P3Y1dqNF4k7QQOl+Wrtl6w== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_key_contents = + R"cert(-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgGNUltvugUGTeKVu1 +0txykTDHfS2nlRGuRUCEHw5KKJuhRANCAATlZbmi2M8q9SR+Cgd6C/pAAfuqGqzn +WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +-----END PRIVATE KEY-----)cert"; + + const QDir dir{cert_dir}; + const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto cert_path = dir.filePath("localhost.pem"); + const auto key_path = dir.filePath("localhost_key.pem"); + + mpt::make_file_with_content(root_cert_path, root_cert_contents); + mpt::make_file_with_content(cert_path, subordinate_cert_contents); + mpt::make_file_with_content(key_path, subordinate_key_contents); + + logger_scope.mock_logger->expect_log(mpl::Level::warning, "Could not load either of"); + logger_scope.mock_logger->expect_log(mpl::Level::info, + "Regenerating certificates for the gRPC server"); + + mp::SSLCertProvider cert_provider{cert_dir, "localhost"}; + + const auto actual_cert = cert_provider.PEM_certificate(); + const auto actual_key = cert_provider.PEM_signing_key(); + + EXPECT_THAT(actual_cert, StrNe(subordinate_cert_contents)); + EXPECT_THAT(actual_key, StrNe(subordinate_key_contents)); +} + +TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenRootCertIsMissing) +{ + const auto [mock_platform, _] = mpt::MockPlatform::inject(); + // move the multipass_root_cert.pem into the temporary directory so it will be deleted + // automatically later + EXPECT_CALL(*mock_platform, get_root_cert_path()) + .WillRepeatedly( + Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + + constexpr auto subordinate_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIB2jCCAYCgAwIBAgIUCl9D+5RERQiuLKYhDXnTHb+z2QYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMjYwNzEwMDg1OTM5WjA1 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRIwEAYDVQQDDAlsb2Nh +bGhvc3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATlZbmi2M8q9SR+Cgd6C/pA +AfuqGqznWizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +o2YwZDAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFPAgeiZPxzoczlQ5 +pJrcHJWugdvYMB8GA1UdIwQYMBaAFI4pJ9mO37tx9GymI5NTYjlro2ZWMAwGA1Ud +EwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIgelfVfOSRmfsEMxxgWuZw6uMQCdFV +BZPeiPY0ZxjUPMcCIQChuXlX+ZuzLHPfv3KzCq11P3Y1dqNF4k7QQOl+Wrtl6w== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_key_contents = + R"cert(-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgGNUltvugUGTeKVu1 +0txykTDHfS2nlRGuRUCEHw5KKJuhRANCAATlZbmi2M8q9SR+Cgd6C/pAAfuqGqzn +WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +-----END PRIVATE KEY-----)cert"; + + const QDir dir{cert_dir}; + const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto cert_path = dir.filePath("localhost.pem"); + const auto key_path = dir.filePath("localhost_key.pem"); + + mpt::make_file_with_content(cert_path, subordinate_cert_contents); + mpt::make_file_with_content(key_path, subordinate_key_contents); + + logger_scope.mock_logger->expect_log(mpl::Level::info, + "Regenerating certificates for the gRPC server"); + + mp::SSLCertProvider cert_provider{cert_dir, "localhost"}; + + const auto actual_cert = cert_provider.PEM_certificate(); + const auto actual_key = cert_provider.PEM_signing_key(); + + EXPECT_THAT(actual_cert, StrNe(subordinate_cert_contents)); + EXPECT_THAT(actual_key, StrNe(subordinate_key_contents)); +} + +TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenSubordCertIsMissing) +{ + const auto [mock_platform, _] = mpt::MockPlatform::inject(); + // move the multipass_root_cert.pem into the temporary directory so it will be deleted + // automatically later + EXPECT_CALL(*mock_platform, get_root_cert_path()) + .WillRepeatedly( + Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + + constexpr auto root_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIBzjCCAXWgAwIBAgIUFSHy1TV98cz/ZOvfMBXOdgH02oYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMzUwNzA4MDg1OTM5WjA9 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRowGAYDVQQDDBFNdWx0 +aXBhc3MgUm9vdCBDQTBZMBMGBy49AgEGCCqGSM49AwEHA0IABIq/jXfQ+0U3 +DYfNb54xG3EKiBC+SDluEOJyELQkg9kGXP2Yvh8d7tWN99bc63Bju1uAR4tWhGxP +EJt8PbSL88ajUzBRMB0GA1UdDgQWBBSOKSfZjt+7cfRspiOTU2I5a6NmVjAfBgNV +HSMEGDAWgBSOKSfZjt+7cfRspiOTU2I5a6NmVjAPBgNVHRMBAf8EBTADAQH/MAoG +CCqGSM49BAMCA0cAMEQCIChkSDoKa5iZqptHa9Ih7267WSYxx2h0nzOZxopZWUMx +AiAr+aaVzBBXe31uTuGvjiv/KccZHp1Rn/vaCOgbDxFATw== +-----END CERTIFICATE-----)cert"; + + const QDir dir{cert_dir}; + const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto cert_path = dir.filePath("localhost.pem"); + const auto key_path = dir.filePath("localhost_key.pem"); + + mpt::make_file_with_content(root_cert_path, root_cert_contents); + + logger_scope.mock_logger->expect_log(mpl::Level::info, + "Regenerating certificates for the gRPC server"); + + mp::SSLCertProvider cert_provider{cert_dir, "localhost"}; + + const auto actual_cert = cert_provider.PEM_certificate(); + const auto actual_key = cert_provider.PEM_signing_key(); + + EXPECT_THAT(actual_cert, StrNe("")); + EXPECT_THAT(actual_key, StrNe("")); +} + +TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenSubordCertIsCorrupt) +{ + const auto [mock_platform, _] = mpt::MockPlatform::inject(); + // move the multipass_root_cert.pem into the temporary directory so it will be deleted + // automatically later + EXPECT_CALL(*mock_platform, get_root_cert_path()) + .WillRepeatedly( + Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + constexpr auto root_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIBzjCCAXWgAwIBAgIUFSHy1TV98cz/ZOvfMBXOdgH02oYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMzUwNzA4MDg1OTM5WjA9 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRowGAYDVQQDDBFNdWx0 +aXBhc3MgUm9vdCBDQTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABIq/jXfQ+0U3 +DYfNb54xG3EKiBC+SDluEOJyELQkg9kGXP2Yvh8d7tWN99bc63Bju1uAR4tWhGxP +EJt8PbSL88ajUzBRMB0GA1UdDgQWBBSOKSfZjt+7cfRspiOTU2I5a6NmVjAfBgNV +HSMEGDAWgBSOKSfZjt+7cfRspiOTU2I5a6NmVjAPBgNVHRMBAf8EBTADAQH/MAoG +CCqGSM49BAMCA0cAMEQCIChkSDoKa5iZqptHa9Ih7267WSYxx2h0nzOZxopZWUMx +AiAr+aaVzBBXe31uTuGvjiv/KccZHp1Rn/vaCOgbDxFATw== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIB2jCCAYCgAwIBAgIUCl9D+5RERQiuLKYhDXnTHb+z2QYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMjYwNzEwMDg1OTM5WjA1 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRIwEAYDVQQDDAlsb2Nh +bGhvc3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATlZbmi2M8q9SR+Cgd6C/pA +AfuqGqznWizZyQgYv6Z/AosKpE6YXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +o2YwZDAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFPAgeiZPxzoczlQ5 +pJrcHJWugdvYMB8GA1UdIwQYMBaAFI4pJ9mO37tx9GymI5NTYjlro2ZWMAwGA1Ud +EwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIgelfVfOSRmfsEMxxgWuZw6uMQCdFV +BZPeiPY0ZxjUPMcCIQChuXlX+ZuzLHPfv3KzCq11P3Y1dqNF4k7QQOl+Wrtl6w== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_key_contents = + R"cert(-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgGNUltvugUGTeKVu1 +0txykTDHfS2nlRGuRUCEHw5KKJuhRANCAATlZbmi2M8q9SR+Cgd6C/pAAfuqGqzn +WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +-----END PRIVATE KEY-----)cert"; + + const QDir dir{cert_dir}; + const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto cert_path = dir.filePath("localhost.pem"); + const auto key_path = dir.filePath("localhost_key.pem"); + + mpt::make_file_with_content(root_cert_path, root_cert_contents); + mpt::make_file_with_content(cert_path, subordinate_cert_contents); + mpt::make_file_with_content(key_path, subordinate_key_contents); + + logger_scope.mock_logger->expect_log(mpl::Level::warning, "Could not load either of"); + logger_scope.mock_logger->expect_log(mpl::Level::info, + "Regenerating certificates for the gRPC server"); + + mp::SSLCertProvider cert_provider{cert_dir, "localhost"}; + + const auto actual_cert = cert_provider.PEM_certificate(); + const auto actual_key = cert_provider.PEM_signing_key(); + + EXPECT_THAT(actual_cert, StrNe(subordinate_cert_contents)); + EXPECT_THAT(actual_key, StrNe(subordinate_key_contents)); +} + +TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenRootCertIsNotTheIssuer) +{ + const auto [mock_platform, _] = mpt::MockPlatform::inject(); + // move the multipass_root_cert.pem into the temporary directory so it will be deleted + // automatically later + EXPECT_CALL(*mock_platform, get_root_cert_path()) + .WillRepeatedly( + Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + constexpr auto root_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIBzzCCAXWgAwIBAgIUAOtHJnyORHMBZb1g3Lub65TFMkgwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMTAzODU2WhcNMzUwNzA4MTAzODU2WjA9 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRowGAYDVQQDDBFNdWx0 +aXBhc3MgUm9vdCBDQTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABOZrJ6hk0ARU +fYynzij63WUdFGXASPzrshLxxzum2FfADoCHfj28O6OiVFTBF5T0q39oHGaxclgV +99fJFjgw0GCjUzBRMB0GA1UdDgQWBBSJAiHfyxwx7DC5ru7QGjFr35H0xTAfBgNV +HSMEGDAWgBSJAiHfyxwx7DC5ru7QGjFr35H0xTAPBgNVHRMBAf8EBTADAQH/MAoG +CCqGSM49BAMCA0gAMEUCICVVwSYOZqyPmd1aXkVCDQHMFm6hOM7hFs/6SRBQqAWZ +AiEAmhbAFiEUSyjZj5MVOhw1TW6NxGe2+45ypLULJaOAE0g= +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_cert_contents = + R"cert(-----BEGIN CERTIFICATE----- +MIIB2jCCAYCgAwIBAgIUCl9D+5RERQiuLKYhDXnTHb+z2QYwCgYIKoZIzj0EAwIw +PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEaMBgGA1UEAwwRTXVs +dGlwYXNzIFJvb3QgQ0EwHhcNMjUwNzEwMDg1OTM5WhcNMjYwNzEwMDg1OTM5WjA1 +MQswCQYDVQQGEwJVUzESMBAGA1UECgwJQ2Fub25pY2FsMRIwEAYDVQQDDAlsb2Nh +bGhvc3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATlZbmi2M8q9SR+Cgd6C/pA +AfuqGqznWizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +o2YwZDAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFPAgeiZPxzoczlQ5 +pJrcHJWugdvYMB8GA1UdIwQYMBaAFI4pJ9mO37tx9GymI5NTYjlro2ZWMAwGA1Ud +EwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIgelfVfOSRmfsEMxxgWuZw6uMQCdFV +BZPeiPY0ZxjUPMcCIQChuXlX+ZuzLHPfv3KzCq11P3Y1dqNF4k7QQOl+Wrtl6w== +-----END CERTIFICATE-----)cert"; + + constexpr auto subordinate_key_contents = + R"cert(-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgGNUltvugUGTeKVu1 +0txykTDHfS2nlRGuRUCEHw5KKJuhRANCAATlZbmi2M8q9SR+Cgd6C/pAAfuqGqzn +WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg +-----END PRIVATE KEY-----)cert"; + + const QDir dir{cert_dir}; + const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto cert_path = dir.filePath("localhost.pem"); + const auto key_path = dir.filePath("localhost_key.pem"); + + mpt::make_file_with_content(root_cert_path, root_cert_contents); + mpt::make_file_with_content(cert_path, subordinate_cert_contents); + mpt::make_file_with_content(key_path, subordinate_key_contents); + + logger_scope.mock_logger->expect_log(mpl::Level::debug, "are valid X.509 files"); + logger_scope.mock_logger->expect_log(mpl::Level::warning, + "is not the signer of the gRPC server certificate"); + logger_scope.mock_logger->expect_log(mpl::Level::info, + "Regenerating certificates for the gRPC server"); + + mp::SSLCertProvider cert_provider{cert_dir, "localhost"}; + + const auto actual_cert = cert_provider.PEM_certificate(); + const auto actual_key = cert_provider.PEM_signing_key(); + + EXPECT_THAT(actual_cert, StrNe(subordinate_cert_contents)); + EXPECT_THAT(actual_key, StrNe(subordinate_key_contents)); +} From 10c95d5ab24479e249950e701b4ca701f77a92d1 Mon Sep 17 00:00:00 2001 From: ScottH <59572507+sharder996@users.noreply.github.com> Date: Thu, 31 Jul 2025 14:46:38 +0000 Subject: [PATCH 2/4] Fix root certificate not found when using external directory (#4252) Changes the logic of where root certificates are stored on all platforms to always be in the same location. This avoids having the client and daemon looking in different locations. fixes #4246 MULTI-2104 --- ...re-where-multipass-stores-external-data.md | 37 ++++++++++-- include/multipass/platform.h | 10 +++- src/daemon/daemon_config.cpp | 3 + src/platform/platform_linux.cpp | 21 +++---- src/platform/platform_osx.cpp | 5 +- src/platform/platform_win.cpp | 32 +++++++---- tests/linux/test_platform_linux.cpp | 10 +++- tests/mock_platform.h | 2 +- tests/test_client_common.cpp | 7 +++ tests/test_daemon.cpp | 15 ++++- tests/test_ssl_cert_provider.cpp | 57 ++++++++++--------- 11 files changed, 132 insertions(+), 67 deletions(-) diff --git a/docs/how-to-guides/customise-multipass/configure-where-multipass-stores-external-data.md b/docs/how-to-guides/customise-multipass/configure-where-multipass-stores-external-data.md index f35d1cdd87..38a1796ff2 100644 --- a/docs/how-to-guides/customise-multipass/configure-where-multipass-stores-external-data.md +++ b/docs/how-to-guides/customise-multipass/configure-where-multipass-stores-external-data.md @@ -9,7 +9,7 @@ This document demonstrates how to configure the location where Multipass stores ```{caution} **Caveats:** - Multipass will not migrate your existing data; this article explains how to do it manually. If you do not transfer the data, you will have to re-download any Ubuntu images and reinitialise any instances that you need. -- When uninstalling Multipass, the uninstaller will not remove data stored in custom locations, so you'll have to deleted it manually. +- When uninstalling Multipass, the uninstaller will not remove data stored in custom locations, so you'll have to delete it manually. ``` `````{tabs} @@ -80,7 +80,7 @@ sudo snap start multipass You can delete the original data at your discretion, to free up space: ```{code-block} text -sudo rm -rf /var/snap/multipass/common/data/multipassd +sudo rm -rf /var/snap/multipass/common/data/multipassd/vault sudo rm -rf /var/snap/multipass/common/cache/multipassd ``` @@ -128,6 +128,12 @@ launchctl load /Library/LaunchDaemons/com.canonical.multipassd.plist First, open a PowerShell prompt with administration privileges. +Stop Multipass instances: + +```{code-block} powershell +multipass stop --all +``` + Stop the Multipass daemon: ```{code-block} powershell @@ -144,13 +150,18 @@ Set-ItemProperty -Path "HKLM:System\CurrentControlSet\Control\Session Manager\En Now you can transfer the data from its original location to the new location: ```{code-block} powershell -Copy-Item -Path "C:\ProgramData\Multipass\*" -Destination "" -Recurse +Copy-Item -Path "C:\ProgramData\Multipass\*" -Recurse -Force -Destination "" ``` ```{caution} It is important to copy any existing data to the new location. This avoids unauthenticated client issues, permission issues, and in general, to have any previously created instances available. ``` +You also need to edit several settings so that the specified paths point to the new Multipass storage directory, otherwise your instances will fail to start: + +* `/data/vault/multipassd-instance-image-records.json`: Update the "path" key for each instance. +* Open Hyper-V Manager > For each instance right-click the instance and open the settings. Navigate to SCSI Controller > Hard Drive and update the Media path. Do the same for SCSI Controller > DVD Drive > Media Image file. + Finally, start the Multipass daemon: ```{code-block} powershell @@ -160,7 +171,8 @@ Start-Service Multipass You can delete the original data at your discretion, to free up space: ```{code-block} powershell -Remove-Item -Path "C:\ProgramData\Multipass\*" -Recurse +Remove-Item -Path "C:\ProgramData\Multipass\cache\*" -Recurse +Remove-Item -Path "C:\ProgramData\Multipass\data\vault\*" -Recurse ``` ```` @@ -200,6 +212,11 @@ sudo cp -r /data /var/snap/multipass/common/data/multipassd sudo cp -r /cache /var/snap/multipass/common/cache/multipassd ``` +You also need to edit the following configuration files so that the specified paths point to the original Multipass storage directory, otherwise your instances will fail to start: + +* `multipass-vm-instances.json`: Update the absolute path of the instance images in the "arguments" key for each instance. +* `vault/multipassd-instance-image-records.json`: Update the "path" key for each instance. + Finally, start the Multipass daemon: ```{code-block} text @@ -252,6 +269,12 @@ launchctl load /Library/LaunchDaemons/com.canonical.multipassd.plist First, open a PowerShell prompt with administrator privileges. +Stop Multipass instances: + +```{code-block} powershell +multipass stop --all +``` + Stop the Multipass daemon: ```{code-block} powershell @@ -267,9 +290,11 @@ Remove-ItemProperty -Path "HKLM:System\CurrentControlSet\Control\Session Manager Now you can transfer the data back to its original location: ```{code-block} powershell -Copy-Item -Path "\*" -Destination "C:\ProgramData\Multipass" -Recurse +Copy-Item -Path "\*" -Destination "C:\ProgramData\Multipass" -Recurse -Force ``` +Follow the same instructions from setting up the custom image location to update the paths to their original location. + Finally, start the Multipass daemon: ```{code-block} powershell @@ -279,7 +304,7 @@ Start-Service Multipass You can delete the data from the custom location at your discretion, to free up space: ```{code-block} powershell -Remove-Item -Path "" -Recurse +Remove-Item -Path "" -Recurse -Force ``` ```` diff --git a/include/multipass/platform.h b/include/multipass/platform.h index 106563c761..26e44e94fc 100644 --- a/include/multipass/platform.h +++ b/include/multipass/platform.h @@ -78,7 +78,9 @@ class Platform : public Singleton [[nodiscard]] virtual std::string bridge_nomenclature() const; virtual int get_cpus() const; virtual long long get_total_ram() const; - [[nodiscard]] virtual std::filesystem::path get_root_cert_path() const; + + [[nodiscard]] virtual std::filesystem::path get_root_cert_dir() const; + [[nodiscard]] std::filesystem::path get_root_cert_path() const; }; QString interpret_setting(const QString& key, const QString& val); @@ -112,3 +114,9 @@ std::string host_version(); inline multipass::platform::Platform::Platform(const PrivatePass& pass) noexcept : Singleton(pass) { } + +inline std::filesystem::path multipass::platform::Platform::get_root_cert_path() const +{ + constexpr auto* root_cert_file_name = "multipass_root_cert.pem"; + return get_root_cert_dir() / root_cert_file_name; +} diff --git a/src/daemon/daemon_config.cpp b/src/daemon/daemon_config.cpp index a46ae36c48..2140cdc5aa 100644 --- a/src/daemon/daemon_config.cpp +++ b/src/daemon/daemon_config.cpp @@ -133,6 +133,9 @@ std::unique_ptr mp::DaemonConfigBuilder::build() auto multiplexing_logger = std::make_shared(std::move(logger)); mpl::set_logger(multiplexing_logger); + MP_UTILS.make_dir(QString::fromStdU16String(MP_PLATFORM.get_root_cert_dir().u16string()), + fs::perms::owner_all | fs::perms::group_exec | fs::perms::others_exec); + MP_PLATFORM.setup_permission_inheritance(true); auto storage_path = MP_PLATFORM.multipass_storage_location(); diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index f19fbd2c69..987eaeffdf 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -189,14 +189,6 @@ std::string get_alias_script_path(const std::string& alias) return aliases_folder.absoluteFilePath(QString::fromStdString(alias)).toStdString(); } - -std::filesystem::path multipass_final_storage_location() -{ - const auto user_specified_mp_storage = MP_PLATFORM.multipass_storage_location(); - const auto mp_final_storage = user_specified_mp_storage.isEmpty() ? mp::utils::snap_common_dir() - : user_specified_mp_storage; - return std::filesystem::path{mp_final_storage.toStdString()}; -} } // namespace std::unique_ptr multipass::platform::detail::find_os_release() @@ -471,11 +463,12 @@ std::string multipass::platform::host_version() : fmt::format("{}-{}", QSysInfo::productType(), QSysInfo::productVersion()); } -std::filesystem::path mp::platform::Platform::get_root_cert_path() const +std::filesystem::path mp::platform::Platform::get_root_cert_dir() const { - constexpr auto* root_cert_file_name = "multipass_root_cert.pem"; - return mp::utils::in_multipass_snap() - ? multipass_final_storage_location() / "data" / daemon_name / "certificates" / - root_cert_file_name - : std::filesystem::path{"/usr/local/share/ca-certificates"} / root_cert_file_name; + using Path = std::filesystem::path; + const auto base_dir = utils::in_multipass_snap() + ? Path{utils::snap_common_dir().toStdString()} / "data" + : Path{"/usr/local/etc"}; + + return base_dir / daemon_name; } diff --git a/src/platform/platform_osx.cpp b/src/platform/platform_osx.cpp index f6d354474a..a01d51b71b 100644 --- a/src/platform/platform_osx.cpp +++ b/src/platform/platform_osx.cpp @@ -386,7 +386,8 @@ std::string mp::platform::reinterpret_interface_id(const std::string& ux_id) return ux_id; } -std::filesystem::path mp::platform::Platform::get_root_cert_path() const +std::filesystem::path mp::platform::Platform::get_root_cert_dir() const { - return std::filesystem::path{"/Library/Keychains/multipass_root_cert.pem"}; + static const std::filesystem::path base_dir = "/usr/local/etc"; + return base_dir / daemon_name; } diff --git a/src/platform/platform_win.cpp b/src/platform/platform_win.cpp index a1c31240e8..cc7befb0b9 100644 --- a/src/platform/platform_win.cpp +++ b/src/platform/platform_win.cpp @@ -49,6 +49,7 @@ #include #include +#include #include #include @@ -347,6 +348,19 @@ QString systemprofile_app_data_path() return ret; } +[[nodiscard]] mp::fs::path get_wellknown_path(REFKNOWNFOLDERID rfid) +{ + PWSTR out = nullptr; + auto guard = sg::make_scope_guard([&out]() noexcept { ::CoTaskMemFree(out); }); + + if (auto err = SHGetKnownFolderPath(rfid, KF_FLAG_DEFAULT, NULL, &out); err != S_OK) + { + throw std::system_error(err, std::system_category(), "Failed to get well known path"); + } + + return out; +} + DWORD set_privilege(HANDLE handle, LPCTSTR privilege, bool enable) { LUID id; @@ -496,16 +510,6 @@ BOOL signal_handler(DWORD dwCtrlType) return FALSE; } } - -std::filesystem::path multipass_final_storage_location() -{ - const auto user_specified_mp_storage = MP_PLATFORM.multipass_storage_location(); - const auto mp_final_storage = - user_specified_mp_storage.isEmpty() - ? MP_STDPATHS.writableLocation(mp::StandardPaths::AppDataLocation) - : user_specified_mp_storage; - return std::filesystem::path{mp_final_storage.toStdString()}; -} } // namespace std::map @@ -1064,7 +1068,11 @@ long long mp::platform::Platform::get_total_ram() const return status.ullTotalPhys; } -std::filesystem::path mp::platform::Platform::get_root_cert_path() const +std::filesystem::path mp::platform::Platform::get_root_cert_dir() const { - return multipass_final_storage_location() / "data" / "certificates" / "multipass_root_cert.pem"; + // FOLDERID_ProgramData returns C:\ProgramData normally + const auto base_dir = get_wellknown_path(FOLDERID_ProgramData); + + // Windows doesn't use `daemon_name` for the data directory (see `program_data_multipass_path`) + return base_dir / "Multipass" / "data"; } diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index f34db12c07..29fecc0d73 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -697,12 +697,16 @@ TEST_F(PlatformLinux, removeAliasScriptThrowsIfCannotRemoveScript) mpt::match_what(StrEq("No such file or directory"))); } -TEST_F(PlatformLinux, testSnapMultipassStorageLocation) +TEST_F(PlatformLinux, testSnapMultipassCertLocation) { + const auto unconfined_location = MP_PLATFORM.get_root_cert_path(); + mpt::SetEnvScope env{"SNAP_NAME", "multipass"}; mpt::SetEnvScope env2("SNAP_COMMON", "common"); - EXPECT_EQ(MP_PLATFORM.get_root_cert_path(), - "data/multipassd/certificates/multipass_root_cert.pem"); + const auto snap_location = MP_PLATFORM.get_root_cert_path(); + + EXPECT_NE(snap_location, unconfined_location); + EXPECT_EQ(snap_location.filename(), unconfined_location.filename()); } } // namespace diff --git a/tests/mock_platform.h b/tests/mock_platform.h index 6502e53854..5499d0e7c9 100644 --- a/tests/mock_platform.h +++ b/tests/mock_platform.h @@ -66,7 +66,7 @@ class MockPlatform : public platform::Platform MOCK_METHOD(QString, default_privileged_mounts, (), (const, override)); MOCK_METHOD(QString, get_username, (), (const, override)); MOCK_METHOD(std::string, bridge_nomenclature, (), (const, override)); - MOCK_METHOD(std::filesystem::path, get_root_cert_path, (), (const, override)); + MOCK_METHOD(std::filesystem::path, get_root_cert_dir, (), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockPlatform, Platform); }; diff --git a/tests/test_client_common.cpp b/tests/test_client_common.cpp index ff6719b671..fb8c4192f8 100644 --- a/tests/test_client_common.cpp +++ b/tests/test_client_common.cpp @@ -23,6 +23,7 @@ #include "mock_client_rpc.h" #include "mock_daemon.h" #include "mock_permission_utils.h" +#include "mock_platform.h" #include "mock_standard_paths.h" #include "mock_utils.h" #include "stub_terminal.h" @@ -102,6 +103,12 @@ TEST_F(TestClientCommon, noValidCertsCreatesNewCommonCert) { const auto common_cert_dir = temp_dir.path() + mp::common_client_cert_dir; + const auto [mock_platform, _] = mpt::MockPlatform::inject(); + // move the multipass_root_cert.pem into the temporary directory so it will be deleted + // automatically later + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{common_cert_dir.toStdU16String()})); + EXPECT_CALL(*mock_cert_store, empty).WillOnce(Return(false)); config_builder.client_cert_store = std::move(mock_cert_store); diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index b48c04e515..5012ef44f1 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -353,7 +353,6 @@ TEST_F(Daemon, dataPathWithStorageValid) QTemporaryDir storage_dir; mpt::SetEnvScope storage(mp::multipass_storage_env_var, storage_dir.path().toUtf8()); - EXPECT_CALL(mpt::MockStandardPaths::mock_instance(), writableLocation(_)).Times(0); EXPECT_CALL(mock_platform, multipass_storage_location()) .WillOnce(Return(mp::utils::get_multipass_storage())); @@ -372,6 +371,19 @@ TEST_F(Daemon, dataPathWithStorageValid) EXPECT_EQ(config->cache_directory, storage_dir.filePath("cache")); } +TEST_F(Daemon, rootCertPathDoesntChangeWithStorage) +{ + QTemporaryDir storage_dir; + + const auto base_location = MP_PLATFORM.get_root_cert_path(); + const auto base_storage = mp::utils::get_multipass_storage(); + + mpt::SetEnvScope storage(mp::multipass_storage_env_var, storage_dir.path().toUtf8()); + + EXPECT_EQ(base_location, MP_PLATFORM.get_root_cert_path()); + EXPECT_NE(base_storage, mp::utils::get_multipass_storage()); +} + TEST_F(Daemon, blueprintsDownloadsFromCorrectURL) { mpt::TempDir cache_dir; @@ -997,6 +1009,7 @@ TEST_F(DaemonCreateLaunchAliasTestSuite, blueprintFoundMountsWorkspaceConfined) mpt::TempDir temp_dir; mpt::SetEnvScope env_scope1("SNAP_NAME", "multipass"); mpt::SetEnvScope env_scope2("SNAP_REAL_HOME", temp_dir.path().toUtf8()); + mpt::SetEnvScope env_scope3("SNAP_COMMON", temp_dir.filePath("common").toUtf8()); config_builder.blueprint_provider = std::move(mock_blueprint_provider); config_builder.vault = std::move(mock_image_vault); diff --git a/tests/test_ssl_cert_provider.cpp b/tests/test_ssl_cert_provider.cpp index 40f7b578ba..9e202db375 100644 --- a/tests/test_ssl_cert_provider.cpp +++ b/tests/test_ssl_cert_provider.cpp @@ -98,9 +98,8 @@ TEST_F(SSLCertProviderFixture, createsDifferentCertsPerServerName) const auto [mock_platform, _] = mpt::MockPlatform::inject(); // move the multipass_root_cert.pem into the temporary directory so it will be deleted // automatically later - EXPECT_CALL(*mock_platform, get_root_cert_path()) - .WillRepeatedly( - Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{cert_dir.toStdU16String()})); mp::SSLCertProvider cert_provider1{cert_dir, "test_server1"}; mp::SSLCertProvider cert_provider2{cert_dir, "test_server2"}; @@ -118,9 +117,9 @@ TEST_F(SSLCertProviderFixture, reusesExistingValidServerCertificates) const auto [mock_platform, _] = mpt::MockPlatform::inject(); // move the multipass_root_cert.pem into the temporary directory so it will be deleted // automatically later - EXPECT_CALL(*mock_platform, get_root_cert_path()) - .WillRepeatedly( - Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{cert_dir.toStdU16String()})); + constexpr auto root_cert_contents = R"cert(-----BEGIN CERTIFICATE----- MIIBzjCCAXWgAwIBAgIUFSHy1TV98cz/ZOvfMBXOdgH02oYwCgYIKoZIzj0EAwIw @@ -157,7 +156,8 @@ WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg -----END PRIVATE KEY-----)cert"; const QDir dir{cert_dir}; - const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto root_cert_path = + QString::fromStdU16String(MP_PLATFORM.get_root_cert_path().u16string()); const auto cert_path = dir.filePath("localhost.pem"); const auto key_path = dir.filePath("localhost_key.pem"); @@ -183,9 +183,9 @@ TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenRootCertIsCorrupt) const auto [mock_platform, _] = mpt::MockPlatform::inject(); // move the multipass_root_cert.pem into the temporary directory so it will be deleted // automatically later - EXPECT_CALL(*mock_platform, get_root_cert_path()) - .WillRepeatedly( - Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{cert_dir.toStdU16String()})); + constexpr auto root_cert_contents = R"cert(-----BEGIN CERTIFICATE----- MIIBzjCCAXWgAwIBAgIUFSHy1TV98cz/ZOvfMBXOdgH02oYwCgYIKoZIzj0EAwIw @@ -222,7 +222,8 @@ WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg -----END PRIVATE KEY-----)cert"; const QDir dir{cert_dir}; - const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto root_cert_path = + QString::fromStdU16String(MP_PLATFORM.get_root_cert_path().u16string()); const auto cert_path = dir.filePath("localhost.pem"); const auto key_path = dir.filePath("localhost_key.pem"); @@ -248,9 +249,8 @@ TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenRootCertIsMissing) const auto [mock_platform, _] = mpt::MockPlatform::inject(); // move the multipass_root_cert.pem into the temporary directory so it will be deleted // automatically later - EXPECT_CALL(*mock_platform, get_root_cert_path()) - .WillRepeatedly( - Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{cert_dir.toStdU16String()})); constexpr auto subordinate_cert_contents = R"cert(-----BEGIN CERTIFICATE----- @@ -274,7 +274,8 @@ WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg -----END PRIVATE KEY-----)cert"; const QDir dir{cert_dir}; - const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto root_cert_path = + QString::fromStdU16String(MP_PLATFORM.get_root_cert_path().u16string()); const auto cert_path = dir.filePath("localhost.pem"); const auto key_path = dir.filePath("localhost_key.pem"); @@ -298,9 +299,8 @@ TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenSubordCertIsMissing) const auto [mock_platform, _] = mpt::MockPlatform::inject(); // move the multipass_root_cert.pem into the temporary directory so it will be deleted // automatically later - EXPECT_CALL(*mock_platform, get_root_cert_path()) - .WillRepeatedly( - Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{cert_dir.toStdU16String()})); constexpr auto root_cert_contents = R"cert(-----BEGIN CERTIFICATE----- @@ -317,7 +317,8 @@ AiAr+aaVzBBXe31uTuGvjiv/KccZHp1Rn/vaCOgbDxFATw== -----END CERTIFICATE-----)cert"; const QDir dir{cert_dir}; - const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto root_cert_path = + QString::fromStdU16String(MP_PLATFORM.get_root_cert_path().u16string()); const auto cert_path = dir.filePath("localhost.pem"); const auto key_path = dir.filePath("localhost_key.pem"); @@ -340,9 +341,9 @@ TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenSubordCertIsCorrupt) const auto [mock_platform, _] = mpt::MockPlatform::inject(); // move the multipass_root_cert.pem into the temporary directory so it will be deleted // automatically later - EXPECT_CALL(*mock_platform, get_root_cert_path()) - .WillRepeatedly( - Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{cert_dir.toStdU16String()})); + constexpr auto root_cert_contents = R"cert(-----BEGIN CERTIFICATE----- MIIBzjCCAXWgAwIBAgIUFSHy1TV98cz/ZOvfMBXOdgH02oYwCgYIKoZIzj0EAwIw @@ -379,7 +380,8 @@ WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg -----END PRIVATE KEY-----)cert"; const QDir dir{cert_dir}; - const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto root_cert_path = + QString::fromStdU16String(MP_PLATFORM.get_root_cert_path().u16string()); const auto cert_path = dir.filePath("localhost.pem"); const auto key_path = dir.filePath("localhost_key.pem"); @@ -405,9 +407,9 @@ TEST_F(SSLCertProviderFixture, regeneratesCertificatesWhenRootCertIsNotTheIssuer const auto [mock_platform, _] = mpt::MockPlatform::inject(); // move the multipass_root_cert.pem into the temporary directory so it will be deleted // automatically later - EXPECT_CALL(*mock_platform, get_root_cert_path()) - .WillRepeatedly( - Return(std::filesystem::path{cert_dir.toStdU16String()} / "multipass_root_cert.pem")); + EXPECT_CALL(*mock_platform, get_root_cert_dir()) + .WillRepeatedly(Return(std::filesystem::path{cert_dir.toStdU16String()})); + constexpr auto root_cert_contents = R"cert(-----BEGIN CERTIFICATE----- MIIBzzCCAXWgAwIBAgIUAOtHJnyORHMBZb1g3Lub65TFMkgwCgYIKoZIzj0EAwIw @@ -444,7 +446,8 @@ WizZyQgYv6Z/AosKpE6DcyIYXGuGn2U/Icpxsn/ZycRel2shM4dP5OBg -----END PRIVATE KEY-----)cert"; const QDir dir{cert_dir}; - const auto root_cert_path = dir.filePath("multipass_root_cert.pem"); + const auto root_cert_path = + QString::fromStdU16String(MP_PLATFORM.get_root_cert_path().u16string()); const auto cert_path = dir.filePath("localhost.pem"); const auto key_path = dir.filePath("localhost_key.pem"); From fc9aa5e0a9529d35a01932a9a861d6da9ecb4af4 Mon Sep 17 00:00:00 2001 From: ScottH <59572507+sharder996@users.noreply.github.com> Date: Thu, 14 Aug 2025 18:45:30 +0000 Subject: [PATCH 3/4] [sshfs-mount] make chown return 0 on Windows (#4288) chown is a platform-specific function which depends uid and gid. these values are not available in Windows, and the chown impl is by default returns -1. This causes filesystem operation failures in Windows hosts when sshfs mounts are used in guests. One example is as follows: ubuntu@foo:~/Downloads$ echo "test" > foo.txt -bash: foo.txt: Operation not permitted This operation fails because it first lands in handle_open, which eventually calls chown to change the file's owner, which is a failure by default in Windows. The patch fixes that by introducing a shim function, chown_shim which returns success when either uid or gid is set to no_id_info_available. MULTI-2155 --- src/platform/platform_win.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/platform/platform_win.cpp b/src/platform/platform_win.cpp index cc7befb0b9..897b8c1e38 100644 --- a/src/platform/platform_win.cpp +++ b/src/platform/platform_win.cpp @@ -73,6 +73,7 @@ namespace mpu = multipass::utils; namespace { static const auto none = QStringLiteral("none"); +static constexpr auto kLogCategory = "platform-win"; time_t time_t_from(const FILETIME* ft) { @@ -690,7 +691,12 @@ mp::UpdatePrompt::UPtr mp::platform::make_update_prompt() int mp::platform::Platform::chown(const char* path, unsigned int uid, unsigned int gid) const { - return -1; + logging::trace(kLogCategory, + "chown() called for `{}` (uid: {}, gid: {}) but it's no-op.", + path, + uid, + gid); + return 0; } // new_ACL returns a new ACL unique ptr that retains all existing Hyper-V ACEs or NULL if no entries From 0e82c4bb73440675e7518b95796e4d2702b3f3f9 Mon Sep 17 00:00:00 2001 From: ScottH <59572507+sharder996@users.noreply.github.com> Date: Thu, 14 Aug 2025 16:42:10 +0000 Subject: [PATCH 4/4] [ci] Install distlib on all macOS versions (#4308) Failing on macOS-14 now https://github.com/canonical/multipass/actions/runs/16967777225/job/48096393820?pr=4294 --- .github/workflows/windows-macos.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/windows-macos.yml b/.github/workflows/windows-macos.yml index 347370a3d1..07263ada0f 100644 --- a/.github/workflows/windows-macos.yml +++ b/.github/workflows/windows-macos.yml @@ -148,6 +148,11 @@ jobs: python \ wget + - name: Install dependencies from pip + if: ${{ runner.os == 'macOS' }} + run: | + ${ARCH_WRAPPER} python3 -m pip install --user --upgrade distlib + - name: Install specific QEMU from Choco if: ${{ runner.os == 'Windows' }} uses: crazy-max/ghaction-chocolatey@v3