From ebfa9a387f1e88a4443fcfb91abff1b6957838de Mon Sep 17 00:00:00 2001 From: Ivan Chelyubeev Date: Fri, 23 Aug 2024 11:41:17 +0300 Subject: [PATCH] auditlog: add logouts Add audit logging for web logout operation. Web logout now require proper authentication as audit record without name of the logged out user is meaningless. Previously web lougout was anonymous. KIKIMR-21764 --- ydb/core/security/login_page.cpp | 59 ++++++- ydb/core/security/ya.make | 1 + .../tx/schemeshard/schemeshard_audit_log.cpp | 3 + .../tx/schemeshard/schemeshard_audit_log.h | 6 + ydb/core/tx/schemeshard/ut_login/ut_login.cpp | 154 +++++++++++++++++- ydb/library/login/login.cpp | 15 +- 6 files changed, 228 insertions(+), 10 deletions(-) diff --git a/ydb/core/security/login_page.cpp b/ydb/core/security/login_page.cpp index 6f5f53029773..88d0c1765190 100644 --- a/ydb/core/security/login_page.cpp +++ b/ydb/core/security/login_page.cpp @@ -6,6 +6,8 @@ #include #include +#include +#include #include #include #include @@ -21,6 +23,24 @@ using namespace NKikimr; using namespace NSchemeShard; using namespace NMonitoring; +void AuditLogWebUILogout(const NHttp::THttpIncomingRequest& request, const TString& userSID) { + static const TString WebLoginComponentName = "web-login"; + static const TString LogoutOperationName = "LOGOUT"; + static const TString EmptyValue = "{none}"; + + auto remoteAddress = NKikimr::NAddressClassifier::ExtractAddress(request.Address->ToString()); + + // NOTE: audit field set here must be in sync with ydb/core/tx/schemeshard/schemeshard_audit_log.h, AuditLogLogin() + AUDIT_LOG( + AUDIT_PART("component", WebLoginComponentName) + AUDIT_PART("remote_address", (!remoteAddress.empty() ? remoteAddress : EmptyValue)) + AUDIT_PART("subject", (!userSID.empty() ? userSID : EmptyValue)) + //NOTE: no database specified as web logout considered cluster-wide + AUDIT_PART("operation", LogoutOperationName) + AUDIT_PART("status", TString("SUCCESS")) + ); +} + using THttpResponsePtr = THolder; class TLoginRequest : public NActors::TActorBootstrapped { @@ -251,6 +271,8 @@ class TLogoutRequest : public NActors::TActorBootstrapped { STATEFN(StateWork) { switch (ev->GetTypeRewrite()) { hFunc(TEvents::TEvPoisonPill, HandlePoisonPill); + hFunc(TEvTicketParser::TEvAuthorizeTicketResult, Handle); + cFunc(TEvents::TSystem::Wakeup, HandleTimeout); } } @@ -265,13 +287,42 @@ class TLogoutRequest : public NActors::TActorBootstrapped { return ReplyErrorAndPassAway("400", "Bad Request", "Invalid method"); } - ReplyDeleteCookieAndPassAway(); + NHttp::TCookies cookies(NHttp::THeaders(Request->Headers)["Cookie"]); + TStringBuf ydbSessionId = cookies["ydb_session_id"]; + if (ydbSessionId.empty()) { + return ReplyErrorAndPassAway("401", "Unauthorized", "No ydb_session_id cookie"); + } + + Send(NKikimr::MakeTicketParserID(), new NKikimr::TEvTicketParser::TEvAuthorizeTicket({ + .Database = TString(), + .Ticket = TString("Login ") + ydbSessionId, + .PeerName = Request->Address->ToString(), + })); + + Become(&TThis::StateWork, Timeout, new TEvents::TEvWakeup()); + } + + void Handle(TEvTicketParser::TEvAuthorizeTicketResult::TPtr& ev) { + const TEvTicketParser::TEvAuthorizeTicketResult& result = *ev->Get(); + if (result.Error) { + return ReplyErrorAndPassAway("403", "Forbidden", result.Error.Message); + } + if (result.Token == nullptr) { + return ReplyErrorAndPassAway("403", "Forbidden", "Empty token"); + } + + ReplyDeleteCookieAndPassAway(result.Token->GetUserSID()); } void HandlePoisonPill(TEvents::TEvPoisonPill::TPtr&) { PassAway(); } + void HandleTimeout() { + ALOG_ERROR(NActorsServices::HTTP, Request->Address << " " << Request->Method << " " << Request->URL << " timeout"); + ReplyErrorAndPassAway("504", "Gateway Timeout", "Timeout"); + } + void ReplyOptionsAndPassAway() { NHttp::THeadersBuilder headers; headers.Set("Allow", "OPTIONS, POST"); @@ -291,12 +342,15 @@ class TLogoutRequest : public NActors::TActorBootstrapped { headers.Set("Access-Control-Allow-Methods", "OPTIONS, GET, POST"); } - void ReplyDeleteCookieAndPassAway() { + void ReplyDeleteCookieAndPassAway(const TString& userSID) { ALOG_DEBUG(NActorsServices::HTTP, "Logout success"); NHttp::THeadersBuilder headers; SetCORS(headers); headers.Set("Set-Cookie", "ydb_session_id=; Max-Age=0"); Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(Request->CreateResponse("200", "OK", headers))); + + AuditLogWebUILogout(*Request, userSID); + PassAway(); } @@ -314,6 +368,7 @@ class TLogoutRequest : public NActors::TActorBootstrapped { protected: TActorId Sender; NHttp::THttpIncomingRequestPtr Request; + TDuration Timeout = TDuration::Seconds(5); }; class TLoginService : public TActor { diff --git a/ydb/core/security/ya.make b/ydb/core/security/ya.make index b4dd549e0dac..d9361c2de4d7 100644 --- a/ydb/core/security/ya.make +++ b/ydb/core/security/ya.make @@ -16,6 +16,7 @@ PEERDIR( ydb/library/grpc/actor_client library/cpp/monlib/service/pages library/cpp/openssl/io + ydb/core/audit ydb/core/base ydb/core/protos ydb/library/aclib diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp index 7e0cd3f54d87..a5b5c7e8272d 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp @@ -3,6 +3,8 @@ #include #include +#include + #include #include #include @@ -383,6 +385,7 @@ void AuditLogLogin(const NKikimrScheme::TEvLogin& request, const NKikimrScheme:: TPath databasePath = TPath::Root(SS); auto peerName = NKikimr::NAddressClassifier::ExtractAddress(request.GetPeerName()); + // NOTE: audit field set here must be in sync with ydb/core/security/audit_log.h, AuditLogWebUILogout() AUDIT_LOG( AUDIT_PART("component", SchemeshardComponentName) AUDIT_PART("remote_address", (!peerName.empty() ? peerName : EmptyValue)) diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log.h b/ydb/core/tx/schemeshard/schemeshard_audit_log.h index 1053823b4d00..7d095db2122d 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log.h +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log.h @@ -20,6 +20,10 @@ class TEvCreateImportRequest; class TEvCreateImportResponse; } +namespace NHttp { +class THttpIncomingRequest; +} + namespace NKikimr::NSchemeShard { class TSchemeShard; @@ -36,4 +40,6 @@ void AuditLogImportStart(const NKikimrImport::TEvCreateImportRequest& request, c void AuditLogImportEnd(const TImportInfo& importInfo, TSchemeShard* SS); void AuditLogLogin(const NKikimrScheme::TEvLogin& request, const NKikimrScheme::TEvLoginResult& response, TSchemeShard* SS); +void AuditLogWebUILogout(const NHttp::THttpIncomingRequest& request, const TString& userSID); + } diff --git a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp index 1401936c99f7..e9a8cae8bbcd 100644 --- a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp +++ b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp @@ -1,9 +1,12 @@ #include +#include +#include #include #include -#include #include +#include +#include using namespace NKikimr; using namespace NSchemeShard; @@ -126,3 +129,152 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { UNIT_ASSERT_STRING_CONTAINS(last, "login_auth_domain={none}"); } } + +namespace NSchemeShardUT_Private { + +void EatWholeString(NHttp::THttpIncomingRequestPtr request, const TString& data) { + request->EnsureEnoughSpaceAvailable(data.size()); + auto size = std::min(request->Avail(), data.size()); + memcpy(request->Pos(), data.data(), size); + request->Advance(size); +} + +NHttp::THttpIncomingRequestPtr MakeLoginRequest(const TString& user, const TString& password) { + TString payload = [](const auto& user, const auto& password) { + NJson::TJsonValue value; + value["user"] = user; + value["password"] = password; + return NJson::WriteJson(value, false); + }(user, password); + TStringBuilder text; + text << "POST /login HTTP/1.1\r\n" + << "Host: test.ydb\r\n" + << "Content-Type: application/json\r\n" + << "Content-Length: " << payload.size() << "\r\n" + << "\r\n" + << payload; + NHttp::THttpIncomingRequestPtr request = new NHttp::THttpIncomingRequest(); + EatWholeString(request, text); + // WebLoginService will crash without address + request->Address = std::make_shared("127.0.0.1", 0); + // Cerr << "TEST: http login request: " << text << Endl; + return request; +} + +NHttp::THttpIncomingRequestPtr MakeLogoutRequest(const TString& cookieName, const TString& cookieValue) { + TStringBuilder text; + text << "POST /logout HTTP/1.1\r\n" + << "Host: test.ydb\r\n" + << "Content-Type: text/plain\r\n" + << "Cookie: " << cookieName << "=" << cookieValue << "\r\n" + << "\r\n"; + NHttp::THttpIncomingRequestPtr request = new NHttp::THttpIncomingRequest(); + EatWholeString(request, text); + // WebLoginService will crash without address + request->Address = std::make_shared("127.0.0.1", 0); + // Cerr << "TEST: http logout request: " << text << Endl; + return request; +} + +} + +Y_UNIT_TEST_SUITE(TWebLoginService) { + + Y_UNIT_TEST(Logout) { + TTestBasicRuntime runtime; + std::vector lines; + runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + TTestEnv env(runtime); + + // Add ticket parser to the mix + { + NKikimrProto::TAuthConfig authConfig; + authConfig.SetUseBlackBox(false); + authConfig.SetUseLoginProvider(true); + + IActor* ticketParser = NKikimr::CreateTicketParser({.AuthConfig = authConfig}); + TActorId ticketParserId = runtime.Register(ticketParser); + runtime.RegisterService(NKikimr::MakeTicketParserID(), ticketParserId); + } + + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 1); // alter root subdomain + + ui64 txId = 100; + + TestCreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1", {{NKikimrScheme::StatusSuccess}}); + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 2); // +user creation + + // test body + const auto target = runtime.Register(CreateWebLoginService()); + const auto edge = runtime.AllocateEdgeActor(); + + TString ydbSessionId; + { + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLoginRequest("user1", "password1") + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "200"); + NHttp::THeaders headers(responseEv->Response->Headers); + NHttp::TCookies cookies(headers["Set-Cookie"]); + ydbSessionId = cookies["ydb_session_id"]; + } + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 3); // +user login + + // New security keys are created in the subdomain as a consequence of a login. + // In real system they are transferred to the ticket parser by the grpc-proxy + // on receiving subdomain update notification. + // Here there are no grpc-proxy, so we should transfer keys to the ticket parser manually. + { + const auto describe = DescribePath(runtime, "/MyRoot"); + const auto& securityState = describe.GetPathDescription().GetDomainDescription().GetSecurityState(); + TActorId edge = runtime.AllocateEdgeActor(); + runtime.Send(new IEventHandle(MakeTicketParserID(), edge, new TEvTicketParser::TEvUpdateLoginSecurityState(securityState)), 0); + } + + // Then we are ready to test some authentication on /logout + { // no cookie + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLogoutRequest("not-an-ydb_session_id", ydbSessionId) + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "401"); + + // no audit record for actions without auth + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 3); + } + { // bad cookie + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLogoutRequest("ydb_session_id", "jklhagsfjhg") + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "403"); + + // no audit record for actions without auth + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 3); + } + { // good cookie + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLogoutRequest("ydb_session_id", ydbSessionId) + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "200"); + + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 4); // +user web logout + + auto last = FindAuditLine(lines, "operation=LOGOUT"); + UNIT_ASSERT_STRING_CONTAINS(last, "component=web-login"); + UNIT_ASSERT_STRING_CONTAINS(last, "remote_address="); // can't check the value + UNIT_ASSERT_STRING_CONTAINS(last, "operation=LOGOUT"); + UNIT_ASSERT_STRING_CONTAINS(last, "status=SUCCESS"); + } + } +} diff --git a/ydb/library/login/login.cpp b/ydb/library/login/login.cpp index 9e4bcd646156..403e2502f0ed 100644 --- a/ydb/library/login/login.cpp +++ b/ydb/library/login/login.cpp @@ -8,10 +8,7 @@ #include #include -#include #include -#include -#include #include @@ -339,7 +336,8 @@ TLoginProvider::TLoginUserResponse TLoginProvider::LoginUser(const TLoginUserReq .set_issued_at(now) .set_expires_at(expires_at); if (!Audience.empty()) { - token.set_audience(Audience); + // jwt.h require audience claim to be a set + token.set_audience(std::set{Audience}); } if (request.ExternalAuth) { @@ -381,7 +379,7 @@ TLoginProvider::TValidateTokenResponse TLoginProvider::ValidateToken(const TVali try { jwt::decoded_jwt decoded_token = jwt::decode(request.Token); if (Audience) { - // we check audience manually because we wan't this error instead of wrong key id in case of databases mismatch + // we check audience manually because we want an explicit error instead of wrong key id in case of databases mismatch auto audience = decoded_token.get_audience(); if (audience.empty() || TString(*audience.begin()) != Audience) { response.Error = "Wrong audience"; @@ -394,7 +392,8 @@ TLoginProvider::TValidateTokenResponse TLoginProvider::ValidateToken(const TVali auto verifier = jwt::verify() .allow_algorithm(jwt::algorithm::ps256(key->PublicKey)); if (Audience) { - verifier.with_audience(std::set({Audience})); + // jwt.h require audience claim to be a set + verifier.with_audience(std::set{Audience}); } verifier.verify(decoded_token); response.User = decoded_token.get_subject(); @@ -435,8 +434,10 @@ TLoginProvider::TValidateTokenResponse TLoginProvider::ValidateToken(const TVali response.Error = "Key not found"; } } - } catch (const jwt::token_verification_exception& e) { + } catch (const jwt::signature_verification_exception& e) { response.Error = e.what(); // invalid token signature + } catch (const jwt::token_verification_exception& e) { + response.Error = e.what(); // invalid token } catch (const std::invalid_argument& e) { response.Error = "Token is not in correct format"; response.TokenUnrecognized = true;