From c2f7ead468d5e65c57e7ecb49d7fbba4254c46b7 Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Tue, 2 Jul 2024 16:47:13 +0200 Subject: Replace the Auth and Auth::TLS singletons Use a builder pattern for creation and validation, in a manner that allows also other authentication methods to be added in the future besides the current TLS/SSL. The main Auth instances are built early and then passed by not_null const pointers, to avoid passing temporaries, replacing the previous Auth::TLS instances passed by simple nullable const pointers. Where needed, these passed Auth instances are also stored, by const ref. Tests also build Auth instances as needed, either with the default 'no certification' or from the test environment arguments. --- src/buildtool/serve_api/remote/TARGETS | 6 ++++-- src/buildtool/serve_api/remote/configuration_client.hpp | 6 ++++-- src/buildtool/serve_api/remote/serve_api.hpp | 4 ++-- src/buildtool/serve_api/remote/source_tree_client.cpp | 5 +++-- src/buildtool/serve_api/remote/source_tree_client.hpp | 3 ++- src/buildtool/serve_api/remote/target_client.cpp | 2 +- .../serve_api/serve_service/serve_server_implementation.cpp | 9 ++++++--- src/buildtool/serve_api/serve_service/target.cpp | 2 +- 8 files changed, 23 insertions(+), 14 deletions(-) (limited to 'src/buildtool/serve_api') diff --git a/src/buildtool/serve_api/remote/TARGETS b/src/buildtool/serve_api/remote/TARGETS index e6f03424..4af6387f 100644 --- a/src/buildtool/serve_api/remote/TARGETS +++ b/src/buildtool/serve_api/remote/TARGETS @@ -16,7 +16,8 @@ , "hdrs": ["source_tree_client.hpp"] , "srcs": ["source_tree_client.cpp"] , "deps": - [ ["src/buildtool/auth", "auth"] + [ ["@", "gsl", "", "gsl"] + , ["src/buildtool/auth", "auth"] , ["src/buildtool/common/remote", "port"] , ["src/buildtool/file_system", "git_types"] , ["src/buildtool/file_system/symlinks_map", "pragma_special"] @@ -80,7 +81,8 @@ , "hdrs": ["configuration_client.hpp"] , "srcs": ["configuration_client.cpp"] , "deps": - [ ["src/buildtool/auth", "auth"] + [ ["@", "gsl", "", "gsl"] + , ["src/buildtool/auth", "auth"] , ["src/buildtool/common/remote", "port"] , ["src/buildtool/logging", "logging"] , ["src/buildtool/common/remote", "client_common"] diff --git a/src/buildtool/serve_api/remote/configuration_client.hpp b/src/buildtool/serve_api/remote/configuration_client.hpp index b7785315..3d0eb7ff 100644 --- a/src/buildtool/serve_api/remote/configuration_client.hpp +++ b/src/buildtool/serve_api/remote/configuration_client.hpp @@ -21,6 +21,7 @@ #include #include +#include "gsl/gsl" #include "justbuild/just_serve/just_serve.grpc.pb.h" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/remote/client_common.hpp" @@ -32,8 +33,9 @@ /// src/buildtool/serve_api/serve_service/just_serve.proto class ConfigurationClient { public: - explicit ConfigurationClient(ServerAddress address, - Auth::TLS const* auth) noexcept + explicit ConfigurationClient( + ServerAddress address, + gsl::not_null const& auth) noexcept : client_serve_address_{std::move(address)}, stub_{justbuild::just_serve::Configuration::NewStub( CreateChannelWithCredentials(client_serve_address_.host, diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp index 9e4a29bf..3374e6aa 100644 --- a/src/buildtool/serve_api/remote/serve_api.hpp +++ b/src/buildtool/serve_api/remote/serve_api.hpp @@ -42,9 +42,9 @@ class ServeApi final { public: explicit ServeApi(ServerAddress const& address, gsl::not_null const& apis) noexcept - : stc_{address, apis->auth}, + : stc_{address, &apis->auth}, tc_{address, apis}, - cc_{address, apis->auth} {} + cc_{address, &apis->auth} {} ~ServeApi() noexcept = default; ServeApi(ServeApi const&) = delete; diff --git a/src/buildtool/serve_api/remote/source_tree_client.cpp b/src/buildtool/serve_api/remote/source_tree_client.cpp index 3070a44f..7a922671 100644 --- a/src/buildtool/serve_api/remote/source_tree_client.cpp +++ b/src/buildtool/serve_api/remote/source_tree_client.cpp @@ -59,8 +59,9 @@ auto PragmaSpecialToSymlinksResolve( } // namespace -SourceTreeClient::SourceTreeClient(ServerAddress const& address, - Auth::TLS const* auth) noexcept { +SourceTreeClient::SourceTreeClient( + ServerAddress const& address, + gsl::not_null const& auth) noexcept { stub_ = justbuild::just_serve::SourceTree::NewStub( CreateChannelWithCredentials(address.host, address.port, auth)); } diff --git a/src/buildtool/serve_api/remote/source_tree_client.hpp b/src/buildtool/serve_api/remote/source_tree_client.hpp index 17eb0c57..e5001029 100644 --- a/src/buildtool/serve_api/remote/source_tree_client.hpp +++ b/src/buildtool/serve_api/remote/source_tree_client.hpp @@ -19,6 +19,7 @@ #include #include +#include "gsl/gsl" #include "justbuild/just_serve/just_serve.grpc.pb.h" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/remote/port.hpp" @@ -33,7 +34,7 @@ class SourceTreeClient { public: explicit SourceTreeClient(ServerAddress const& address, - Auth::TLS const* auth) noexcept; + gsl::not_null const& auth) noexcept; // An error + data union type using result_t = expected; diff --git a/src/buildtool/serve_api/remote/target_client.cpp b/src/buildtool/serve_api/remote/target_client.cpp index d19ac26f..526121ab 100644 --- a/src/buildtool/serve_api/remote/target_client.cpp +++ b/src/buildtool/serve_api/remote/target_client.cpp @@ -31,7 +31,7 @@ TargetClient::TargetClient(ServerAddress const& address, gsl::not_null const& apis) noexcept : apis_{*apis} { stub_ = justbuild::just_serve::Target::NewStub( - CreateChannelWithCredentials(address.host, address.port, apis->auth)); + CreateChannelWithCredentials(address.host, address.port, &apis->auth)); } auto TargetClient::ServeTarget(const TargetCacheKey& key, diff --git a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp index 1d81ebb0..906f331c 100644 --- a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp +++ b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp @@ -18,6 +18,7 @@ #include #include +#include #ifdef __unix__ #include @@ -132,13 +133,15 @@ auto ServeServerImpl::Run(RemoteServeConfig const& serve_config, .RegisterService(&op); } + // check authentication credentials; currently only TLS/SSL is supported std::shared_ptr creds; - if (apis.auth != nullptr) { + if (const auto* tls_auth = std::get_if(&apis.auth.method); + tls_auth != nullptr) { auto tls_opts = grpc::SslServerCredentialsOptions{}; - tls_opts.pem_root_certs = apis.auth->CACert(); + tls_opts.pem_root_certs = tls_auth->ca_cert; grpc::SslServerCredentialsOptions::PemKeyCertPair keycert = { - apis.auth->ServerKey(), apis.auth->ServerCert()}; + tls_auth->server_key, tls_auth->server_cert}; tls_opts.pem_key_cert_pairs.emplace_back(keycert); diff --git a/src/buildtool/serve_api/serve_service/target.cpp b/src/buildtool/serve_api/serve_service/target.cpp index fc09ee8c..44632590 100644 --- a/src/buildtool/serve_api/serve_service/target.cpp +++ b/src/buildtool/serve_api/serve_service/target.cpp @@ -499,7 +499,7 @@ auto TargetService::ServeTarget( // Use a new ApiBundle that knows about local repository config for // traversing. - ApiBundle const local_apis{&repository_config, apis_.auth, address}; + ApiBundle const local_apis{&repository_config, &apis_.auth, address}; GraphTraverser const traverser{ std::move(traverser_args), &repository_config, -- cgit v1.2.3