Skip to content

Commit 0a7a710

Browse files
UgnineSirdisUgnineSirdis
authored
Sanitized token field for audit log (#9287)
1 parent 9b3c2bd commit 0a7a710

26 files changed

+163
-34
lines changed

‎ydb/core/audit/audit_log_impl.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ TString GetJsonLog(const TEvAuditLog::TEvWriteAuditLog::TPtr& ev) {
9393

9494
TString GetJsonLogCompatibleLog(const TEvAuditLog::TEvWriteAuditLog::TPtr& ev) {
9595
const auto* msg = ev->Get();
96-
NJsonWriter::TBuf json;
96+
TStringStream ss;
97+
NJsonWriter::TBuf json(NJsonWriter::HEM_DONT_ESCAPE_HTML, &ss);
9798
{
9899
auto obj = json.BeginObject();
99100
obj
@@ -107,7 +108,8 @@ TString GetJsonLogCompatibleLog(const TEvAuditLog::TEvWriteAuditLog::TPtr& ev) {
107108
}
108109
json.EndObject();
109110
}
110-
return json.Str();
111+
ss << Endl;
112+
return ss.Str();
111113
}
112114

113115
TString GetTxtLog(const TEvAuditLog::TEvWriteAuditLog::TPtr& ev) {

‎ydb/core/cms/console/console__replace_yaml_config.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
2020
, Config(ev->Get()->Record.GetRequest().config())
2121
, Peer(ev->Get()->Record.GetPeerName())
2222
, Sender(ev->Sender)
23-
, UserSID(NACLib::TUserToken(ev->Get()->Record.GetUserToken()).GetUserSID())
23+
, UserToken(ev->Get()->Record.GetUserToken())
2424
, Force(force)
2525
, AllowUnknownFields(ev->Get()->Record.GetRequest().allow_unknown_fields())
2626
, DryRun(ev->Get()->Record.GetRequest().dry_run())
@@ -57,7 +57,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
5757
oldVolatileConfig.SetConfig(config);
5858
}
5959

60-
Self->Logger.DbLogData(UserSID, logData, txc, ctx);
60+
Self->Logger.DbLogData(UserToken.GetUserSID(), logData, txc, ctx);
6161
}
6262

6363
bool Execute(TTransactionContext &txc, const TActorContext &ctx) override
@@ -190,7 +190,8 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
190190
if (!Error && Modify && !DryRun) {
191191
AuditLogReplaceConfigTransaction(
192192
/* peer = */ Peer,
193-
/* userSID = */ UserSID,
193+
/* userSID = */ UserToken.GetUserSID(),
194+
/* sanitizedToken = */ UserToken.GetSanitizedToken(),
194195
/* oldConfig = */ Self->YamlConfig,
195196
/* newConfig = */ Config,
196197
/* reason = */ {},
@@ -207,7 +208,8 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
207208
} else if (Error && !DryRun) {
208209
AuditLogReplaceConfigTransaction(
209210
/* peer = */ Peer,
210-
/* userSID = */ UserSID,
211+
/* userSID = */ UserToken.GetUserSID(),
212+
/* sanitizedToken = */ UserToken.GetSanitizedToken(),
211213
/* oldConfig = */ Self->YamlConfig,
212214
/* newConfig = */ Config,
213215
/* reason = */ ErrorReason,
@@ -221,7 +223,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
221223
const TString Config;
222224
const TString Peer;
223225
const TActorId Sender;
224-
const TString UserSID;
226+
const NACLib::TUserToken UserToken;
225227
const bool Force = false;
226228
const bool AllowUnknownFields = false;
227229
const bool DryRun = false;

‎ydb/core/cms/console/console_audit.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ namespace NKikimr::NConsole {
88
void AuditLogReplaceConfigTransaction(
99
const TString& peer,
1010
const TString& userSID,
11+
const TString& sanitizedToken,
1112
const TString& oldConfig,
1213
const TString& newConfig,
1314
const TString& reason,
@@ -23,6 +24,7 @@ void AuditLogReplaceConfigTransaction(
2324
AUDIT_PART("component", COMPONENT_NAME)
2425
AUDIT_PART("remote_address", (!peerName.empty() ? peerName : EMPTY_VALUE))
2526
AUDIT_PART("subject", (!userSID.empty() ? userSID : EMPTY_VALUE))
27+
AUDIT_PART("sanitized_token", (!sanitizedToken.empty() ? sanitizedToken : EMPTY_VALUE))
2628
AUDIT_PART("status", TString(success ? "SUCCESS" : "ERROR"))
2729
AUDIT_PART("reason", reason, !reason.empty())
2830
AUDIT_PART("operation", TString("REPLACE DYNCONFIG"))

‎ydb/core/cms/console/console_audit.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ namespace NKikimr::NConsole {
77
void AuditLogReplaceConfigTransaction(
88
const TString& peer,
99
const TString& userSID,
10+
const TString& sanitizedToken,
1011
const TString& oldConfig,
1112
const TString& newConfig,
1213
const TString& reason,

‎ydb/core/cms/console/console_configs_manager.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,7 @@ void TConfigsManager::HandleUnauthorized(TEvConsole::TEvReplaceYamlConfigRequest
980980
AuditLogReplaceConfigTransaction(
981981
/* peer = */ ev->Get()->Record.GetPeerName(),
982982
/* userSID = */ ev->Get()->Record.GetUserToken(),
983+
/* sanitizedToken = */ TString(),
983984
/* oldConfig = */ YamlConfig,
984985
/* newConfig = */ ev->Get()->Record.GetRequest().config(),
985986
/* reason = */ "Unauthorized.",
@@ -990,6 +991,7 @@ void TConfigsManager::HandleUnauthorized(TEvConsole::TEvSetYamlConfigRequest::TP
990991
AuditLogReplaceConfigTransaction(
991992
/* peer = */ ev->Get()->Record.GetPeerName(),
992993
/* userSID = */ ev->Get()->Record.GetUserToken(),
994+
/* sanitizedToken = */ TString(),
993995
/* oldConfig = */ YamlConfig,
994996
/* newConfig = */ ev->Get()->Record.GetRequest().config(),
995997
/* reason = */ "Unauthorized.",

‎ydb/core/grpc_services/audit_dml_operations.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@ namespace {
6161

6262
namespace NKikimr::NGRpcService {
6363

64-
void AuditContextStart(IAuditCtx* ctx, const TString& database, const TString& userSID, const std::vector<std::pair<TString, TString>>& databaseAttrs) {
64+
void AuditContextStart(IAuditCtx* ctx, const TString& database, const TString& userSID, const TString& sanitizedToken, const std::vector<std::pair<TString, TString>>& databaseAttrs) {
6565
ctx->AddAuditLogPart("remote_address", NKikimr::NAddressClassifier::ExtractAddress(ctx->GetPeerName()));
6666
ctx->AddAuditLogPart("subject", userSID);
67+
static const TString EMPTY_VALUE = "{none}";
68+
ctx->AddAuditLogPart("sanitized_token", !sanitizedToken.empty() ? sanitizedToken : EMPTY_VALUE);
6769
ctx->AddAuditLogPart("database", database);
6870
ctx->AddAuditLogPart("operation", ctx->GetRequestName());
6971
ctx->AddAuditLogPart("start_time", TInstant::Now().ToString());

‎ydb/core/grpc_services/audit_dml_operations.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class IAuditCtx;
4545
// AuditContextAppend() specializations extract specific info from request (and result) protos.
4646
//
4747

48-
void AuditContextStart(IAuditCtx* ctx, const TString& database, const TString& userSID, const std::vector<std::pair<TString, TString>>& databaseAttrs);
48+
void AuditContextStart(IAuditCtx* ctx, const TString& database, const TString& userSID, const TString& sanitizedToken, const std::vector<std::pair<TString, TString>>& databaseAttrs);
4949
void AuditContextEnd(IAuditCtx* ctx);
5050

5151
template <class TProtoRequest>

‎ydb/core/grpc_services/audit_log.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
namespace NKikimr {
1010
namespace NGRpcService {
1111

12-
void AuditLogConn(const IRequestProxyCtx* ctx, const TString& database, const TString& userSID)
12+
//NOTE: EmptyValue couldn't be an empty string as AUDIT_PART() skips parts with an empty values
13+
static const TString EmptyValue = "{none}";
14+
15+
void AuditLogConn(const IRequestProxyCtx* ctx, const TString& database, const TString& userSID, const TString& sanitizedToken)
1316
{
1417
static const TString GrpcConnComponentName = "grpc-conn";
1518

@@ -18,6 +21,7 @@ void AuditLogConn(const IRequestProxyCtx* ctx, const TString& database, const TS
1821

1922
AUDIT_PART("remote_address", NKikimr::NAddressClassifier::ExtractAddress(ctx->GetPeerName()))
2023
AUDIT_PART("subject", userSID)
24+
AUDIT_PART("sanitized_token", (!sanitizedToken.empty() ? sanitizedToken : EmptyValue))
2125
AUDIT_PART("database", database)
2226
AUDIT_PART("operation", ctx->GetRequestName())
2327
);
@@ -35,9 +39,6 @@ void AuditLog(ui32 status, const TAuditLogParts& parts)
3539
{
3640
static const TString GrpcProxyComponentName = "grpc-proxy";
3741

38-
//NOTE: EmptyValue couldn't be an empty string as AUDIT_PART() skips parts with an empty values
39-
static const TString EmptyValue = "{none}";
40-
4142
AUDIT_LOG(
4243
AUDIT_PART("component", GrpcProxyComponentName)
4344

@@ -56,4 +57,3 @@ void AuditLog(ui32 status, const TAuditLogParts& parts)
5657

5758
}
5859
}
59-

‎ydb/core/grpc_services/audit_log.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class IRequestProxyCtx;
88
class IRequestCtxMtSafe;
99

1010
// grpc "connections" log
11-
void AuditLogConn(const IRequestProxyCtx* reqCtx, const TString& database, const TString& userSID);
11+
void AuditLogConn(const IRequestProxyCtx* reqCtx, const TString& database, const TString& userSID, const TString& sanitizedToken);
1212

1313
using TAuditLogParts = TVector<std::pair<TString, TString>>;
1414

‎ydb/core/grpc_services/grpc_request_check_actor.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ class TGrpcRequestCheckActor
179179

180180
if (AppData(ctx)->FeatureFlags.GetEnableGrpcAudit()) {
181181
// log info about input connection (remote address, basically)
182-
AuditLogConn(GrpcRequestBaseCtx_, CheckedDatabaseName_, TBase::GetUserSID());
182+
AuditLogConn(GrpcRequestBaseCtx_, CheckedDatabaseName_, TBase::GetUserSID(), TBase::GetSanitizedToken());
183183
}
184184

185185
// Simple rps limitation
@@ -417,11 +417,11 @@ class TGrpcRequestCheckActor
417417
return DmlAuditEnabled_ && !DmlAuditExpectedSubjects_.contains(userSID);
418418
};
419419

420-
void AuditRequest(IRequestProxyCtx* requestBaseCtx, const TString& databaseName, const TString& userSID) const {
420+
void AuditRequest(IRequestProxyCtx* requestBaseCtx, const TString& databaseName, const TString& userSID, const TString& sanitizedToken) const {
421421
const bool dmlAuditEnabled = requestBaseCtx->IsAuditable() && IsAuditEnabledFor(userSID);
422422

423423
if (dmlAuditEnabled) {
424-
AuditContextStart(requestBaseCtx, databaseName, userSID, Attributes_);
424+
AuditContextStart(requestBaseCtx, databaseName, userSID, sanitizedToken, Attributes_);
425425
requestBaseCtx->SetAuditLogHook([requestBaseCtx](ui32 status, const TAuditLogParts& parts) {
426426
AuditContextEnd(requestBaseCtx);
427427
AuditLog(status, parts);
@@ -475,7 +475,7 @@ class TGrpcRequestCheckActor
475475
void HandleAndDie(TAutoPtr<TEventHandle<TEvProxyRuntimeEvent>>& event) {
476476
// Request audit happen after successful authentication
477477
// and authorization check against the database
478-
AuditRequest(GrpcRequestBaseCtx_, CheckedDatabaseName_, TBase::GetUserSID());
478+
AuditRequest(GrpcRequestBaseCtx_, CheckedDatabaseName_, TBase::GetUserSID(), TBase::GetSanitizedToken());
479479

480480
GrpcRequestBaseCtx_->FinishSpan();
481481
event->Release().Release()->Pass(*this);

0 commit comments

Comments
 (0)