Skip to content

Commit

Permalink
Check client certificate/token when option EnforceUserTokenCheckRequi…
Browse files Browse the repository at this point in the history 
…rement is on (ydb-platform#7511)

(cherry picked from commit 1a6f195)
  • Loading branch information
@UgnineSirdis @uzhastik
UgnineSirdis authored and uzhastik committed Sep 24, 2024
1 parent 8ced177 commit 842aebb
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 74 deletions.
19 changes: 16 additions & 3 deletions ydb/core/grpc_services/grpc_request_proxy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,9 +420,22 @@ void TGRpcRequestProxyImpl::HandleUndelivery(TEvents::TEvUndelivered::TPtr& ev)

bool TGRpcRequestProxyImpl::IsAuthStateOK(const IRequestProxyCtx& ctx) {
const auto& state = ctx.GetAuthState();
return state.State == NYdbGrpc::TAuthState::AS_OK ||
state.State == NYdbGrpc::TAuthState::AS_FAIL && state.NeedAuth == false ||
state.NeedAuth == false && !ctx.GetYdbToken();
if (state.State == NYdbGrpc::TAuthState::AS_OK) {
return true;
}

const bool authorizationParamsAreSet = ctx.GetYdbToken() || !ctx.FindClientCertPropertyValues().empty();
if (!state.NeedAuth && !authorizationParamsAreSet) {
return true;
}

if (!state.NeedAuth && state.State == NYdbGrpc::TAuthState::AS_FAIL) {
if (AppData()->EnforceUserTokenCheckRequirement && authorizationParamsAreSet) {
return false;
}
return true;
}
return false;
}

void TGRpcRequestProxyImpl::MaybeStartTracing(IRequestProxyCtx& ctx) {
Expand Down
19 changes: 16 additions & 3 deletions ydb/core/grpc_services/grpc_request_proxy_simple.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,22 @@ void TGRpcRequestProxySimple::HandleUndelivery(TEvents::TEvUndelivered::TPtr& ev

bool TGRpcRequestProxySimple::IsAuthStateOK(const IRequestProxyCtx& ctx) {
const auto& state = ctx.GetAuthState();
return state.State == NYdbGrpc::TAuthState::AS_OK ||
state.State == NYdbGrpc::TAuthState::AS_FAIL && state.NeedAuth == false ||
state.NeedAuth == false && !ctx.GetYdbToken();
if (state.State == NYdbGrpc::TAuthState::AS_OK) {
return true;
}

const bool authorizationParamsAreSet = ctx.GetYdbToken() || !ctx.FindClientCertPropertyValues().empty();
if (!state.NeedAuth && !authorizationParamsAreSet) {
return true;
}

if (!state.NeedAuth && state.State == NYdbGrpc::TAuthState::AS_FAIL) {
if (AppData()->EnforceUserTokenCheckRequirement && authorizationParamsAreSet) {
return false;
}
return true;
}
return false;
}

template<typename TEvent>
Expand Down
18 changes: 8 additions & 10 deletions ydb/library/grpc/server/grpc_request.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,11 @@ class TGRpcRequestImpl
, RequestLimiter_(std::move(limiter))
, Writer_(new grpc::ServerAsyncResponseWriter<TUniversalResponseRef<TOut>>(&this->Context))
, StateFunc_(&TThis::SetRequestDone)
, Request_(google::protobuf::Arena::CreateMessage<TIn>(&Arena_))
, AuthState_(Server_->NeedAuth())
{
AuthState_ = Server_->NeedAuth() ? TAuthState(true) : TAuthState(false);
Request_ = google::protobuf::Arena::CreateMessage<TIn>(&Arena_);
Y_ABORT_UNLESS(Request_);
GRPC_LOG_DEBUG(Logger_, "[%p] created request Name# %s", this, Name_);
FinishPromise_ = NThreading::NewPromise<EFinishStatus>();
}

TGRpcRequestImpl(TService* server,
Expand All @@ -101,13 +100,12 @@ class TGRpcRequestImpl
, RequestLimiter_(std::move(limiter))
, StreamWriter_(new grpc::ServerAsyncWriter<TUniversalResponse<TOut>>(&this->Context))
, StateFunc_(&TThis::SetRequestDone)
, Request_(google::protobuf::Arena::CreateMessage<TIn>(&Arena_))
, AuthState_(Server_->NeedAuth())
, StreamAdaptor_(CreateStreamAdaptor())
{
AuthState_ = Server_->NeedAuth() ? TAuthState(true) : TAuthState(false);
Request_ = google::protobuf::Arena::CreateMessage<TIn>(&Arena_);
Y_ABORT_UNLESS(Request_);
GRPC_LOG_DEBUG(Logger_, "[%p] created streaming request Name# %s", this, Name_);
FinishPromise_ = NThreading::NewPromise<EFinishStatus>();
StreamAdaptor_ = CreateStreamAdaptor();
}

TAsyncFinishResult GetFinishFuture() override {
Expand Down Expand Up @@ -549,7 +547,7 @@ class TGRpcRequestImpl
}

using TStateFunc = bool (TThis::*)(bool);
TService* Server_;
TService* Server_ = nullptr;
TOnRequest Cb_;
TRequestCallback RequestCallback_;
TStreamRequestCallback StreamRequestCallback_;
Expand All @@ -561,9 +559,9 @@ class TGRpcRequestImpl
THolder<grpc::ServerAsyncResponseWriter<TUniversalResponseRef<TOut>>> Writer_;
THolder<grpc::ServerAsyncWriterInterface<TUniversalResponse<TOut>>> StreamWriter_;
TStateFunc StateFunc_;
TIn* Request_;

google::protobuf::Arena Arena_;
TIn* Request_ = nullptr;
TOnNextReply NextReplyCb_;
ui32 RequestSize = 0;
ui32 ResponseSize = 0;
Expand All @@ -577,7 +575,7 @@ class TGRpcRequestImpl

using TFixedEvent = TQueueFixedEvent<TGRpcRequestImpl>;
TFixedEvent OnFinishTag = { this, &TGRpcRequestImpl::OnFinish };
NThreading::TPromise<EFinishStatus> FinishPromise_;
NThreading::TPromise<EFinishStatus> FinishPromise_ = NThreading::NewPromise<EFinishStatus>();
bool SkipUpdateCountersOnError = false;
IStreamAdaptor::TPtr StreamAdaptor_;
std::atomic<bool> ClientLost_ = false;
Expand Down
2 changes: 1 addition & 1 deletion ydb/services/ydb/ydb_common_ut.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class TBasicKikimrWithGrpcAndRootSchema {

NYdbGrpc::TServerOptions grpcOption;
if (TestSettings::AUTH) {
grpcOption.SetUseAuth(true);
grpcOption.SetUseAuth(appConfig.GetDomainsConfig().GetSecurityConfig().GetEnforceUserTokenRequirement()); // In real life UseAuth is initialized with EnforceUserTokenRequirement. To avoid incorrect tests we must do the same.
}
grpcOption.SetPort(grpc);
if (TestSettings::SSL) {
Expand Down
45 changes: 45 additions & 0 deletions ydb/services/ydb/ydb_register_node_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ struct TKikimrServerForTestNodeRegistration : TBasicKikimrWithGrpcAndRootSchema<

struct TServerInitialization {
bool EnforceUserToken = false;
bool EnforceCheckUserToken = false; // Takes effect when EnforceUserToken = false
bool EnableDynamicNodeAuth = false;
bool EnableWrongIdentity = false;
bool SetNodeAuthValues = false;
Expand All @@ -72,6 +73,9 @@ struct TKikimrServerForTestNodeRegistration : TBasicKikimrWithGrpcAndRootSchema<
if (serverInitialization.EnforceUserToken) {
securityConfig.SetEnforceUserTokenRequirement(true);
}
if (serverInitialization.EnforceCheckUserToken) {
securityConfig.SetEnforceUserTokenCheckRequirement(true);
}
if (serverInitialization.EnableDynamicNodeAuth) {
config.MutableClientCertificateAuthorization()->SetRequestClientCertificate(true);
// config.MutableFeatureFlags()->SetEnableDynamicNodeAuthorization(true);
Expand Down Expand Up @@ -884,6 +888,47 @@ Y_UNIT_TEST(ServerWithoutCertVerification_ClientDoesNotProvideClientCerts) {
}
}

Y_UNIT_TEST(ServerWithCertVerification_AuthNotRequired) {
// Scenario when we want to turn on secure node registration, but to check it in safe way
const TCertAndKey& caCert = TKikimrTestWithServerCert::GetCACertAndKey();
TProps props = TProps::AsClientServer();
const TCertAndKey clientServerCert = GenerateSignedCert(caCert, props);

props.Organization = "Enemy Org";
const TCertAndKey clientServerEnemyCert = GenerateSignedCert(caCert, props);

TKikimrServerForTestNodeRegistration server({
.EnforceUserToken = false, // still allow not secure way
.EnforceCheckUserToken = true, // when attempt to register with cert arrives, check it as if EnforceUserToken was switched on
.EnableDynamicNodeAuth = true,
.SetNodeAuthValues = true,
.RegisterNodeAllowedSids = {"DefaultClientAuth@cert"}
});
ui16 grpc = server.GetPort();
TString location = TStringBuilder() << "localhost:" << grpc;

SetLogPriority(server);

TDriverConfig secureConnectionConfig;
secureConnectionConfig.UseSecureConnection(caCert.Certificate.c_str())
.UseClientCertificate(clientServerCert.Certificate.c_str(),clientServerCert.PrivateKey.c_str())
.SetEndpoint(location);

TDriverConfig insecureConnectionConfig;
insecureConnectionConfig.UseSecureConnection(caCert.Certificate.c_str())
.SetEndpoint(location);

TDriverConfig enemyConnectionConfig;
enemyConnectionConfig.UseSecureConnection(caCert.Certificate.c_str())
.UseClientCertificate(clientServerEnemyCert.Certificate.c_str(),clientServerEnemyCert.PrivateKey.c_str())
.SetEndpoint(location);

CheckGood(RegisterNode(secureConnectionConfig));
CheckGood(RegisterNode(insecureConnectionConfig)); // without token and cert // EnforceUserToken = false
CheckAccessDenied(RegisterNode(insecureConnectionConfig.SetAuthToken("invalid token")), "Unknown token");
CheckAccessDeniedRegisterNode(RegisterNode(enemyConnectionConfig), "Client certificate failed verification");
}

}

namespace {
Expand Down
128 changes: 71 additions & 57 deletions ydb/services/ydb/ydb_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,38 @@ Y_UNIT_TEST_SUITE(TGRpcClientLowTest) {
UNIT_ASSERT(allDoneOk);
}

std::pair<Ydb::StatusIds::StatusCode, grpc::StatusCode> MakeTestRequest(NGRpcProxy::TGRpcClientConfig& clientConfig, const TString& database, const TString& token) {
NYdbGrpc::TCallMeta meta;
if (token) { // empty token => no token
meta.Aux.push_back({YDB_AUTH_TICKET_HEADER, token});
}
meta.Aux.push_back({YDB_DATABASE_HEADER, database});

NYdbGrpc::TGRpcClientLow clientLow;
auto connection = clientLow.CreateGRpcServiceConnection<Ydb::Table::V1::TableService>(clientConfig);

Ydb::StatusIds::StatusCode status;
grpc::StatusCode gStatus;

do {
auto promise = NThreading::NewPromise<void>();
Ydb::Table::CreateSessionRequest request;
NYdbGrpc::TResponseCallback<Ydb::Table::CreateSessionResponse> responseCb =
[&status, &gStatus, promise](NYdbGrpc::TGrpcStatus&& grpcStatus, Ydb::Table::CreateSessionResponse&& response) mutable {
UNIT_ASSERT(!grpcStatus.InternalError);
gStatus = grpc::StatusCode(grpcStatus.GRpcStatusCode);
auto deferred = response.operation();
status = deferred.status();
promise.SetValue();
};

connection->DoRequest(request, std::move(responseCb), &Ydb::Table::V1::TableService::Stub::AsyncCreateSession, meta);
promise.GetFuture().Wait();
} while (status == Ydb::StatusIds::UNAVAILABLE);
Cerr << "TestRequest(database=\"" << database << "\", token=\"" << token << "\") => {" << Ydb::StatusIds::StatusCode_Name(status) << ", " << int(gStatus) << "}" << Endl;
return std::make_pair(status, gStatus);
}

Y_UNIT_TEST(GrpcRequestProxy) {
NKikimrConfig::TAppConfig appConfig;
appConfig.MutableDomainsConfig()->MutableSecurityConfig()->SetEnforceUserTokenRequirement(true);
Expand All @@ -197,79 +229,61 @@ Y_UNIT_TEST_SUITE(TGRpcClientLowTest) {
ui16 grpc = server.GetPort();
TString location = TStringBuilder() << "localhost:" << grpc;
auto clientConfig = NGRpcProxy::TGRpcClientConfig(location);
auto doTest = [&](const TString& database) {
NYdbGrpc::TCallMeta meta;
meta.Aux.push_back({YDB_AUTH_TICKET_HEADER, "root@builtin"});
meta.Aux.push_back({YDB_DATABASE_HEADER, database});

NYdbGrpc::TGRpcClientLow clientLow;
auto connection = clientLow.CreateGRpcServiceConnection<Ydb::Table::V1::TableService>(clientConfig);

Ydb::StatusIds::StatusCode status;
int gStatus;
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/Root", "root@builtin"), std::make_pair(Ydb::StatusIds::SUCCESS, grpc::StatusCode::OK));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/blabla", "root@builtin"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "blabla", "root@builtin"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
}

do {
auto promise = NThreading::NewPromise<void>();
Ydb::Table::CreateSessionRequest request;
NYdbGrpc::TResponseCallback<Ydb::Table::CreateSessionResponse> responseCb =
[&status, &gStatus, promise](NYdbGrpc::TGrpcStatus&& grpcStatus, Ydb::Table::CreateSessionResponse&& response) mutable {
UNIT_ASSERT(!grpcStatus.InternalError);
gStatus = grpcStatus.GRpcStatusCode;
auto deferred = response.operation();
status = deferred.status();
promise.SetValue();
};
Y_UNIT_TEST(GrpcRequestProxyWithoutToken) {
NKikimrConfig::TAppConfig appConfig;
appConfig.MutableDomainsConfig()->MutableSecurityConfig()->SetEnforceUserTokenRequirement(true);
TKikimrWithGrpcAndRootSchemaWithAuth server(appConfig);

connection->DoRequest(request, std::move(responseCb), &Ydb::Table::V1::TableService::Stub::AsyncCreateSession, meta);
promise.GetFuture().Wait();
} while (status == Ydb::StatusIds::UNAVAILABLE);
return std::make_pair(status, gStatus);
};
ui16 grpc = server.GetPort();
TString location = TStringBuilder() << "localhost:" << grpc;
auto clientConfig = NGRpcProxy::TGRpcClientConfig(location);

UNIT_ASSERT_VALUES_EQUAL(doTest("/Root"), std::make_pair(Ydb::StatusIds::SUCCESS, 0));
UNIT_ASSERT_VALUES_EQUAL(doTest("/blabla"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, 16));
UNIT_ASSERT_VALUES_EQUAL(doTest("blabla"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, 16));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/Root", ""), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/blabla", ""), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "blabla", ""), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
}

Y_UNIT_TEST(GrpcRequestProxyWithoutToken) {
void GrpcRequestProxyCheckTokenWhenItIsSpecified(bool enforceUserTokenCheckRequirement) {
NKikimrConfig::TAppConfig appConfig;
appConfig.MutableDomainsConfig()->MutableSecurityConfig()->SetEnforceUserTokenRequirement(true);
appConfig.MutableDomainsConfig()->MutableSecurityConfig()->SetEnforceUserTokenRequirement(false);
appConfig.MutableDomainsConfig()->MutableSecurityConfig()->SetEnforceUserTokenCheckRequirement(enforceUserTokenCheckRequirement);
TKikimrWithGrpcAndRootSchemaWithAuth server(appConfig);

ui16 grpc = server.GetPort();
TString location = TStringBuilder() << "localhost:" << grpc;
auto clientConfig = NGRpcProxy::TGRpcClientConfig(location);
auto doTest = [&](const TString& database) {
NYdbGrpc::TCallMeta meta;
meta.Aux.push_back({YDB_DATABASE_HEADER, database});

NYdbGrpc::TGRpcClientLow clientLow;
auto connection = clientLow.CreateGRpcServiceConnection<Ydb::Table::V1::TableService>(clientConfig);
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/Root", ""), std::make_pair(Ydb::StatusIds::SUCCESS, grpc::StatusCode::OK));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/blabla", ""), std::make_pair(Ydb::StatusIds::SUCCESS, grpc::StatusCode::OK));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "blabla", ""), std::make_pair(Ydb::StatusIds::SUCCESS, grpc::StatusCode::OK));

Ydb::StatusIds::StatusCode status;
grpc::StatusCode gStatus;
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/Root", "root@builtin"), std::make_pair(Ydb::StatusIds::SUCCESS, grpc::StatusCode::OK));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/blabla", "root@builtin"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "blabla", "root@builtin"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));

do {
auto promise = NThreading::NewPromise<void>();
Ydb::Table::CreateSessionRequest request;
NYdbGrpc::TResponseCallback<Ydb::Table::CreateSessionResponse> responseCb =
[&status, &gStatus, promise](NYdbGrpc::TGrpcStatus&& grpcStatus, Ydb::Table::CreateSessionResponse&& response) mutable {
UNIT_ASSERT(!grpcStatus.InternalError);
gStatus = grpc::StatusCode(grpcStatus.GRpcStatusCode);
auto deferred = response.operation();
status = deferred.status();
promise.SetValue();
};
const auto reqResultWithInvalidToken = MakeTestRequest(clientConfig, "/Root", "invalid token");
if (enforceUserTokenCheckRequirement) {
UNIT_ASSERT_EQUAL(reqResultWithInvalidToken, std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
} else {
UNIT_ASSERT_EQUAL(reqResultWithInvalidToken, std::make_pair(Ydb::StatusIds::SUCCESS, grpc::StatusCode::OK));
}

connection->DoRequest(request, std::move(responseCb), &Ydb::Table::V1::TableService::Stub::AsyncCreateSession, meta);
promise.GetFuture().Wait();
} while (status == Ydb::StatusIds::UNAVAILABLE);
return std::make_pair(status, gStatus);
};
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "/blabla", "invalid token"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
UNIT_ASSERT_EQUAL(MakeTestRequest(clientConfig, "blabla", "invalid token"), std::make_pair(Ydb::StatusIds::STATUS_CODE_UNSPECIFIED, grpc::StatusCode::UNAUTHENTICATED));
}

UNIT_ASSERT_EQUAL(doTest("/Root").second, grpc::StatusCode::UNAUTHENTICATED);
UNIT_ASSERT_EQUAL(doTest("/blabla").second, grpc::StatusCode::UNAUTHENTICATED);
UNIT_ASSERT_EQUAL(doTest("blabla").second, grpc::StatusCode::UNAUTHENTICATED);
Y_UNIT_TEST(GrpcRequestProxyCheckTokenWhenItIsSpecified_Ignore) {
GrpcRequestProxyCheckTokenWhenItIsSpecified(false);
}

Y_UNIT_TEST(GrpcRequestProxyCheckTokenWhenItIsSpecified_Check) {
GrpcRequestProxyCheckTokenWhenItIsSpecified(true);
}

Y_UNIT_TEST(BiStreamPing) {
Expand Down Expand Up @@ -5614,7 +5628,7 @@ Y_UNIT_TEST(DisableWritesToDatabase) {

TTenants tenants(server);
tenants.Run(tenantPath, 1);

TString table = Sprintf("%s/table", tenantPath.c_str());
ExecSQL(server, sender, Sprintf(R"(
CREATE TABLE `%s` (
Expand Down

0 comments on commit 842aebb

Please sign in to comment.