Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create dynamic libraries with C interfaces for Dart FFI #3098

Merged
merged 2 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 3rd-party/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ option(YAML_CPP_BUILD_CONTRIB OFF)
option(YAML_CPP_BUILD_TESTS OFF)

add_subdirectory(yaml-cpp EXCLUDE_FROM_ALL)
set_target_properties(yaml-cpp PROPERTIES POSITION_INDEPENDENT_CODE ON)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need PIC here? Anyway, I'm OK with it, especially because it's third party. Just asking out of curiosity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without it, I was simply getting a compilation error that said it needs PIC to compile some functions in that library :)


add_library(yaml INTERFACE)

Expand Down Expand Up @@ -121,6 +122,7 @@ set(BUILD_SHARED_LIBS OFF)
add_subdirectory(semver EXCLUDE_FROM_ALL)
target_include_directories(semver INTERFACE
${CMAKE_CURRENT_SOURCE_DIR}/semver/include)
set_target_properties(semver PROPERTIES POSITION_INDEPENDENT_CODE ON)

# xz-decoder config
add_subdirectory(xz-decoder EXCLUDE_FROM_ALL)
Expand Down
4 changes: 2 additions & 2 deletions include/multipass/cli/client_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ namespace client
{
QString persistent_settings_filename();
void register_global_settings_handlers();
std::shared_ptr<grpc::Channel> make_channel(const std::string& server_address, CertProvider* cert_provider);
std::shared_ptr<grpc::Channel> make_channel(const std::string& server_address, const CertProvider& cert_provider);
std::string get_server_address();
std::unique_ptr<SSLCertProvider> get_cert_provider();
std::unique_ptr<SSLCertProvider> get_cert_provider(const std::string& server_address);
void set_logger();
void set_logger(multipass::logging::Level verbosity); // full param qualification makes sure msvc is happy
void pre_setup();
Expand Down
18 changes: 18 additions & 0 deletions include/multipass/dart_ffi.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#ifndef MULTIPASS_DART_FFI_H
#define MULTIPASS_DART_FFI_H

extern "C" const char* multipass_version();

extern "C" const char* generate_petname();

extern "C" const char* get_server_address();

extern "C" struct KeyCertificatePair
{
const char* pem_cert;
const char* pem_priv_key;
};

extern "C" struct KeyCertificatePair get_cert_pair();

#endif // MULTIPASS_DART_FFI_H
2 changes: 1 addition & 1 deletion src/client/cli/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ auto make_handler_unregisterer(mp::SettingsHandler* handler)
} // namespace

mp::Client::Client(ClientConfig& config)
: stub{mp::Rpc::NewStub(mp::client::make_channel(config.server_address, config.cert_provider.get()))},
: stub{mp::Rpc::NewStub(mp::client::make_channel(config.server_address, *config.cert_provider))},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this config->cert_provider?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. We need to pass a reference to make_channel and cert_provider in this case is a unique_ptr, so we need * to get that reference.

term{config.term},
aliases{config.term}
{
Expand Down
4 changes: 3 additions & 1 deletion src/client/cli/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ int main_impl(int argc, char* argv[])

mp::client::register_global_settings_handlers();

mp::ClientConfig config{mp::client::get_server_address(), mp::client::get_cert_provider(), term.get()};
auto server_address = mp::client::get_server_address();
mp::ClientConfig config{mp::client::get_server_address(), mp::client::get_cert_provider(server_address),
term.get()};
mp::Client client{config};

return client.run(QCoreApplication::arguments());
Expand Down
84 changes: 35 additions & 49 deletions src/client/common/client_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ std::string message_box(const std::string& message)
return '\n' + divider + '\n' + message + '\n' + divider + '\n';
}

grpc::SslCredentialsOptions get_ssl_credentials_opts_from(const QString& cert_dir_path)
grpc::SslCredentialsOptions get_ssl_credentials_opts_from(const mp::CertProvider& cert_provider)
{
mp::SSLCertProvider cert_provider{cert_dir_path};
auto opts = grpc::SslCredentialsOptions();

opts.server_certificate_request = GRPC_SSL_REQUEST_SERVER_CERTIFICATE_BUT_DONT_VERIFY;
Expand Down Expand Up @@ -191,51 +190,9 @@ void mp::client::register_global_settings_handlers()
}

std::shared_ptr<grpc::Channel> mp::client::make_channel(const std::string& server_address,
mp::CertProvider* cert_provider)
const mp::CertProvider& cert_provider)
Comment on lines -194 to +193
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thnk this interface update is a drive-by change. But nevermind, it's a good change so I'm perfectly OK with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we could've kept the pointer but since after this refactoring we never pass a null pointer to this function, we can use a reference instead for more safety

{
// No common client certificates exist yet.
// TODO: Remove the following logic when we are comfortable all installed clients are using the common cert
if (!cert_provider)
{
auto data_location{MP_STDPATHS.writableLocation(StandardPaths::GenericDataLocation)};
auto common_client_cert_dir_path{data_location + common_client_cert_dir};

// The following logic is for determing which certificate to use when the client starts up.
// 1. Check if the multipass-gui certificate exists and determine if it's authenticated
// with the daemon already. If it is, copy it to the common client certificate directory and use it.
// 2. If that fails, then try the certificate from the cli client in the same manner.
// 3. Delete any per-client certificate dirs.
// 4. Lastly, no known certificate for the user exists, so create a new common certificate and use that.

const std::vector<QString> cert_dirs{data_location + gui_client_cert_dir, data_location + cli_client_cert_dir};
for (const auto& cert_dir : cert_dirs)
{
if (client_certs_exist(cert_dir))
{
if (auto rpc_channel{
create_channel_and_validate(server_address, get_ssl_credentials_opts_from(cert_dir))})
{
copy_client_certs_to_common_dir(cert_dir, common_client_cert_dir_path);
mp::utils::remove_directories(cert_dirs);

return rpc_channel;
}
}
}

mp::utils::remove_directories(cert_dirs);
MP_UTILS.make_dir(common_client_cert_dir_path);

return grpc::CreateChannel(server_address,
grpc::SslCredentials(get_ssl_credentials_opts_from(common_client_cert_dir_path)));
}

auto opts = grpc::SslCredentialsOptions();
opts.server_certificate_request = GRPC_SSL_REQUEST_SERVER_CERTIFICATE_BUT_DONT_VERIFY;
opts.pem_cert_chain = cert_provider->PEM_certificate();
opts.pem_private_key = cert_provider->PEM_signing_key();

return grpc::CreateChannel(server_address, grpc::SslCredentials(opts));
return grpc::CreateChannel(server_address, grpc::SslCredentials(get_ssl_credentials_opts_from(cert_provider)));
}

std::string mp::client::get_server_address()
Expand All @@ -250,17 +207,46 @@ std::string mp::client::get_server_address()
return mp::platform::default_server_address();
}

std::unique_ptr<mp::SSLCertProvider> mp::client::get_cert_provider()
std::unique_ptr<mp::SSLCertProvider> mp::client::get_cert_provider(const std::string& server_address)
{
auto data_location{MP_STDPATHS.writableLocation(StandardPaths::GenericDataLocation)};
auto common_client_cert_dir_path{data_location + common_client_cert_dir};

if (client_certs_exist(common_client_cert_dir_path))
{
return std::make_unique<mp::SSLCertProvider>(common_client_cert_dir_path);
return std::make_unique<SSLCertProvider>(common_client_cert_dir_path);
}

// No common client certificates exist yet.
// TODO: Remove the following logic when we are comfortable all installed clients are using the common cert

// The following logic is for determining which certificate to use when the client starts up.
// 1. Check if the multipass-gui certificate exists and determine if it's authenticated
// with the daemon already. If it is, copy it to the common client certificate directory and use it.
// 2. If that fails, then try the certificate from the cli client in the same manner.
// 3. Delete any per-client certificate dirs.
// 4. Lastly, no known certificate for the user exists, so create a new common certificate and use that.

const std::vector<QString> cert_dirs{data_location + gui_client_cert_dir, data_location + cli_client_cert_dir};
for (const auto& cert_dir : cert_dirs)
{
if (client_certs_exist(cert_dir))
{
auto ssl_cert_provider = std::make_unique<SSLCertProvider>(cert_dir);
if (auto rpc_channel{
create_channel_and_validate(server_address, get_ssl_credentials_opts_from(*ssl_cert_provider))})
{
copy_client_certs_to_common_dir(cert_dir, common_client_cert_dir_path);
utils::remove_directories(cert_dirs);

return ssl_cert_provider;
}
}
}

return nullptr;
utils::remove_directories(cert_dirs);
MP_UTILS.make_dir(common_client_cert_dir_path);
return std::make_unique<SSLCertProvider>(common_client_cert_dir_path);
}

void mp::client::set_logger()
Expand Down
9 changes: 9 additions & 0 deletions src/client/desktop_gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,12 @@ if(MULTIPASS_ENABLE_FLUTTER_GUI)

include("CMakeLists.txt.linux")
endif()

add_library(dart_ffi SHARED ffi/dart_ffi.cpp)

target_link_libraries(dart_ffi
petname
client_common
)

install(TARGETS dart_ffi LIBRARY COMPONENT multipass)
76 changes: 76 additions & 0 deletions src/client/desktop_gui/ffi/dart_ffi.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#include "multipass/dart_ffi.h"
#include "multipass/cli/client_common.h"
#include "multipass/format.h"
#include "multipass/logging/log.h"
#include "multipass/name_generator.h"
#include "multipass/version.h"

namespace mp = multipass;
namespace mpc = multipass::client;
namespace mpl = multipass::logging;

constexpr auto category = "dart-ffi";

extern "C" const char* multipass_version()
{
return mp::version_string;
}

extern "C" const char* generate_petname()
try
{
static mp::NameGenerator::UPtr generator = mp::make_default_name_generator();
const auto name = generator->make_name();
return strdup(name.c_str());
}
catch (const std::exception& e)
{
mpl::log(mpl::Level::warning, category, fmt::format("failed generating petname: {}", e.what()));
return nullptr;
}
catch (...)
{
mpl::log(mpl::Level::warning, category, "failed generating petname");
return nullptr;
}

extern "C" const char* get_server_address()
try
{
const auto address = mpc::get_server_address();
return strdup(address.c_str());
}
catch (const std::exception& e)
{
mpl::log(mpl::Level::warning, category, fmt::format("failed retrieving server address: {}", e.what()));
return nullptr;
}
catch (...)
{
mpl::log(mpl::Level::warning, category, "failed retrieving server address");
return nullptr;
}

extern "C" struct KeyCertificatePair get_cert_pair()
try
{
const auto provider = mpc::get_cert_provider(mpc::get_server_address());
const auto cert = provider->PEM_certificate();
const auto key = provider->PEM_signing_key();
struct KeyCertificatePair pair
{
};
pair.pem_cert = strdup(cert.c_str());
pair.pem_priv_key = strdup(key.c_str());
return pair;
}
catch (const std::exception& e)
{
mpl::log(mpl::Level::warning, category, fmt::format("failed retrieving certificate key pair: {}", e.what()));
return KeyCertificatePair{nullptr, nullptr};
}
catch (...)
{
mpl::log(mpl::Level::warning, category, "failed retrieving certificate key pair");
return KeyCertificatePair{nullptr, nullptr};
}
2 changes: 1 addition & 1 deletion src/client/gui/client_gui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace mp = multipass;
namespace mpl = multipass::logging;

mp::ClientGui::ClientGui(ClientConfig& config)
: stub{mp::Rpc::NewStub(mp::client::make_channel(config.server_address, config.cert_provider.get()))},
: stub{mp::Rpc::NewStub(mp::client::make_channel(config.server_address, *config.cert_provider))},
gui_cmd{std::make_unique<cmd::GuiCmd>(*stub, null_stream, null_stream)}
{
}
Expand Down
3 changes: 2 additions & 1 deletion src/client/gui/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ int main_impl(int argc, char* argv[])

mp::client::register_global_settings_handlers();

mp::ClientConfig config{mp::client::get_server_address(), mp::client::get_cert_provider()};
auto server_address = mp::client::get_server_address();
mp::ClientConfig config{server_address, mp::client::get_cert_provider(server_address)};
mp::ClientGui client{config};

return client.run(app.arguments());
Expand Down
8 changes: 4 additions & 4 deletions tests/mock_cert_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ struct MockCertProvider : public CertProvider
{
MockCertProvider()
{
EXPECT_CALL(*this, PEM_certificate()).WillRepeatedly(Return(client_cert));
EXPECT_CALL(*this, PEM_signing_key()).WillRepeatedly(Return(client_key));
ON_CALL(*this, PEM_certificate).WillByDefault(Return(client_cert));
ON_CALL(*this, PEM_signing_key).WillByDefault(Return(client_key));
}

MOCK_METHOD(std::string, PEM_certificate, (), (const, override));
MOCK_METHOD(std::string, PEM_signing_key, (), (const, override));
MOCK_METHOD(std::string, PEM_certificate, (), (override, const));
MOCK_METHOD(std::string, PEM_signing_key, (), (override, const));
};
} // namespace multipass::test
#endif // MULTIPASS_MOCK_CERT_PROVIDER_H
10 changes: 5 additions & 5 deletions tests/test_cli_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,6 @@ struct Client : public Test
.Times(AnyNumber())
.WillRepeatedly(Return("")); /* Avoid writing to Windows Terminal settings. We use an "expectation" so that
it gets reset at the end of each test (by VerifyAndClearExpectations) */

EXPECT_CALL(*client_cert_provider, PEM_certificate()).WillOnce(Return(mpt::client_cert));
EXPECT_CALL(*client_cert_provider, PEM_signing_key()).WillOnce(Return(mpt::client_key));
}

void TearDown() override
Expand All @@ -171,7 +168,7 @@ struct Client : public Test

int setup_client_and_run(const std::vector<std::string>& command, mp::Terminal& term)
{
mp::ClientConfig client_config{server_address, std::move(client_cert_provider), &term};
mp::ClientConfig client_config{server_address, get_client_cert_provider(), &term};
mp::Client client{client_config};
QStringList args = QStringList() << "multipass_test";

Expand Down Expand Up @@ -359,7 +356,10 @@ struct Client : public Test
#else
std::string server_address{"unix:/tmp/test-multipassd.socket"};
#endif
std::unique_ptr<mpt::MockCertProvider> client_cert_provider{std::make_unique<mpt::MockCertProvider>()};
static std::unique_ptr<mpt::MockCertProvider> get_client_cert_provider()
{
return std::make_unique<mpt::MockCertProvider>();
};
std::unique_ptr<mpt::MockCertProvider> daemon_cert_provider{std::make_unique<mpt::MockCertProvider>()};
mpt::MockPlatform::GuardedMock attr{mpt::MockPlatform::inject<NiceMock>()};
mpt::MockPlatform* mock_platform = attr.first;
Expand Down
8 changes: 4 additions & 4 deletions tests/test_client_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ TEST_F(TestClientCommon, usesCommonCertWhenItExists)
mpt::make_file_with_content(common_client_cert_file, mpt::client_cert);
mpt::make_file_with_content(common_client_key_file, mpt::client_key);

EXPECT_TRUE(mp::client::make_channel(server_address, mp::client::get_cert_provider().get()));
EXPECT_TRUE(mp::client::make_channel(server_address, *mp::client::get_cert_provider(server_address)));
}

TEST_F(TestClientCommon, usesExistingGuiCert)
Expand All @@ -88,7 +88,7 @@ TEST_F(TestClientCommon, usesExistingGuiCert)

mpt::MockDaemon daemon{make_secure_server()};

EXPECT_TRUE(mp::client::make_channel(server_address, mp::client::get_cert_provider().get()));
EXPECT_TRUE(mp::client::make_channel(server_address, *mp::client::get_cert_provider(server_address)));
EXPECT_FALSE(QFile::exists(gui_cert_dir));
}

Expand All @@ -113,7 +113,7 @@ TEST_F(TestClientCommon, failsGuiCertUsesExistingCliCert)

mpt::MockDaemon daemon{make_secure_server()};

EXPECT_TRUE(mp::client::make_channel(server_address, mp::client::get_cert_provider().get()));
EXPECT_TRUE(mp::client::make_channel(server_address, *mp::client::get_cert_provider(server_address)));
EXPECT_FALSE(QFile::exists(gui_cert_dir));
EXPECT_FALSE(QFile::exists(cli_cert_dir));
}
Expand All @@ -139,7 +139,7 @@ TEST_F(TestClientCommon, noValidCertsCreatesNewCommonCert)

mpt::MockDaemon daemon{make_secure_server()};

EXPECT_TRUE(mp::client::make_channel(server_address, mp::client::get_cert_provider().get()));
EXPECT_TRUE(mp::client::make_channel(server_address, *mp::client::get_cert_provider(server_address)));
EXPECT_TRUE(QFile::exists(common_cert_dir + "/" + mp::client_cert_file));
EXPECT_TRUE(QFile::exists(common_cert_dir + "/" + mp::client_key_file));
EXPECT_FALSE(QFile::exists(gui_cert_dir));
Expand Down