From c47e03f99130636d3f7e7e0cc714671f74eae740 Mon Sep 17 00:00:00 2001 From: Bartosz Wieczorek Date: Mon, 2 Oct 2023 18:51:38 +0200 Subject: [PATCH] Fail when user gives wrong code --- CMakeLists.txt | 4 +- .../rublon/authentication_step_interface.hpp | 2 +- PAM/ssh/include/rublon/core_handler.hpp | 7 +-- PAM/ssh/include/rublon/init.hpp | 4 +- .../rublon/method/passcode_based_auth.hpp | 19 ++++--- PAM/ssh/lib/pam.cpp | 49 +++++++++++-------- PAM/ssh/tests/core_handler_mock.hpp | 22 ++++----- PAM/ssh/tests/http_mock.hpp | 12 ++++- PAM/ssh/tests/init_test.cpp | 8 +-- PAM/ssh/tests/passcode_auth_tests.cpp | 4 +- 10 files changed, 75 insertions(+), 56 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 38648ae..78f635b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,8 +14,8 @@ set(CMAKE_CXX_EXTENSIONS NO) add_compile_options(-Wall -Wextra -Wpedantic -Wno-format-security) -#add_compile_options(-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize=null -fno-sanitize=alignment -fno-omit-frame-pointer) -#add_link_options(-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize=null -fno-sanitize=alignment -fno-omit-frame-pointer) +add_compile_options(-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize=null -fno-sanitize=alignment -fno-omit-frame-pointer) +add_link_options(-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize=null -fno-sanitize=alignment -fno-omit-frame-pointer) option(ENABLE_TESTS "Enable tests" OFF) diff --git a/PAM/ssh/include/rublon/authentication_step_interface.hpp b/PAM/ssh/include/rublon/authentication_step_interface.hpp index 6be7da0..99d0398 100644 --- a/PAM/ssh/include/rublon/authentication_step_interface.hpp +++ b/PAM/ssh/include/rublon/authentication_step_interface.hpp @@ -47,7 +47,7 @@ class AuthenticationStep { log(LogLevel::Error, "Socker error, forcing override"); return Error{PamBaypass{}}; case Error::k_CoreHandlerError: - return Error{PamBaypass{}}; + return Error{PamDeny{}}; default: Critical{}; } diff --git a/PAM/ssh/include/rublon/core_handler.hpp b/PAM/ssh/include/rublon/core_handler.hpp index 4a9bafb..602657d 100644 --- a/PAM/ssh/include/rublon/core_handler.hpp +++ b/PAM/ssh/include/rublon/core_handler.hpp @@ -67,13 +67,14 @@ class CoreHandler : public CoreHandlerInterface< CoreHandler< HttpHandler > > { return resp; } - + tl::unexpected< Error > handleCoreException(std::string_view exceptionString) const { - if(exceptionString == "UserBypassedException" or exceptionString == "UserNotFoundException") + if(exceptionString == "UserBypassedException" or exceptionString == "UserNotFoundException") { return tl::unexpected{Error{PamBaypass{}}}; - else + } else { return tl::unexpected{ Error{CoreHandlerError{CoreHandlerError::CoreException, std::string{exceptionString.data(), exceptionString.size()}}}}; + } } tl::unexpected< Error > handleHttpError() const { diff --git a/PAM/ssh/include/rublon/init.hpp b/PAM/ssh/include/rublon/init.hpp index 58736b5..61cb64f 100644 --- a/PAM/ssh/include/rublon/init.hpp +++ b/PAM/ssh/include/rublon/init.hpp @@ -49,8 +49,8 @@ class Init : public AuthenticationStep< Init< MethodSelect_t > > { Value params{rapidjson::kObjectType}; params.AddMember("userIP", Value{pam.ip().get(), alloc}, alloc); - 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 + params.AddMember("appVer", "v.0.0.1", alloc); /// TODO add version to cmake + params.AddMember("os", Value{osNameImpl().c_str(), alloc}, alloc); coreRequest.AddMember("params", std::move(params), alloc); } diff --git a/PAM/ssh/include/rublon/method/passcode_based_auth.hpp b/PAM/ssh/include/rublon/method/passcode_based_auth.hpp index 8731e1e..7dbfa4a 100644 --- a/PAM/ssh/include/rublon/method/passcode_based_auth.hpp +++ b/PAM/ssh/include/rublon/method/passcode_based_auth.hpp @@ -27,13 +27,13 @@ class PasscodeBasedAuth : public AuthenticationStep< PasscodeBasedAuth > { template < typename PamInfo_t = LinuxPam > tl::expected< std::reference_wrapper< Document >, Error > readPasscode(Document & body, const PamInfo_t & pam) const { - auto & alloc = body.GetAllocator(); + auto & alloc = body.GetAllocator(); auto vericode = pam.scan([](const char * userInput) { return std::string{userInput}; }, userMessage); if(isProperLength(vericode) and hasDigitsOnly(vericode)) { body.AddMember("vericode", Value{vericode.c_str(), alloc}, alloc); return body; - } + } return tl::unexpected{Error{WerificationError{WerificationError::WrongCode}}}; } @@ -44,14 +44,17 @@ class PasscodeBasedAuth : public AuthenticationStep< PasscodeBasedAuth > { return readPasscode(body, pam); } - tl::expected< AuthenticationStatus, Error > checkAuthenticationStatus(const Document & coreResponse) const { + template < typename PamInfo_t = LinuxPam > + tl::expected< AuthenticationStatus, Error > checkAuthenticationStatus(const Document & coreResponse, const PamInfo_t & pam) const { RapidJSONPMRStackAlloc< 1024 > alloc; auto error = JSONPointer{"/result/error", &alloc}.Get(coreResponse); if(error) { + pam.print("Wrong code"); return tl::unexpected{Error{WerificationError{WerificationError::WrongCode}}}; } - + + pam.print("Verification code validated"); return AuthenticationStatus{AuthenticationStatus::Action::Confirmed}; } @@ -66,9 +69,9 @@ class PasscodeBasedAuth : public AuthenticationStep< PasscodeBasedAuth > { RapidJSONPMRStackAlloc< 1024 > alloc{}; 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(alloc, uri, body); }; - const auto askForPasscodeAgain = [&](const auto & /*error*/) { return this->askForPasscodeAgain(body, pam); }; + const auto checkCodeValidity = [&](const auto & coreResponse) { return this->checkAuthenticationStatus(coreResponse, pam); }; + 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); this->addTid(body); @@ -76,7 +79,7 @@ class PasscodeBasedAuth : public AuthenticationStep< PasscodeBasedAuth > { return readPasscode(body, pam) // .or_else(askForPasscodeAgain) .and_then(requestAuthorization) - .and_then(checkAuthenticationStatus); + .and_then(checkCodeValidity); } }; diff --git a/PAM/ssh/lib/pam.cpp b/PAM/ssh/lib/pam.cpp index 10e2c8f..f95398a 100644 --- a/PAM/ssh/lib/pam.cpp +++ b/PAM/ssh/lib/pam.cpp @@ -48,33 +48,42 @@ pam_sm_authenticate(pam_handle_t * pamh, [[maybe_unused]] int flags, [[maybe_unu CoreHandler CH{rublonConfig.value()}; - auto selectMethod = [&](const MethodSelect & selector) { - rublon::log(LogLevel::Debug, "seelctor.create()"); - return selector.create(pam); - }; + auto selectMethod = [&](const MethodSelect & selector) { return selector.create(pam); }; auto confirmMethod = [&](const PostMethod & confirm) { return confirm.fire(CH); }; - auto verifi = [&](const MethodProxy & method) { return method.fire(CH, pam); }; + auto confirmCode = [&](const MethodProxy & method) { return method.fire(CH, pam); }; - auto authStatus = Init{rublonConfig.value()} - .fire(CH, pam) // - .and_then(selectMethod) - .and_then(confirmMethod) - .and_then(verifi); + auto allowLogin = [&](const AuthenticationStatus & status) -> tl::expected< int, Error > { + if(status.userAuthorized()) { + rublon::log(rublon::LogLevel::Info, "Auth OK"); + pam.print("RUBLON authentication SUCCESS!\n"); + return PAM_SUCCESS; + }else{ + rublon::log(rublon::LogLevel::Info, "User unauthorized"); + pam.print("RUBLON authentication FAILED"); + return PAM_MAXTRIES; + } + }; - if(authStatus.has_value()) { - rublon::log(rublon::LogLevel::Info, "Auth OK"); - pam.print("RUBLON authentication SUCCESS!\n"); - return PAM_SUCCESS; - } else { - const auto & error = authStatus.error(); + auto mapError = [&](const Error & error) -> tl::expected + { rublon::log( LogLevel::Error, "auth problems due to %d class and %d category", error.errorClass(), static_cast< int >(error.category())); if(error.is< PamBaypass >()) { pam.print("\n RUBLON authentication bypased"); return PAM_SUCCESS; } - } - pam.print("RUBLON authentication Success"); - rublon::log(LogLevel::Warning, "User login failed"); - return PAM_SUCCESS; + pam.print("RUBLON authentication FAILED"); + rublon::log(LogLevel::Warning, "User login failed"); + + return PAM_MAXTRIES; + }; + + auto ret = Init{rublonConfig.value()} + .fire(CH, pam) // + .and_then(selectMethod) + .and_then(confirmMethod) + .and_then(confirmCode) + .and_then(allowLogin).or_else(mapError); + + return ret.value_or(PAM_MAXTRIES); } diff --git a/PAM/ssh/tests/core_handler_mock.hpp b/PAM/ssh/tests/core_handler_mock.hpp index c8f9c32..5c7af53 100644 --- a/PAM/ssh/tests/core_handler_mock.hpp +++ b/PAM/ssh/tests/core_handler_mock.hpp @@ -8,19 +8,15 @@ namespace mocks { class CoreHandlerMock : public rublon::CoreHandlerInterface< CoreHandlerMock > { public: - 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); -// }; + tl::expected< rublon::Document, rublon::Error > + request(rublon::RapidJSONPMRAlloc &alloc, std::string_view path, const rublon::Document & req) const { +// const rublon::Document &doc = request(path, req).value(); +// rublon::Document ret{&alloc}; +// ret.CopyFrom(doc, alloc); + return request(path, req); + }; + + MOCK_METHOD(( tl::expected< rublon::Document, rublon::Error > ), request, ( std::string_view, const rublon::Document & ), (const)); }; } // namespace mocks diff --git a/PAM/ssh/tests/http_mock.hpp b/PAM/ssh/tests/http_mock.hpp index 12049b0..b316adb 100644 --- a/PAM/ssh/tests/http_mock.hpp +++ b/PAM/ssh/tests/http_mock.hpp @@ -20,7 +20,8 @@ rublon::Configuration conf{rublon::Configuration::Parameters{// true, true, false, - false}}; + false, + true}}; rublon::Configuration confBypass{rublon::Configuration::Parameters{// "320BAB778C4D4262B54CD243CDEFFAFD", "39e8d771d83a2ed3cc728811911c25", @@ -29,6 +30,7 @@ rublon::Configuration confBypass{rublon::Configuration::Parameters{// true, true, false, + true, true}}; inline std::string randomTID() { @@ -189,6 +191,11 @@ class ResponseBase { // options.error = rublon::Error{rublon::CoreHandlerError{rublon::CoreHandlerError::ConnectionError}}; return *this; } + + ResponseBase & withBasSignature() { + options.error = rublon::Error{rublon::CoreHandlerError{rublon::CoreHandlerError::BadSigature}}; + return *this; + } operator auto() { return options.error.category() == rublon::Error::k_None ? static_cast< Generator * >(this)->generate() : tl::unexpected{error()}; @@ -205,8 +212,9 @@ class ResponseBase { class RawCoreResponse : public ResponseBase< RawCoreResponse > { public: tl::expected< rublon::Document, rublon::Error > generate() { + static rublon::RapidJSONPMRAlloc alloc; auto jsonString = std::visit([&](const auto generator) { return generator.generate(options); }, _coreGenerator); - rublon::Document doc; + rublon::Document doc{&alloc}; doc.Parse(jsonString.c_str()); return doc; } diff --git a/PAM/ssh/tests/init_test.cpp b/PAM/ssh/tests/init_test.cpp index c2c6fef..426037f 100644 --- a/PAM/ssh/tests/init_test.cpp +++ b/PAM/ssh/tests/init_test.cpp @@ -48,10 +48,10 @@ TEST_F(RublonHttpInitTest, rublon_Accept_pamLoginWhenThereIsNoConnection) { AllOf(Not(HasValue()), IsPAMBaypass())); } -// TEST_F(RublonHttpInitTest, rublon_Decline_pamLoginWhenServerHasBadSignature) { -// EXPECT_CALL(coreHandler, request(_, _)).WillOnce(Return(tl::unexpected{CoreHandlerError{CoreHandlerError::BadSigature}})); -// EXPECT_THAT(sut.handle(coreHandler, pam), HoldsPamAction(PamAction::decline)); -// } + TEST_F(RublonHttpInitTest, rublon_Decline_pamLoginWhenServerHasBadSignature) { + EXPECT_CALL(coreHandler, request(_, _)).WillOnce(Return(RawCoreResponse{}.withBasSignature())); + EXPECT_THAT(sut.handle(coreHandler, pam), IsPAMDeny()); + } // TEST_F(RublonHttpInitTest, rublon_Decline_pamLoginWhenServerReturnsBrokenData) { // EXPECT_CALL(coreHandler, request(_, _)).WillOnce(Return(tl::unexpected{CoreHandlerError{CoreHandlerError::BrokenData}})); diff --git a/PAM/ssh/tests/passcode_auth_tests.cpp b/PAM/ssh/tests/passcode_auth_tests.cpp index c4c5dc2..474682a 100644 --- a/PAM/ssh/tests/passcode_auth_tests.cpp +++ b/PAM/ssh/tests/passcode_auth_tests.cpp @@ -26,6 +26,7 @@ class PasscodeBasedAuthTest : public Test { TEST_F(PasscodeBasedAuthTest, wrongPasscodeShouldFail) { EXPECT_CALL(coreHandler, request(_, _)).WillOnce(Return(RawCoreResponse{}.codeConfirmationMessage().withWrongCodeResponse())); EXPECT_CALL(pam, scan_mock(_)).WillOnce(Return("123456")); + EXPECT_CALL(pam, print_mock(_)); EXPECT_THAT(sut.handle(coreHandler, pam), Unexpected(WerificationError{WerificationError::WrongCode})); } @@ -37,7 +38,7 @@ TEST_F(PasscodeBasedAuthTest, whenGivenBadPasscodeWeNeedToAskAgain) { .WillOnce(Return("1_3456")) .WillOnce(Return("123456")); - EXPECT_CALL(pam, print_mock(_) ); + EXPECT_CALL(pam, print_mock(_) ).Times(2); EXPECT_THAT(sut.handle(coreHandler, pam), HasValue() ); } @@ -55,6 +56,7 @@ TEST_F(PasscodeBasedAuthTest, whenGiveenBadPasscodeMultipleTimesWeAbort) { TEST_F(PasscodeBasedAuthTest, goodPasscodeShouldPass){ EXPECT_CALL(coreHandler, request(_, _)).WillOnce(Return(RawCoreResponse{}.codeConfirmationMessage())); EXPECT_CALL(pam, scan_mock(_)).WillOnce(Return("123456")); + EXPECT_CALL(pam, print_mock(_)); EXPECT_THAT(sut.handle(coreHandler, pam), HasValue() ); }