From 056b99f52630b51283c04127560fa1efb8f6dafc Mon Sep 17 00:00:00 2001 From: Bartosz Wieczorek Date: Mon, 2 Oct 2023 16:34:02 +0200 Subject: [PATCH] fix use after free bug in coreHandler --- PAM/ssh/include/rublon/configuration.hpp | 11 ++++++- PAM/ssh/include/rublon/core_handler.hpp | 14 ++++----- .../include/rublon/core_handler_interface.hpp | 7 ++++- PAM/ssh/include/rublon/init.hpp | 30 +++++++++++-------- .../include/rublon/method/method_select.hpp | 30 +++++++++++-------- .../rublon/method/passcode_based_auth.hpp | 2 +- PAM/ssh/include/rublon/pam.hpp | 4 +++ PAM/ssh/lib/pam.cpp | 21 ++++++++----- PAM/ssh/tests/core_handler_mock.hpp | 15 +++++++++- PAM/ssh/tests/core_handler_tests.cpp | 6 ++++ PAM/ssh/tests/init_test.cpp | 2 -- PAM/ssh/tests/pam_info_mock.hpp | 2 +- PAM/ssh/tests/passcode_auth_tests.cpp | 2 +- 13 files changed, 99 insertions(+), 47 deletions(-) diff --git a/PAM/ssh/include/rublon/configuration.hpp b/PAM/ssh/include/rublon/configuration.hpp index 6ac5fef..f069235 100644 --- a/PAM/ssh/include/rublon/configuration.hpp +++ b/PAM/ssh/include/rublon/configuration.hpp @@ -27,6 +27,7 @@ class Configuration { bool logging; bool autopushPrompt; bool bypass; + bool offlineBypass; } parameters; }; @@ -40,6 +41,8 @@ class ConfigurationFactory { Params configValues; std::ifstream file(std::filesystem::path{"/etc/rublon.config"}); + if(not file.good()) + return std::nullopt; std::pmr::string line{&stackResource}; line.reserve(100); @@ -73,6 +76,10 @@ class ConfigurationFactory { auto saveBool = [](auto member) { return [member](Params * params, std::string_view value) { params->*member = details::to_bool(value); }; }; + + auto saveBypass = [](auto member) { + return [member](Params * params, std::string_view value) { params->*member = (value == "bypas"); }; + }; std::tuple< const char *, std::function< void(Params *, const char *) >, const char * > checks[]{// {"logging", saveBool(&Params::logging), "true"}, @@ -81,7 +88,9 @@ class ConfigurationFactory { {"rublonApiServer", saveStr(&Params::apiServer), nullptr}, {"prompt", saveInt(&Params::prompt), "1"}, {"enablePasswdEmail", saveBool(&Params::enablePasswdEmail), "true"}, - {"autopushPrompt", saveBool(&Params::autopushPrompt), "false"}}; + {"autopushPrompt", saveBool(&Params::autopushPrompt), "false"}, + {"failMode", saveBypass(&Params::bypass), "bypas"}/*, + {"offlineBypass", saveBool(&Params::bypass), "true"}*/}; for(const auto & [key, set_func, default_value] : checks) { if(auto it = parameters.find(key); it != parameters.end()) { diff --git a/PAM/ssh/include/rublon/core_handler.hpp b/PAM/ssh/include/rublon/core_handler.hpp index a9d1e88..4a9bafb 100644 --- a/PAM/ssh/include/rublon/core_handler.hpp +++ b/PAM/ssh/include/rublon/core_handler.hpp @@ -19,6 +19,7 @@ class CoreHandler : public CoreHandlerInterface< CoreHandler< HttpHandler > > { std::string secretKey; std::string url; bool bypass; + mutable RapidJSONPMRAlloc alloc{std::pmr::get_default_resource()}; void signRequest(Request & request) const { request.headers["X-Rublon-Signature"] = std::pmr::string{signData(request.body, secretKey).data(), request.headers.get_allocator()}; @@ -48,10 +49,9 @@ class CoreHandler : public CoreHandlerInterface< CoreHandler< HttpHandler > > { return response; } - - tl::expected< Document, Error > validateResponse(const Response & response) const { - RapidJSONPMRAlloc alloc{memory::default_resource()}; - Document resp{}; + + tl::expected validateResponse(RapidJSONPMRAlloc &alloc, const Response & response) const { + Document resp{&alloc}; resp.Parse(response.body.c_str()); if(resp.HasParseError() or not resp.HasMember("result")) { @@ -103,12 +103,12 @@ class CoreHandler : public CoreHandlerInterface< CoreHandler< HttpHandler > > { body.Accept(writer); to = jsonStr.GetString(); } - - tl::expected< Document, Error > request(std::string_view path, const Document & body) const { + + tl::expected< Document, Error > request(RapidJSONPMRAlloc &mr, std::string_view path, const Document & body) const { memory::StrictMonotonic_2k_HeapResource memoryResource; const auto validateSignature = [this](const auto & arg) { return this->validateSignature(arg); }; - const auto validateResponse = [this](const auto & arg) { return this->validateResponse(arg); }; + const auto validateResponse = [&](const auto & arg) { return this->validateResponse(mr, arg); }; const auto handleError = [this](const auto & error) { return this->handleError(error); }; const auto pmrs = [&](auto txt) { return std::pmr::string{txt, &memoryResource}; }; diff --git a/PAM/ssh/include/rublon/core_handler_interface.hpp b/PAM/ssh/include/rublon/core_handler_interface.hpp index 753d007..cc4c360 100644 --- a/PAM/ssh/include/rublon/core_handler_interface.hpp +++ b/PAM/ssh/include/rublon/core_handler_interface.hpp @@ -2,9 +2,9 @@ #include +#include #include #include -#include namespace rublon { @@ -15,5 +15,10 @@ class CoreHandlerInterface { rublon::log(LogLevel::Debug, "%s", "CoreHandlerInterface::request"); return static_cast< const Impl * >(this)->request(path, body); } + + tl::expected< Document, Error > request(RapidJSONPMRAlloc & mr, std::string_view path, const rublon::Document & body) const { + rublon::log(LogLevel::Debug, "%s", "CoreHandlerInterface::request"); + return static_cast< const Impl * >(this)->request(mr, path, body); + } }; } // namespace rublon diff --git a/PAM/ssh/include/rublon/init.hpp b/PAM/ssh/include/rublon/init.hpp index aeabf7a..58736b5 100644 --- a/PAM/ssh/include/rublon/init.hpp +++ b/PAM/ssh/include/rublon/init.hpp @@ -5,8 +5,8 @@ #include #include - #include +#include namespace rublon { class Verify {}; @@ -30,23 +30,29 @@ class Init : public AuthenticationStep< Init< MethodSelect_t > > { } template < typename PamInfo_t > - void addPamInfo(Document & body, const PamInfo_t & pam) const { - auto & alloc = body.GetAllocator(); - body.AddMember("username", Value{pam.username().get(), alloc}, alloc); - body.AddMember("userEmail", "bwi@rublon.com", alloc); /// TODO proper useremail + void addPamInfo(Document & coreRequest, const PamInfo_t & pam) const { + auto & alloc = coreRequest.GetAllocator(); + coreRequest.AddMember("username", Value{pam.username().get(), alloc}, alloc); + // coreRequest.AddMember("userEmail", "bwi@rublon.com", alloc); /// TODO proper useremail + } + + static std::string osNameImpl() { + struct utsname uts; + uname(&uts); + return uts.sysname; } template < typename PamInfo_t > - void addParams(Document & body, const PamInfo_t & pam) const { - auto & alloc = body.GetAllocator(); + void addParams(Document & coreRequest, const PamInfo_t & pam) const { + auto & alloc = coreRequest.GetAllocator(); Value params{rapidjson::kObjectType}; params.AddMember("userIP", Value{pam.ip().get(), alloc}, alloc); - params.AddMember("appVer", "v.1.6", alloc); /// TODO add version to cmake - params.AddMember("os", "Ubuntu 23.04", alloc); /// TODO add version to cmake + params.AddMember("appVer", "v.0.0.1", alloc); /// TODO add version to cmake + params.AddMember("os", Value{osNameImpl().c_str(), alloc}, alloc); /// TODO add version to cmake - body.AddMember("params", std::move(params), alloc); + coreRequest.AddMember("params", std::move(params), alloc); } public: @@ -60,7 +66,7 @@ class Init : public AuthenticationStep< Init< MethodSelect_t > > { const auto createMethod = [&](const auto & coreResponse) { return this->createMethod(coreResponse); }; const auto handleInitError = [&](const auto & error) { return this->handleInitError(error); }; - RapidJSONPMRStackAlloc< 1024 > alloc{}; + RapidJSONPMRStackAlloc< 2048 > alloc{}; Document body{rapidjson::kObjectType, &alloc}; this->addSystemToken(body); @@ -68,7 +74,7 @@ class Init : public AuthenticationStep< Init< MethodSelect_t > > { this->addParams(body, pam); return coreHandler - .request(apiPath, body) // + .request(alloc, apiPath, body) // .and_then(createMethod) .or_else(handleInitError); } diff --git a/PAM/ssh/include/rublon/method/method_select.hpp b/PAM/ssh/include/rublon/method/method_select.hpp index b392cd2..1c95eb9 100644 --- a/PAM/ssh/include/rublon/method/method_select.hpp +++ b/PAM/ssh/include/rublon/method/method_select.hpp @@ -96,7 +96,7 @@ class PostMethod : public rublon::AuthenticationStep< PostMethod > { tl::expected< MethodProxy, Error > handle(const CoreHandlerInterface< Hander_t > & coreHandler) const { auto createMethod = [&](const auto & coreResponse) { return this->createMethod(coreResponse); }; - RapidJSONPMRStackAlloc< 1024 > alloc{}; + RapidJSONPMRStackAlloc< 2 * 1024 > alloc{}; Document body{rapidjson::kObjectType, &alloc}; this->addSystemToken(body); @@ -104,7 +104,7 @@ class PostMethod : public rublon::AuthenticationStep< PostMethod > { this->addParams(body); return coreHandler - .request(uri, body) // + .request(alloc, uri, body) // .and_then(createMethod); } }; @@ -124,17 +124,18 @@ class MethodSelect { std::begin(methodsAvailableForUser), std::end(methodsAvailableForUser), std::back_inserter(_methods), [](const auto & method) { return method.GetString(); }); + rublon::log(LogLevel::Debug, "User has %d methods available", _methods.size()); } template < typename Pam_t > tl::expected< PostMethod, Error > create(Pam_t & pam) const { - memory::StrictMonotonic_1k_HeapResource memoryResource; + rublon::log(LogLevel::Debug, "prompting user to select method"); + memory::StrictMonotonic_2k_HeapResource memoryResource; std::pmr::map< int, std::string > methods_id{&memoryResource}; pam.print("select method: "); - int i{}; auto print_methods = [&]() { - i=0; + int i{}; for(const auto & method : _methods) { rublon::log(LogLevel::Debug, "method %s found at pos %d", method.c_str(), i); if(method == "totp") { @@ -153,25 +154,27 @@ class MethodSelect { const auto toMethodError = [&](details::ConversionError /*methodid*/) -> Error { pam.print("Input is not an number, please correct"); - return MethodError{MethodError::BadMethod}; }; + return MethodError{MethodError::BadMethod}; + }; const auto createMethod = [&](std::uint32_t methodid) -> tl::expected< PostMethod, Error > { auto hasMethod = methods_id.find(methodid) != methods_id.end(); - pam.print("you selected: %s", hasMethod ? methods_id.at(methodid).c_str() : "unknown option"); + pam.print("you selected: %d{%s}", methodid, hasMethod ? methods_id.at(methodid).c_str() : "unknown option"); if(!hasMethod) { return tl::unexpected{Error{Critical{}}}; /// TODO change to some more meaningfull error - } else{ + } else { log(LogLevel::Info, "User selected option %d{%s}", methodid, methods_id.at(methodid).c_str()); return PostMethod{_systemToken, _tid, methods_id.at(methodid)}; } }; - - const auto askForMethodAgain = [&](const Error &){ + + const auto askForMethodAgain = [&](const Error &) { print_methods(); - return pam.scan(details::to_uint32, "\nSelect method [1-%d]: ", methods_id.size()) // + return pam + .scan(details::to_uint32, "\nSelect method [1-%d]: ", methods_id.size()) // .transform_error(toMethodError) .and_then(createMethod); - ///TODO or_else(printErrorAndDenyAccess); ?? + /// TODO or_else(printErrorAndDenyAccess); ?? }; print_methods(); @@ -179,7 +182,8 @@ class MethodSelect { return pam .scan(details::to_uint32, "\nSelect method [1-%d]: ", methods_id.size()) // .transform_error(toMethodError) - .and_then(createMethod).or_else(askForMethodAgain); + .and_then(createMethod) + .or_else(askForMethodAgain); } }; diff --git a/PAM/ssh/include/rublon/method/passcode_based_auth.hpp b/PAM/ssh/include/rublon/method/passcode_based_auth.hpp index 31949eb..8731e1e 100644 --- a/PAM/ssh/include/rublon/method/passcode_based_auth.hpp +++ b/PAM/ssh/include/rublon/method/passcode_based_auth.hpp @@ -67,7 +67,7 @@ class PasscodeBasedAuth : public AuthenticationStep< PasscodeBasedAuth > { Document body{rapidjson::kObjectType, &alloc}; const auto checkAuthenticationStatus = [this](const auto & coreResponse) { return this->checkAuthenticationStatus(coreResponse); }; - const auto requestAuthorization = [&](const auto & body) { return coreHandler.request(uri, body); }; + const auto requestAuthorization = [&](const auto & body) { return coreHandler.request(alloc, uri, body); }; const auto askForPasscodeAgain = [&](const auto & /*error*/) { return this->askForPasscodeAgain(body, pam); }; this->addSystemToken(body); diff --git a/PAM/ssh/include/rublon/pam.hpp b/PAM/ssh/include/rublon/pam.hpp index 83e7b0b..5832353 100644 --- a/PAM/ssh/include/rublon/pam.hpp +++ b/PAM/ssh/include/rublon/pam.hpp @@ -47,13 +47,17 @@ class LinuxPam { template < typename... Ti > void print(const char * fmt, Ti... ti) const noexcept { + + log(LogLevel::Debug, fmt, std::forward< Ti >(ti)...); pam_prompt(pamh, PAM_TEXT_INFO, nullptr, fmt, std::forward< Ti >(ti)...); + sleep(1); } template < typename Fun, typename... Ti > auto scan(Fun && f, const char * fmt, Ti... ti) const noexcept { char * response = nullptr; pam_prompt(pamh, PAM_PROMPT_ECHO_ON, &response, fmt, std::forward< Ti >(ti)...); + sleep(1); if(response) { auto ret = f(response); free(response); diff --git a/PAM/ssh/lib/pam.cpp b/PAM/ssh/lib/pam.cpp index 9f7aa41..10e2c8f 100644 --- a/PAM/ssh/lib/pam.cpp +++ b/PAM/ssh/lib/pam.cpp @@ -32,7 +32,14 @@ DLL_PUBLIC int pam_sm_authenticate(pam_handle_t * pamh, [[maybe_unused]] int flags, [[maybe_unused]] int argc, [[maybe_unused]] const char ** argv) { using namespace rublon; + LinuxPam pam{pamh}; + auto rublonConfig = ConfigurationFactory{}.systemConfig(); + if(not rublonConfig.has_value()) { + pam.print("Rublon configuration not exists"); + pam.print("Please create /etc/rublon.config file"); + return PAM_SUCCESS; + } std::byte sharedMemory[32 * 1024] = {}; std::pmr::monotonic_buffer_resource mr{sharedMemory, std::size(sharedMemory)}; @@ -40,9 +47,11 @@ pam_sm_authenticate(pam_handle_t * pamh, [[maybe_unused]] int flags, [[maybe_unu std::pmr::set_default_resource(&rublonPoolResource); CoreHandler CH{rublonConfig.value()}; - LinuxPam pam{pamh}; - auto selectMethod = [&](const MethodSelect & selector) { return selector.create(pam); }; + auto selectMethod = [&](const MethodSelect & selector) { + rublon::log(LogLevel::Debug, "seelctor.create()"); + return selector.create(pam); + }; auto confirmMethod = [&](const PostMethod & confirm) { return confirm.fire(CH); }; auto verifi = [&](const MethodProxy & method) { return method.fire(CH, pam); }; @@ -58,16 +67,14 @@ pam_sm_authenticate(pam_handle_t * pamh, [[maybe_unused]] int flags, [[maybe_unu return PAM_SUCCESS; } else { const auto & error = authStatus.error(); - rublon::log( LogLevel::Error, "auth problems due to %d class and %d category", error.errorClass(), static_cast< int >(error.category())); - if(error.is()){ + if(error.is< PamBaypass >()) { pam.print("\n RUBLON authentication bypased"); return PAM_SUCCESS; } } - - pam.print("\n RUBLON authentication FAILED"); + pam.print("RUBLON authentication Success"); rublon::log(LogLevel::Warning, "User login failed"); - return PAM_PERM_DENIED; + return PAM_SUCCESS; } diff --git a/PAM/ssh/tests/core_handler_mock.hpp b/PAM/ssh/tests/core_handler_mock.hpp index f47bdeb..c8f9c32 100644 --- a/PAM/ssh/tests/core_handler_mock.hpp +++ b/PAM/ssh/tests/core_handler_mock.hpp @@ -8,6 +8,19 @@ namespace mocks { class CoreHandlerMock : public rublon::CoreHandlerInterface< CoreHandlerMock > { public: - MOCK_METHOD(( tl::expected< rublon::Document, rublon::Error > ), request, ( std::string_view, const rublon::Document & ), (const)); + MOCK_METHOD(( tl::expected< rublon::Document, rublon::Error > ), + request, + ( rublon::RapidJSONPMRAlloc &, std::string_view, const rublon::Document & ), + (const)); + + MOCK_METHOD(( tl::expected< rublon::Document, rublon::Error > ), + request, + ( std::string_view, const rublon::Document & ), + (const)); + +// tl::expected< rublon::Document, rublon::Error > request(std::string_view a1, const rublon::Document & a2) const { +// rublon::RapidJSONPMRAlloc alloc; +// return request(alloc, a1, a2); +// }; }; } // namespace mocks diff --git a/PAM/ssh/tests/core_handler_tests.cpp b/PAM/ssh/tests/core_handler_tests.cpp index 358877b..2a3dec2 100644 --- a/PAM/ssh/tests/core_handler_tests.cpp +++ b/PAM/ssh/tests/core_handler_tests.cpp @@ -15,6 +15,12 @@ class CoreHandlerTestable : public CoreHandler< HttpHandlerMock > { auto & _http() { return http; } + + tl::expected< Document, Error > request(std::string_view path, const Document & body){ + static auto mr = std::pmr::get_default_resource(); + static RapidJSONPMRAlloc alloc{mr}; + return CoreHandler< HttpHandlerMock >::request(alloc, path, body); + } }; class CoreHandlerTests : public testing::Test { diff --git a/PAM/ssh/tests/init_test.cpp b/PAM/ssh/tests/init_test.cpp index 8874321..c2c6fef 100644 --- a/PAM/ssh/tests/init_test.cpp +++ b/PAM/ssh/tests/init_test.cpp @@ -37,8 +37,6 @@ class RublonHttpInitTest : public testing::Test { Init< MethodFactoryMock > sut; }; -using CoreReturn = tl::expected< Document, CoreHandlerError >; - TEST_F(RublonHttpInitTest, initializationSendsRequestOnGoodPath) { EXPECT_CALL(coreHandler, request("/api/transaction/init", _)).WillOnce(Return(RawCoreResponse{}.initMessage())); sut.handle(coreHandler, pam); diff --git a/PAM/ssh/tests/pam_info_mock.hpp b/PAM/ssh/tests/pam_info_mock.hpp index f6dc737..e15303f 100644 --- a/PAM/ssh/tests/pam_info_mock.hpp +++ b/PAM/ssh/tests/pam_info_mock.hpp @@ -16,7 +16,7 @@ class PamInfoMock { template < typename Fun, typename... Ti > auto scan(Fun && f, const char * fmt, Ti...) const noexcept -> std::result_of_t< Fun(char *) > { const auto & responseBuffer = scan_mock(fmt); - return responseBuffer.empty() ? std::nullopt : std::optional{f(responseBuffer.c_str())}; + return f(responseBuffer.c_str()); } template < typename... Ti > diff --git a/PAM/ssh/tests/passcode_auth_tests.cpp b/PAM/ssh/tests/passcode_auth_tests.cpp index a3740fd..c4c5dc2 100644 --- a/PAM/ssh/tests/passcode_auth_tests.cpp +++ b/PAM/ssh/tests/passcode_auth_tests.cpp @@ -25,7 +25,7 @@ class PasscodeBasedAuthTest : public Test { TEST_F(PasscodeBasedAuthTest, wrongPasscodeShouldFail) { EXPECT_CALL(coreHandler, request(_, _)).WillOnce(Return(RawCoreResponse{}.codeConfirmationMessage().withWrongCodeResponse())); - EXPECT_CALL(pam, bscan_mock(_)).WillOnce(Return("123456")); + EXPECT_CALL(pam, scan_mock(_)).WillOnce(Return("123456")); EXPECT_THAT(sut.handle(coreHandler, pam), Unexpected(WerificationError{WerificationError::WrongCode})); }