Skip to content

Commit

Permalink
auditlog: add logouts
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ijon committed Sep 11, 2024
1 parent 6e413ab commit ebfa9a3
Show file tree
Hide file tree
Showing 6 changed files with 228 additions and 10 deletions.
59 changes: 57 additions & 2 deletions ydb/core/security/login_page.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include <library/cpp/json/json_reader.h>
#include <library/cpp/json/json_writer.h>

#include <ydb/core/util/address_classifier.h>
#include <ydb/core/audit/audit_log.h>
#include <ydb/core/base/tablet_pipe.h>
#include <ydb/core/tx/scheme_cache/scheme_cache.h>
#include <ydb/core/tx/schemeshard/schemeshard.h>
Expand All @@ -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<NMon::IEvHttpInfoRes>;

class TLoginRequest : public NActors::TActorBootstrapped<TLoginRequest> {
Expand Down Expand Up @@ -251,6 +271,8 @@ class TLogoutRequest : public NActors::TActorBootstrapped<TLogoutRequest> {
STATEFN(StateWork) {
switch (ev->GetTypeRewrite()) {
hFunc(TEvents::TEvPoisonPill, HandlePoisonPill);
hFunc(TEvTicketParser::TEvAuthorizeTicketResult, Handle);
cFunc(TEvents::TSystem::Wakeup, HandleTimeout);
}
}

Expand All @@ -265,13 +287,42 @@ class TLogoutRequest : public NActors::TActorBootstrapped<TLogoutRequest> {
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");
Expand All @@ -291,12 +342,15 @@ class TLogoutRequest : public NActors::TActorBootstrapped<TLogoutRequest> {
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();
}

Expand All @@ -314,6 +368,7 @@ class TLogoutRequest : public NActors::TActorBootstrapped<TLogoutRequest> {
protected:
TActorId Sender;
NHttp::THttpIncomingRequestPtr Request;
TDuration Timeout = TDuration::Seconds(5);
};

class TLoginService : public TActor<TLoginService> {
Expand Down
1 change: 1 addition & 0 deletions ydb/core/security/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions ydb/core/tx/schemeshard/schemeshard_audit_log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <ydb/public/api/protos/ydb_export.pb.h>
#include <ydb/public/api/protos/ydb_import.pb.h>

#include <ydb/library/actors/http/http.h>

#include <ydb/core/protos/flat_tx_scheme.pb.h>
#include <ydb/core/protos/export.pb.h>
#include <ydb/core/protos/import.pb.h>
Expand Down Expand Up @@ -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))
Expand Down
6 changes: 6 additions & 0 deletions ydb/core/tx/schemeshard/schemeshard_audit_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ class TEvCreateImportRequest;
class TEvCreateImportResponse;
}

namespace NHttp {
class THttpIncomingRequest;
}

namespace NKikimr::NSchemeShard {

class TSchemeShard;
Expand All @@ -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);

}
154 changes: 153 additions & 1 deletion ydb/core/tx/schemeshard/ut_login/ut_login.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
#include <util/string/join.h>

#include <ydb/library/login/login.h>
#include <ydb/library/actors/http/http_proxy.h>
#include <ydb/core/tx/schemeshard/ut_helpers/helpers.h>
#include <ydb/core/tx/schemeshard/ut_helpers/auditlog_helpers.h>
#include <ydb/library/login/login.h>
#include <ydb/core/protos/auth.pb.h>
#include <ydb/core/security/ticket_parser.h>
#include <ydb/core/security/login_page.h>

using namespace NKikimr;
using namespace NSchemeShard;
Expand Down Expand Up @@ -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<TSockAddrInet>("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<TSockAddrInet>("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<std::string> 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<IEventHandle> handle;
auto responseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(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<IEventHandle> handle;
auto responseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(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<IEventHandle> handle;
auto responseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(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<IEventHandle> handle;
auto responseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(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");
}
}
}
15 changes: 8 additions & 7 deletions ydb/library/login/login.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
#include <openssl/pem.h>
#include <openssl/rand.h>

#include <util/generic/singleton.h>
#include <util/string/builder.h>
#include <util/string/cast.h>
#include <util/string/hex.h>

#include <deque>

Expand Down Expand Up @@ -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<std::string>{Audience});
}

if (request.ExternalAuth) {
Expand Down Expand Up @@ -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";
Expand All @@ -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<std::string>({Audience}));
// jwt.h require audience claim to be a set
verifier.with_audience(std::set<std::string>{Audience});
}
verifier.verify(decoded_token);
response.User = decoded_token.get_subject();
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit ebfa9a3

Please sign in to comment.