From fc947860a12b8ed219799b87b68418e6c6678476 Mon Sep 17 00:00:00 2001 From: Maksim Denisov Date: Mon, 2 Sep 2024 16:22:57 +0200 Subject: Validate hashes in serve's target --- src/buildtool/serve_api/serve_service/target.cpp | 62 +++++++++++++----------- 1 file changed, 34 insertions(+), 28 deletions(-) (limited to 'src/buildtool/serve_api/serve_service/target.cpp') diff --git a/src/buildtool/serve_api/serve_service/target.cpp b/src/buildtool/serve_api/serve_service/target.cpp index d2e66ae0..5bd94ba2 100644 --- a/src/buildtool/serve_api/serve_service/target.cpp +++ b/src/buildtool/serve_api/serve_service/target.cpp @@ -25,6 +25,8 @@ #include "src/buildtool/build_engine/target_map/configured_target.hpp" #include "src/buildtool/build_engine/target_map/result_map.hpp" #include "src/buildtool/common/artifact.hpp" +#include "src/buildtool/common/artifact_digest.hpp" +#include "src/buildtool/common/artifact_digest_factory.hpp" #include "src/buildtool/common/remote/retry_config.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" @@ -45,7 +47,6 @@ #include "src/buildtool/storage/garbage_collector.hpp" #include "src/buildtool/storage/repository_garbage_collector.hpp" #include "src/buildtool/storage/target_cache_key.hpp" -#include "src/utils/cpp/verify_hash.hpp" auto TargetService::GetDispatchList( ArtifactDigest const& dispatch_digest) noexcept @@ -114,7 +115,7 @@ auto TargetService::HandleFailureLog( return ::grpc::Status{::grpc::StatusCode::UNAVAILABLE, msg}; } // set response with log digest - (*response->mutable_log()) = static_cast(*digest); + (*response->mutable_log()) = ArtifactDigestFactory::ToBazel(*digest); return ::grpc::Status::OK; } @@ -127,13 +128,16 @@ auto TargetService::CreateRemoteExecutionConfig( platform_properties[p.name()] = p.value(); } // read in the dispatch list - if (auto msg = IsAHash(request->dispatch_info().hash()); msg) { - logger_->Emit(LogLevel::Error, "{}", *msg); - return unexpected{ - ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, *msg}}; + auto const dispatch_info_digest = ArtifactDigestFactory::FromBazel( + local_context_.storage_config->hash_function.GetType(), + request->dispatch_info()); + if (not dispatch_info_digest) { + logger_->Emit(LogLevel::Error, "{}", dispatch_info_digest.error()); + return unexpected{::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, + dispatch_info_digest.error()}}; } - auto const& dispatch_info_digest = ArtifactDigest{request->dispatch_info()}; - auto res = GetDispatchList(dispatch_info_digest); + + auto res = GetDispatchList(*dispatch_info_digest); if (not res) { auto err = move(res).error(); logger_->Emit(LogLevel::Error, "{}", err.error_message()); @@ -152,12 +156,14 @@ auto TargetService::ServeTarget( const ::justbuild::just_serve::ServeTargetRequest* request, ::justbuild::just_serve::ServeTargetResponse* response) -> ::grpc::Status { // check target cache key hash for validity - if (auto msg = IsAHash(request->target_cache_key_id().hash()); msg) { - logger_->Emit(LogLevel::Error, "{}", *msg); - return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, *msg}; + auto const target_cache_key_digest = ArtifactDigestFactory::FromBazel( + local_context_.storage_config->hash_function.GetType(), + request->target_cache_key_id()); + if (not target_cache_key_digest) { + logger_->Emit(LogLevel::Error, "{}", target_cache_key_digest.error()); + return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, + target_cache_key_digest.error()}; } - auto const& target_cache_key_digest = - ArtifactDigest{request->target_cache_key_id()}; // acquire locks auto repo_lock = @@ -207,7 +213,7 @@ auto TargetService::ServeTarget( : std::nullopt; auto const& tc = local_context_.storage->TargetCache().WithShard(shard); auto const& tc_key = - TargetCacheKey{{target_cache_key_digest, ObjectType::File}}; + TargetCacheKey{{*target_cache_key_digest, ObjectType::File}}; // check if target-level cache entry has already been computed if (auto target_entry = tc.Read(tc_key); target_entry) { @@ -233,15 +239,15 @@ auto TargetService::ServeTarget( } // populate response with the target cache value (*response->mutable_target_value()) = - static_cast(target_entry->second.digest); + ArtifactDigestFactory::ToBazel(target_entry->second.digest); return ::grpc::Status::OK; } // get target description from remote cas auto const& target_cache_key_info = Artifact::ObjectInfo{ - .digest = target_cache_key_digest, .type = ObjectType::File}; + .digest = *target_cache_key_digest, .type = ObjectType::File}; - if (not apis_.local->IsAvailable(target_cache_key_digest) and + if (not apis_.local->IsAvailable(*target_cache_key_digest) and not apis_.remote->RetrieveToCas({target_cache_key_info}, *apis_.local)) { auto msg = fmt::format( @@ -268,7 +274,7 @@ auto TargetService::ServeTarget( nlohmann::json::parse(*target_description_str)); } catch (std::exception const& ex) { auto msg = fmt::format("Parsing TargetCacheKey {} failed with:\n{}", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), ex.what()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::INTERNAL, msg}; @@ -277,7 +283,7 @@ auto TargetService::ServeTarget( not target_description_dict->IsMap()) { auto msg = fmt::format("TargetCacheKey {} should contain a map, but found {}", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), target_description_dict.ToJson().dump()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::NOT_FOUND, msg}; @@ -293,7 +299,7 @@ auto TargetService::ServeTarget( if (not target_description_dict->At(key)) { error_msg = fmt::format("TargetCacheKey {} does not contain key \"{}\"", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), key); logger_->Emit(LogLevel::Error, "{}", error_msg); return false; @@ -313,7 +319,7 @@ auto TargetService::ServeTarget( auto msg = fmt::format( "TargetCacheKey {}: \"repo_key\" value should be a string, but " "found {}", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), repo_key.ToJson().dump()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::NOT_FOUND, msg}; @@ -361,7 +367,7 @@ auto TargetService::ServeTarget( auto msg = fmt::format( "TargetCacheKey {}: \"target_name\" value should be a string, but" " found {}", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), target_expr.ToJson().dump()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::FAILED_PRECONDITION, msg}; @@ -372,7 +378,7 @@ auto TargetService::ServeTarget( } catch (std::exception const& ex) { auto msg = fmt::format( "TargetCacheKey {}: parsing \"target_name\" failed with:\n{}", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), ex.what()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::FAILED_PRECONDITION, msg}; @@ -385,7 +391,7 @@ auto TargetService::ServeTarget( auto msg = fmt::format( "TargetCacheKey {}: \"effective_config\" value should be a string," " but found {}", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), config_expr.ToJson().dump()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::FAILED_PRECONDITION, msg}; @@ -397,7 +403,7 @@ auto TargetService::ServeTarget( } catch (std::exception const& ex) { auto msg = fmt::format( "TargetCacheKey {}: parsing \"effective_config\" failed with:\n{}", - target_cache_key_digest.hash(), + target_cache_key_digest->hash(), ex.what()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::FAILED_PRECONDITION, msg}; @@ -574,13 +580,13 @@ auto TargetService::ServeTarget( } // populate response with the target cache value (*response->mutable_target_value()) = - static_cast(target_entry->second.digest); + ArtifactDigestFactory::ToBazel(target_entry->second.digest); return ::grpc::Status::OK; } // target cache value missing -- internally something is very wrong auto msg = fmt::format("Failed to read TargetCacheKey {} after store", - target_cache_key_digest.hash()); + target_cache_key_digest->hash()); logger_->Emit(LogLevel::Error, "{}", msg); return ::grpc::Status{::grpc::StatusCode::INTERNAL, msg}; } @@ -931,7 +937,7 @@ auto TargetService::ServeTargetDescription( // populate response (*response->mutable_description_id()) = - static_cast(*dgst); + ArtifactDigestFactory::ToBazel(*dgst); return ::grpc::Status::OK; } // failed to store blob -- cgit v1.2.3