From f37db1da1a4d26d23acff38093fa3134ab756527 Mon Sep 17 00:00:00 2001 From: sharpeye Date: Thu, 28 Nov 2024 15:42:47 +0000 Subject: [PATCH 01/13] restore style --- cloud/blockstore/libs/daemon/ydb/config_initializer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloud/blockstore/libs/daemon/ydb/config_initializer.cpp b/cloud/blockstore/libs/daemon/ydb/config_initializer.cpp index fbeae8efd12..f6c0c40980b 100644 --- a/cloud/blockstore/libs/daemon/ydb/config_initializer.cpp +++ b/cloud/blockstore/libs/daemon/ydb/config_initializer.cpp @@ -37,7 +37,7 @@ using namespace NCloud::NBlockStore::NDiscovery; //////////////////////////////////////////////////////////////////////////////// TConfigInitializerYdb::TConfigInitializerYdb(TOptionsYdbPtr options) - : TConfigInitializerCommon(options) + : TConfigInitializerCommon(options) , NCloud::NStorage::TConfigInitializerYdbBase(options) , Options(options) {} From 784413fa90e0e456cd3ee5fce37fbd442939c1a5 Mon Sep 17 00:00:00 2001 From: sharpeye Date: Thu, 28 Nov 2024 20:15:12 +0000 Subject: [PATCH 02/13] defer log creation --- cloud/blockstore/libs/root_kms/impl/client.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cloud/blockstore/libs/root_kms/impl/client.cpp b/cloud/blockstore/libs/root_kms/impl/client.cpp index a4a914eeaa4..df728c59611 100644 --- a/cloud/blockstore/libs/root_kms/impl/client.cpp +++ b/cloud/blockstore/libs/root_kms/impl/client.cpp @@ -214,9 +214,7 @@ TRootKmsClient::TRootKmsClient( TCreateRootKmsClientParams params) : Logging(std::move(logging)) , Params(std::move(params)) - , Log(Logging->CreateLog("ROOT_KMS_CLIENT")) -{ -} +{} TRootKmsClient::~TRootKmsClient() { @@ -225,6 +223,8 @@ TRootKmsClient::~TRootKmsClient() void TRootKmsClient::Start() { + Log = Logging->CreateLog("ROOT_KMS_CLIENT"); + grpc::SslCredentialsOptions sslOpts{ .pem_root_certs = ReadFile(Params.RootCertsFile), .pem_private_key = ReadFile(Params.PrivateKeyFile), From c5b78fd3d4ac37de02aaa6796df421be52193c5d Mon Sep 17 00:00:00 2001 From: sharpeye Date: Thu, 28 Nov 2024 20:16:22 +0000 Subject: [PATCH 03/13] setup encryption_at_rest test --- .../tests/encryption_at_rest/test.py | 160 ++++++++++++++++++ .../tests/encryption_at_rest/ya.make | 20 +++ cloud/blockstore/tests/ya.make | 1 + 3 files changed, 181 insertions(+) create mode 100644 cloud/blockstore/tests/encryption_at_rest/test.py create mode 100644 cloud/blockstore/tests/encryption_at_rest/ya.make diff --git a/cloud/blockstore/tests/encryption_at_rest/test.py b/cloud/blockstore/tests/encryption_at_rest/test.py new file mode 100644 index 00000000000..400f2dd627c --- /dev/null +++ b/cloud/blockstore/tests/encryption_at_rest/test.py @@ -0,0 +1,160 @@ +import json +import os +import pytest +import time + +import yatest.common as yatest_common +import cloud.blockstore.tests.python.lib.daemon as daemon + +from cloud.blockstore.public.sdk.python.client import CreateClient +from cloud.blockstore.public.sdk.python.protos import TCmsActionRequest, \ + TAction, STORAGE_MEDIA_SSD_NONREPLICATED +from cloud.blockstore.config.root_kms_pb2 import TRootKmsConfig +from cloud.blockstore.tests.python.lib.config import NbsConfigurator, \ + generate_disk_agent_txt + +from contrib.ydb.tests.library.harness.kikimr_runner import \ + get_unique_path_for_current_test, ensure_path_exists + + +DEVICE_SIZE = 1024**3 # 1 GiB +DEVICE_COUNT = 6 +DEVICE_PADDING = 4096 +DEVICE_HEADER = 4096 + + +@pytest.fixture(name='agent_id') +def get_agent_id(): + return daemon.get_fqdn() + + +@pytest.fixture(name='data_path') +def create_data_path(): + + p = get_unique_path_for_current_test( + output_path=yatest_common.output_path(), + sub_folder="data") + + p = os.path.join(p, "dev", "disk", "by-partlabel") + ensure_path_exists(p) + + return p + + +@pytest.fixture(autouse=True) +def create_device_storage(data_path): + with open(os.path.join(data_path, 'NVMENBS01'), 'wb') as f: + os.truncate(f.fileno(), DEVICE_HEADER + DEVICE_SIZE * DEVICE_COUNT + (DEVICE_COUNT - 1) * DEVICE_PADDING) + + +@pytest.fixture(name='ydb') +def start_ydb_cluster(): + + ydb_cluster = daemon.start_ydb() + + yield ydb_cluster + + ydb_cluster.stop() + + +@pytest.fixture(name='nbs') +def start_nbs_daemon(ydb): + + nbs_configurator = NbsConfigurator(ydb) + nbs_configurator.generate_default_nbs_configs() + nbs_configurator.files["storage"].AllocationUnitNonReplicatedSSD = 1 # 1 GiB + + root_kms = TRootKmsConfig() + root_kms.Address = f'localhost:{os.environ.get("FAKE_ROOT_KMS_PORT")}' + root_kms.KeyId = 'nbs' + root_kms.RootCertsFile = os.environ.get("FAKE_ROOT_KMS_CA") + root_kms.CertChainFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_CRT") + root_kms.PrivateKeyFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_KEY") + + nbs_configurator.files['root-kms'] = root_kms + + nbs = daemon.start_nbs(nbs_configurator) + + client = CreateClient(f"localhost:{nbs.port}") + client.execute_action( + action="DiskRegistrySetWritableState", + input_bytes=str.encode('{"State": true}')) + client.update_disk_registry_config({ + "KnownDevicePools": + [{"Kind": "DEVICE_POOL_KIND_DEFAULT", "AllocationUnit": DEVICE_SIZE}] + }) + + yield nbs + + nbs.kill() + + +def _add_host(client, agent_id): + request = TCmsActionRequest() + action = request.Actions.add() + action.Type = TAction.ADD_HOST + action.Host = agent_id + + return client.cms_action(request) + + +def _wait_for_devices_to_be_cleared(client, expected_dirty_count=0): + while True: + response = client.execute_action( + action="BackupDiskRegistryState", + input_bytes=str.encode('{"BackupLocalDB": true}')) + bkp = json.loads(response)["Backup"] + agents = bkp.get("Agents", []) + dirty_devices = bkp.get("DirtyDevices", []) + dirty_count = len(dirty_devices) + if len(agents) != 0 and dirty_count == expected_dirty_count: + break + time.sleep(1) + + +@pytest.fixture(name='disk_agent') +def start_disk_agent(ydb, nbs, agent_id, data_path): + + configurator = NbsConfigurator(ydb, 'disk-agent') + configurator.generate_default_nbs_configs() + configurator.files["disk-agent"] = generate_disk_agent_txt( + agent_id='', + device_erase_method='DEVICE_ERASE_METHOD_NONE', # speed up tests + storage_discovery_config={ + "PathConfigs": [{ + "PathRegExp": f"{data_path}/NVMENBS([0-9]+)", + "PoolConfigs": [{ + "Layout": { + "DeviceSize": DEVICE_SIZE, + "DevicePadding": DEVICE_PADDING, + "HeaderSize": DEVICE_HEADER + } + }]} + ]}) + + disk_agent = daemon.start_disk_agent(configurator) + disk_agent.wait_for_registration() + + client = CreateClient(f"localhost:{nbs.port}") + + _add_host(client, agent_id) + _wait_for_devices_to_be_cleared(client) + + yield disk_agent + + disk_agent.stop() + + +def test_create_encrypted_volume(nbs, disk_agent): + + client = CreateClient(f"localhost:{nbs.port}") + + client.create_volume( + disk_id="vol0", + block_size=4096, + blocks_count=2*DEVICE_SIZE//4096, + storage_media_kind=STORAGE_MEDIA_SSD_NONREPLICATED) + + vol0 = client.stat_volume("vol0")["Volume"] + + assert len(vol0.Devices) == 2 diff --git a/cloud/blockstore/tests/encryption_at_rest/ya.make b/cloud/blockstore/tests/encryption_at_rest/ya.make new file mode 100644 index 00000000000..14925aaeace --- /dev/null +++ b/cloud/blockstore/tests/encryption_at_rest/ya.make @@ -0,0 +1,20 @@ +PY3TEST() + +INCLUDE(${ARCADIA_ROOT}/cloud/storage/core/tests/recipes/medium.inc) + +TEST_SRCS(test.py) + +DEPENDS( + cloud/blockstore/apps/client + cloud/blockstore/apps/disk_agent + cloud/blockstore/apps/server + contrib/ydb/apps/ydbd +) + +PEERDIR( + cloud/blockstore/tests/python/lib +) + +INCLUDE(${ARCADIA_ROOT}/cloud/blockstore/tests/recipes/fake-root-kms/recipe.inc) + +END() diff --git a/cloud/blockstore/tests/ya.make b/cloud/blockstore/tests/ya.make index f268e0c80eb..1c342760325 100644 --- a/cloud/blockstore/tests/ya.make +++ b/cloud/blockstore/tests/ya.make @@ -10,6 +10,7 @@ RECURSE( config_dispatcher csi_driver disk_agent_config + encryption_at_rest external_endpoint e2e-tests fio From d5fd99f49c5c74f72fa376ca26c57f35a6c1769d Mon Sep 17 00:00:00 2001 From: sharpeye Date: Thu, 28 Nov 2024 21:24:52 +0000 Subject: [PATCH 04/13] issue-176: encryption at rest for Disk Registry based disks --- cloud/blockstore/config/storage.proto | 3 + .../blockstore/libs/daemon/ydb/bootstrap.cpp | 17 +-- cloud/blockstore/libs/storage/core/config.cpp | 3 + cloud/blockstore/libs/storage/core/config.h | 7 + .../libs/storage/init/server/actorsystem.cpp | 3 +- .../libs/storage/init/server/actorsystem.h | 2 + .../libs/storage/service/service.cpp | 6 +- .../blockstore/libs/storage/service/service.h | 4 +- .../libs/storage/service/service_actor.cpp | 7 +- .../libs/storage/service/service_actor.h | 7 +- .../storage/service/service_actor_create.cpp | 123 ++++++++++++++++-- .../storage/service/service_events_private.h | 14 ++ .../libs/storage/testlib/test_env.cpp | 4 +- .../tests/encryption_at_rest/test.py | 37 ++++-- 14 files changed, 200 insertions(+), 37 deletions(-) diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto index e42e5df9736..e91459547b4 100644 --- a/cloud/blockstore/config/storage.proto +++ b/cloud/blockstore/config/storage.proto @@ -1068,4 +1068,7 @@ message TStorageServiceConfig // When enabled, the Disk Registry REMOVE_HOST CMS action will "forget" // agents devices and suspend local devices. optional bool DiskRegistryCleanupConfigOnRemoveHost = 392; + + // Enables the encryption at rest for Disk Registry based disks. + optional bool EncryptionAtRestForDiskRegistryBasedDisksEnabled = 393; } diff --git a/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp b/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp index 13097e62557..fcc86f73ae6 100644 --- a/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp +++ b/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp @@ -561,18 +561,19 @@ void TBootstrapYdb::InitKikimrService() args.NvmeManager = NvmeManager; args.UserCounterProviders = {VolumeStats->GetUserCounters()}; args.IsDiskRegistrySpareNode = [&] { - if (!Configs->StorageConfig->GetDisableLocalService()) { - return false; - } + if (!Configs->StorageConfig->GetDisableLocalService()) { + return false; + } - const auto& nodes = Configs->StorageConfig->GetKnownSpareNodes(); - const auto& fqdn = FQDNHostName(); - const ui32 p = Configs->StorageConfig->GetSpareNodeProbability(); + const auto& nodes = Configs->StorageConfig->GetKnownSpareNodes(); + const auto& fqdn = FQDNHostName(); + const ui32 p = Configs->StorageConfig->GetSpareNodeProbability(); - return FindPtr(nodes, fqdn) || CityHash64(fqdn) % 100 < p; - }(); + return FindPtr(nodes, fqdn) || CityHash64(fqdn) % 100 < p; + }(); args.VolumeBalancerSwitch = VolumeBalancerSwitch; args.EndpointEventHandler = EndpointEventHandler; + args.RootKmsKeyProvider = RootKmsKeyProvider; ActorSystem = NStorage::CreateActorSystem(args); diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp index f39aa9a5e49..543d1b4350c 100644 --- a/cloud/blockstore/libs/storage/core/config.cpp +++ b/cloud/blockstore/libs/storage/core/config.cpp @@ -516,6 +516,8 @@ TDuration MSeconds(ui32 value) xxx(DiskRegistryDisksNotificationTimeout, TDuration, Seconds(5) )\ xxx(BlobStorageAsyncGetTimeoutHDD, TDuration, Seconds(0) )\ xxx(BlobStorageAsyncGetTimeoutSSD, TDuration, Seconds(0) )\ + \ + xxx(EncryptionAtRestForDiskRegistryBasedDisksEnabled, bool, false ) \ // BLOCKSTORE_STORAGE_CONFIG_RW @@ -546,6 +548,7 @@ BLOCKSTORE_STORAGE_CONFIG(BLOCKSTORE_STORAGE_DECLARE_CONFIG) xxx(ReplaceDevice) \ xxx(UseNonReplicatedHDDInsteadOfReplicated) \ xxx(AddingUnconfirmedBlobs) \ + xxx(EncryptionAtRestForDiskRegistryBasedDisks) \ // BLOCKSTORE_BINARY_FEATURES diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h index 4df3cbaa4eb..96bbca08a83 100644 --- a/cloud/blockstore/libs/storage/core/config.h +++ b/cloud/blockstore/libs/storage/core/config.h @@ -361,6 +361,11 @@ class TStorageConfig const TString& folderId, const TString& diskId) const; + [[nodiscard]] bool IsEncryptionAtRestForDiskRegistryBasedDisksFeatureEnabled( + const TString& cloudId, + const TString& folderId, + const TString& diskId) const; + TDuration GetMaxTimedOutDeviceStateDurationFeatureValue( const TString& cloudId, const TString& folderId, @@ -613,6 +618,8 @@ class TStorageConfig TDuration GetBlobStorageAsyncGetTimeoutHDD() const; TDuration GetBlobStorageAsyncGetTimeoutSSD() const; + + [[nodiscard]] bool GetEncryptionAtRestForDiskRegistryBasedDisksEnabled() const; }; ui64 GetAllocationUnit( diff --git a/cloud/blockstore/libs/storage/init/server/actorsystem.cpp b/cloud/blockstore/libs/storage/init/server/actorsystem.cpp index 16b1f9cb488..3b4adabaf95 100644 --- a/cloud/blockstore/libs/storage/init/server/actorsystem.cpp +++ b/cloud/blockstore/libs/storage/init/server/actorsystem.cpp @@ -216,7 +216,8 @@ class TStorageServicesInitializer final Args.EndpointEventHandler, Args.RdmaClient, Args.VolumeStats, - Args.PreemptedVolumes); + Args.PreemptedVolumes, + Args.RootKmsKeyProvider); setup->LocalServices.emplace_back( MakeStorageServiceId(), diff --git a/cloud/blockstore/libs/storage/init/server/actorsystem.h b/cloud/blockstore/libs/storage/init/server/actorsystem.h index 2b215f3c2be..2c30e6edf1e 100644 --- a/cloud/blockstore/libs/storage/init/server/actorsystem.h +++ b/cloud/blockstore/libs/storage/init/server/actorsystem.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -65,6 +66,7 @@ struct TServerActorSystemArgs NNvme::INvmeManagerPtr NvmeManager; IVolumeBalancerSwitchPtr VolumeBalancerSwitch; NServer::IEndpointEventHandlerPtr EndpointEventHandler; + IRootKmsKeyProviderPtr RootKmsKeyProvider; TVector UserCounterProviders; diff --git a/cloud/blockstore/libs/storage/service/service.cpp b/cloud/blockstore/libs/storage/service/service.cpp index ec5a091a0d8..f52a9d42134 100644 --- a/cloud/blockstore/libs/storage/service/service.cpp +++ b/cloud/blockstore/libs/storage/service/service.cpp @@ -18,7 +18,8 @@ IActorPtr CreateStorageService( NServer::IEndpointEventHandlerPtr endpointEventHandler, NRdma::IClientPtr rdmaClient, IVolumeStatsPtr volumeStats, - TManuallyPreemptedVolumesPtr preemptedVolumes) + TManuallyPreemptedVolumesPtr preemptedVolumes, + IRootKmsKeyProviderPtr rootKmsKeyProvider) { return std::make_unique( std::move(config), @@ -30,7 +31,8 @@ IActorPtr CreateStorageService( std::move(endpointEventHandler), std::move(rdmaClient), std::move(volumeStats), - std::move(preemptedVolumes)); + std::move(preemptedVolumes), + std::move(rootKmsKeyProvider)); } } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/service/service.h b/cloud/blockstore/libs/storage/service/service.h index 41fec562de9..4f62a978bcf 100644 --- a/cloud/blockstore/libs/storage/service/service.h +++ b/cloud/blockstore/libs/storage/service/service.h @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -23,6 +24,7 @@ NActors::IActorPtr CreateStorageService( NServer::IEndpointEventHandlerPtr endpointEventHandler, NRdma::IClientPtr rdmaClient, IVolumeStatsPtr volumeStats, - TManuallyPreemptedVolumesPtr preemptedVolumes); + TManuallyPreemptedVolumesPtr preemptedVolumes, + IRootKmsKeyProviderPtr rootKmsKeyProvider); } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/service/service_actor.cpp b/cloud/blockstore/libs/storage/service/service_actor.cpp index 1806a040ee0..cabd3520dda 100644 --- a/cloud/blockstore/libs/storage/service/service_actor.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor.cpp @@ -27,7 +27,8 @@ TServiceActor::TServiceActor( NServer::IEndpointEventHandlerPtr endpointEventHandler, NRdma::IClientPtr rdmaClient, IVolumeStatsPtr volumeStats, - TManuallyPreemptedVolumesPtr preemptedVolumes) + TManuallyPreemptedVolumesPtr preemptedVolumes, + IRootKmsKeyProviderPtr rootKmsKeyProvider) : Config(std::move(config)) , DiagnosticsConfig(std::move(diagnosticsConfig)) , ProfileLog(std::move(profileLog)) @@ -37,12 +38,12 @@ TServiceActor::TServiceActor( , EndpointEventHandler(std::move(endpointEventHandler)) , RdmaClient(std::move(rdmaClient)) , VolumeStats(std::move(volumeStats)) + , RootKmsKeyProvider(std::move(rootKmsKeyProvider)) , SharedCounters(MakeIntrusive(Config)) , State(std::move(preemptedVolumes)) {} -TServiceActor::~TServiceActor() -{} +TServiceActor::~TServiceActor() = default; void TServiceActor::Bootstrap(const TActorContext& ctx) { diff --git a/cloud/blockstore/libs/storage/service/service_actor.h b/cloud/blockstore/libs/storage/service/service_actor.h index 1db6c291033..34cbf7e1f3a 100644 --- a/cloud/blockstore/libs/storage/service/service_actor.h +++ b/cloud/blockstore/libs/storage/service/service_actor.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ class TServiceActor final const NServer::IEndpointEventHandlerPtr EndpointEventHandler; const NRdma::IClientPtr RdmaClient; const IVolumeStatsPtr VolumeStats; + const IRootKmsKeyProviderPtr RootKmsKeyProvider; TSharedServiceCountersPtr SharedCounters; @@ -73,8 +75,9 @@ class TServiceActor final NServer::IEndpointEventHandlerPtr endpointEventHandler, NRdma::IClientPtr rdmaClient, IVolumeStatsPtr volumeStats, - TManuallyPreemptedVolumesPtr preemptedVolumes); - ~TServiceActor(); + TManuallyPreemptedVolumesPtr preemptedVolumes, + IRootKmsKeyProviderPtr rootKmsKeyProvider); + ~TServiceActor() override; void Bootstrap(const NActors::TActorContext& ctx); diff --git a/cloud/blockstore/libs/storage/service/service_actor_create.cpp b/cloud/blockstore/libs/storage/service/service_actor_create.cpp index 81225ff070d..e313eae21c0 100644 --- a/cloud/blockstore/libs/storage/service/service_actor_create.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor_create.cpp @@ -18,6 +18,8 @@ #include #include +#include + namespace NCloud::NBlockStore::NStorage { using namespace NActors; @@ -34,6 +36,7 @@ class TCreateVolumeActor final const TStorageConfigPtr Config; const NProto::TCreateVolumeRequest Request; + const IRootKmsKeyProviderPtr KeyProvider; ui64 BaseDiskTabletId = 0; @@ -41,16 +44,25 @@ class TCreateVolumeActor final TCreateVolumeActor( TRequestInfoPtr requestInfo, TStorageConfigPtr config, - NProto::TCreateVolumeRequest request); + NProto::TCreateVolumeRequest request, + IRootKmsKeyProviderPtr keyProvider); void Bootstrap(const TActorContext& ctx); private: ui32 GetBlockSize() const; NCloud::NProto::EStorageMediaKind GetStorageMediaKind() const; + bool ShouldCreateVolumeWithEncryptionAtRest() const; void DescribeBaseVolume(const TActorContext& ctx); void CreateVolume(const TActorContext& ctx); + void HandleCreateEncryptionKeyResponse( + const TEvServicePrivate::TEvCreateEncryptionKeyResponse::TPtr& ev, + const TActorContext& ctx); + + void CreateVolumeImpl( + const TActorContext& ctx, + std::optional encryptionDesc); void HandleDescribeVolumeResponse( const TEvSSProxy::TEvDescribeVolumeResponse::TPtr& ev, @@ -77,10 +89,12 @@ class TCreateVolumeActor final TCreateVolumeActor::TCreateVolumeActor( TRequestInfoPtr requestInfo, TStorageConfigPtr config, - NProto::TCreateVolumeRequest request) + NProto::TCreateVolumeRequest request, + IRootKmsKeyProviderPtr keyProvider) : RequestInfo(std::move(requestInfo)) , Config(std::move(config)) , Request(std::move(request)) + , KeyProvider(std::move(keyProvider)) {} void TCreateVolumeActor::Bootstrap(const TActorContext& ctx) @@ -123,7 +137,64 @@ void TCreateVolumeActor::DescribeBaseVolume(const TActorContext& ctx) } } +bool TCreateVolumeActor::ShouldCreateVolumeWithEncryptionAtRest() const +{ + if (GetStorageMediaKind() == NProto::STORAGE_MEDIA_SSD_LOCAL) { + // Encryption at rest for local disks is not implemented. TODO: issue + return false; + } + + return IsDiskRegistryMediaKind(GetStorageMediaKind()) && + // Client did not request encryption with the provided key + Request.GetEncryptionSpec().GetMode() == NProto::NO_ENCRYPTION && + (Config->GetEncryptionAtRestForDiskRegistryBasedDisksEnabled() || + Config->IsEncryptionAtRestForDiskRegistryBasedDisksFeatureEnabled( + Request.GetCloudId(), + Request.GetFolderId(), + Request.GetDiskId())); +} + void TCreateVolumeActor::CreateVolume(const TActorContext& ctx) +{ + if (ShouldCreateVolumeWithEncryptionAtRest()) { + LOG_INFO_S( + ctx, + TBlockStoreComponents::SERVICE, + "Generate DEK for " << Request.GetDiskId().Quote()); + + auto* actorSystem = TActivationContext::ActorSystem(); + auto selfId = ctx.SelfID; + + KeyProvider->GenerateDataEncryptionKey(Request.GetDiskId()) + .Subscribe( + [=](const auto& future) { + auto [key, error] = future.GetValue(); + + auto response = std::make_unique< + TEvServicePrivate::TEvCreateEncryptionKeyResponse>( + std::move(error), + std::move(key)); + + actorSystem->Send(selfId, response.release()); + }); + return; + } + + std::optional encryptionDesc; + + const auto& encryptionSpec = Request.GetEncryptionSpec(); + if (encryptionSpec.GetMode() != NProto::NO_ENCRYPTION) { + auto& desc = encryptionDesc.emplace(); + desc.SetMode(encryptionSpec.GetMode()); + desc.SetKeyHash(encryptionSpec.GetKeyHash()); + } + + CreateVolumeImpl(ctx, std::move(encryptionDesc)); +} + +void TCreateVolumeActor::CreateVolumeImpl( + const TActorContext& ctx, + std::optional encryptionDesc) { NKikimrBlockStore::TVolumeConfig config; @@ -215,11 +286,14 @@ void TCreateVolumeActor::CreateVolume(const TActorContext& ctx) } config.MutableAgentIds()->CopyFrom(Request.GetAgentIds()); - const auto& encryptionSpec = Request.GetEncryptionSpec(); - if (encryptionSpec.GetMode() != NProto::NO_ENCRYPTION) { - auto& desc = *config.MutableEncryptionDesc(); - desc.SetMode(encryptionSpec.GetMode()); - desc.SetKeyHash(encryptionSpec.GetKeyHash()); + if (encryptionDesc) { + LOG_DEBUG_S( + ctx, + TBlockStoreComponents::SERVICE, + "Creating volume with an encryption: " + << NProto::EEncryptionMode_Name(encryptionDesc->GetMode())); + + *config.MutableEncryptionDesc() = std::move(*encryptionDesc); } auto request = std::make_unique( @@ -236,6 +310,35 @@ void TCreateVolumeActor::CreateVolume(const TActorContext& ctx) RequestInfo->Cookie); } +void TCreateVolumeActor::HandleCreateEncryptionKeyResponse( + const TEvServicePrivate::TEvCreateEncryptionKeyResponse::TPtr& ev, + const TActorContext& ctx) +{ + const auto& msg = *ev->Get(); + + std::optional encryptionDesc; + + if (const auto& error = msg.GetError(); HasError(error)) { + LOG_ERROR_S( + ctx, + TBlockStoreComponents::SERVICE, + "Failed to generate encryption key: " + << FormatError(error) << ". Create an unencrypted volume."); + } else { + LOG_INFO_S( + ctx, + TBlockStoreComponents::SERVICE, + "Create volume " << Request.GetDiskId().Quote() + << " with default AES XTS encryption"); + + auto& desc = encryptionDesc.emplace(); + desc.SetMode(NProto::ENCRYPTION_DEFAULT_AES_XTS); + // desc.MutableEncryptionKey()->CopyFrom(msg.KmsKey); + } + + CreateVolumeImpl(ctx, std::move(encryptionDesc)); +} + void TCreateVolumeActor::HandleDescribeVolumeResponse( const TEvSSProxy::TEvDescribeVolumeResponse::TPtr& ev, const TActorContext& ctx) @@ -344,6 +447,9 @@ STFUNC(TCreateVolumeActor::StateWork) HFunc( TEvSSProxy::TEvDescribeVolumeResponse, HandleDescribeVolumeResponse); + HFunc( + TEvServicePrivate::TEvCreateEncryptionKeyResponse, + HandleCreateEncryptionKeyResponse); default: HandleUnexpectedEvent(ev, TBlockStoreComponents::SERVICE); @@ -539,7 +645,8 @@ void TServiceActor::HandleCreateVolume( ctx, std::move(requestInfo), Config, - std::move(request)); + std::move(request), + RootKmsKeyProvider); } } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/service/service_events_private.h b/cloud/blockstore/libs/storage/service/service_events_private.h index 951778b167c..6245a50e1bc 100644 --- a/cloud/blockstore/libs/storage/service/service_events_private.h +++ b/cloud/blockstore/libs/storage/service/service_events_private.h @@ -235,6 +235,15 @@ struct TEvServicePrivate NProto::TMountVolumeResponse Record; }; + // + // Create encryption key + // + + struct TCreateEncryptionKeyResponse + { + NProto::TKmsKey KmsKey; + }; + // // UpdateManuallyPreemptedVolume notification // @@ -298,6 +307,7 @@ struct TEvServicePrivate EvUpdateManuallyPreemptedVolume, EvSyncManuallyPreemptedVolumesComplete, EvSelfPing, + EvCreateEncryptionKeyResponse, EvEnd }; @@ -387,6 +397,10 @@ struct TEvServicePrivate >; using TEvSelfPing = TRequestEvent; + + using TEvCreateEncryptionKeyResponse = TResponseEvent< + TCreateEncryptionKeyResponse, + EvCreateEncryptionKeyResponse>; }; } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/testlib/test_env.cpp b/cloud/blockstore/libs/storage/testlib/test_env.cpp index c74402b04f3..32020824414 100644 --- a/cloud/blockstore/libs/storage/testlib/test_env.cpp +++ b/cloud/blockstore/libs/storage/testlib/test_env.cpp @@ -376,7 +376,9 @@ ui32 TTestEnv::CreateBlockStoreNode( NServer::CreateEndpointEventProxy(), nullptr, // rdmaClient CreateVolumeStatsStub(), - std::move(manuallyPreemptedVolumes)); + std::move(manuallyPreemptedVolumes), + CreateRootKmsKeyProviderStub()); + auto storageServiceId = Runtime.Register( storageService.release(), nodeIdx, diff --git a/cloud/blockstore/tests/encryption_at_rest/test.py b/cloud/blockstore/tests/encryption_at_rest/test.py index 400f2dd627c..595babe30b8 100644 --- a/cloud/blockstore/tests/encryption_at_rest/test.py +++ b/cloud/blockstore/tests/encryption_at_rest/test.py @@ -6,12 +6,16 @@ import yatest.common as yatest_common import cloud.blockstore.tests.python.lib.daemon as daemon +from cloud.blockstore.public.api.protos.encryption_pb2 import \ + ENCRYPTION_DEFAULT_AES_XTS + from cloud.blockstore.public.sdk.python.client import CreateClient from cloud.blockstore.public.sdk.python.protos import TCmsActionRequest, \ TAction, STORAGE_MEDIA_SSD_NONREPLICATED from cloud.blockstore.config.root_kms_pb2 import TRootKmsConfig from cloud.blockstore.tests.python.lib.config import NbsConfigurator, \ generate_disk_agent_txt +from cloud.storage.core.config.features_pb2 import TFeaturesConfig from contrib.ydb.tests.library.harness.kikimr_runner import \ get_unique_path_for_current_test, ensure_path_exists @@ -21,6 +25,7 @@ DEVICE_COUNT = 6 DEVICE_PADDING = 4096 DEVICE_HEADER = 4096 +KEK_ID = 'nbs' @pytest.fixture(name='agent_id') @@ -60,20 +65,27 @@ def start_ydb_cluster(): @pytest.fixture(name='nbs') def start_nbs_daemon(ydb): - nbs_configurator = NbsConfigurator(ydb) - nbs_configurator.generate_default_nbs_configs() - nbs_configurator.files["storage"].AllocationUnitNonReplicatedSSD = 1 # 1 GiB + cfg = NbsConfigurator(ydb) + cfg.generate_default_nbs_configs() + cfg.files['storage'].AllocationUnitNonReplicatedSSD = 1 # 1 GiB + + features = TFeaturesConfig() + feature = features.Features.add() + feature.Name = 'EncryptionAtRestForDiskRegistryBasedDisks' + feature.Whitelist.EntityIds.append("vol0") + + cfg.files['features'] = features root_kms = TRootKmsConfig() root_kms.Address = f'localhost:{os.environ.get("FAKE_ROOT_KMS_PORT")}' - root_kms.KeyId = 'nbs' + root_kms.KeyId = KEK_ID root_kms.RootCertsFile = os.environ.get("FAKE_ROOT_KMS_CA") root_kms.CertChainFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_CRT") root_kms.PrivateKeyFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_KEY") - nbs_configurator.files['root-kms'] = root_kms + cfg.files['root-kms'] = root_kms - nbs = daemon.start_nbs(nbs_configurator) + nbs = daemon.start_nbs(cfg) client = CreateClient(f"localhost:{nbs.port}") client.execute_action( @@ -115,9 +127,9 @@ def _wait_for_devices_to_be_cleared(client, expected_dirty_count=0): @pytest.fixture(name='disk_agent') def start_disk_agent(ydb, nbs, agent_id, data_path): - configurator = NbsConfigurator(ydb, 'disk-agent') - configurator.generate_default_nbs_configs() - configurator.files["disk-agent"] = generate_disk_agent_txt( + cfg = NbsConfigurator(ydb, 'disk-agent') + cfg.generate_default_nbs_configs() + cfg.files["disk-agent"] = generate_disk_agent_txt( agent_id='', device_erase_method='DEVICE_ERASE_METHOD_NONE', # speed up tests storage_discovery_config={ @@ -132,7 +144,7 @@ def start_disk_agent(ydb, nbs, agent_id, data_path): }]} ]}) - disk_agent = daemon.start_disk_agent(configurator) + disk_agent = daemon.start_disk_agent(cfg) disk_agent.wait_for_registration() client = CreateClient(f"localhost:{nbs.port}") @@ -145,7 +157,7 @@ def start_disk_agent(ydb, nbs, agent_id, data_path): disk_agent.stop() -def test_create_encrypted_volume(nbs, disk_agent): +def test_create_volume_with_default_ecnryption(nbs, disk_agent): client = CreateClient(f"localhost:{nbs.port}") @@ -158,3 +170,6 @@ def test_create_encrypted_volume(nbs, disk_agent): vol0 = client.stat_volume("vol0")["Volume"] assert len(vol0.Devices) == 2 + assert vol0.EncryptionDesc.Mode == ENCRYPTION_DEFAULT_AES_XTS + # assert vol0.EncryptionDesc.EncryptionKey.KekId == KEK_ID + # assert len(vol0.EncryptionDesc.EncryptionKey.EncryptedDEK) > 0 From 1e1af267c1d52fd15e79bbab1b5f2652839e129f Mon Sep 17 00:00:00 2001 From: sharpeye Date: Thu, 28 Nov 2024 21:28:07 +0000 Subject: [PATCH 05/13] fix tabs --- cloud/blockstore/libs/daemon/ydb/bootstrap.cpp | 16 ++++++++-------- .../storage/service/service_actor_create.cpp | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp b/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp index fcc86f73ae6..941075b4573 100644 --- a/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp +++ b/cloud/blockstore/libs/daemon/ydb/bootstrap.cpp @@ -561,16 +561,16 @@ void TBootstrapYdb::InitKikimrService() args.NvmeManager = NvmeManager; args.UserCounterProviders = {VolumeStats->GetUserCounters()}; args.IsDiskRegistrySpareNode = [&] { - if (!Configs->StorageConfig->GetDisableLocalService()) { - return false; - } + if (!Configs->StorageConfig->GetDisableLocalService()) { + return false; + } - const auto& nodes = Configs->StorageConfig->GetKnownSpareNodes(); - const auto& fqdn = FQDNHostName(); - const ui32 p = Configs->StorageConfig->GetSpareNodeProbability(); + const auto& nodes = Configs->StorageConfig->GetKnownSpareNodes(); + const auto& fqdn = FQDNHostName(); + const ui32 p = Configs->StorageConfig->GetSpareNodeProbability(); - return FindPtr(nodes, fqdn) || CityHash64(fqdn) % 100 < p; - }(); + return FindPtr(nodes, fqdn) || CityHash64(fqdn) % 100 < p; + }(); args.VolumeBalancerSwitch = VolumeBalancerSwitch; args.EndpointEventHandler = EndpointEventHandler; args.RootKmsKeyProvider = RootKmsKeyProvider; diff --git a/cloud/blockstore/libs/storage/service/service_actor_create.cpp b/cloud/blockstore/libs/storage/service/service_actor_create.cpp index e313eae21c0..1bd848d538c 100644 --- a/cloud/blockstore/libs/storage/service/service_actor_create.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor_create.cpp @@ -56,10 +56,6 @@ class TCreateVolumeActor final void DescribeBaseVolume(const TActorContext& ctx); void CreateVolume(const TActorContext& ctx); - void HandleCreateEncryptionKeyResponse( - const TEvServicePrivate::TEvCreateEncryptionKeyResponse::TPtr& ev, - const TActorContext& ctx); - void CreateVolumeImpl( const TActorContext& ctx, std::optional encryptionDesc); @@ -76,6 +72,10 @@ class TCreateVolumeActor final const TEvVolume::TEvWaitReadyResponse::TPtr& ev, const TActorContext& ctx); + void HandleCreateEncryptionKeyResponse( + const TEvServicePrivate::TEvCreateEncryptionKeyResponse::TPtr& ev, + const TActorContext& ctx); + void ReplyAndDie( const TActorContext& ctx, std::unique_ptr response); From 025602f0a8099c01e8af12fefc18893476eeb345 Mon Sep 17 00:00:00 2001 From: sharpeye Date: Thu, 28 Nov 2024 21:41:18 +0000 Subject: [PATCH 06/13] tweak --- .../storage/service/service_actor_create.cpp | 1 + .../tests/encryption_at_rest/test.py | 37 ++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/cloud/blockstore/libs/storage/service/service_actor_create.cpp b/cloud/blockstore/libs/storage/service/service_actor_create.cpp index 1bd848d538c..c2d5fdc2658 100644 --- a/cloud/blockstore/libs/storage/service/service_actor_create.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor_create.cpp @@ -333,6 +333,7 @@ void TCreateVolumeActor::HandleCreateEncryptionKeyResponse( auto& desc = encryptionDesc.emplace(); desc.SetMode(NProto::ENCRYPTION_DEFAULT_AES_XTS); + // XXX // desc.MutableEncryptionKey()->CopyFrom(msg.KmsKey); } diff --git a/cloud/blockstore/tests/encryption_at_rest/test.py b/cloud/blockstore/tests/encryption_at_rest/test.py index 595babe30b8..8149f0e7325 100644 --- a/cloud/blockstore/tests/encryption_at_rest/test.py +++ b/cloud/blockstore/tests/encryption_at_rest/test.py @@ -33,25 +33,6 @@ def get_agent_id(): return daemon.get_fqdn() -@pytest.fixture(name='data_path') -def create_data_path(): - - p = get_unique_path_for_current_test( - output_path=yatest_common.output_path(), - sub_folder="data") - - p = os.path.join(p, "dev", "disk", "by-partlabel") - ensure_path_exists(p) - - return p - - -@pytest.fixture(autouse=True) -def create_device_storage(data_path): - with open(os.path.join(data_path, 'NVMENBS01'), 'wb') as f: - os.truncate(f.fileno(), DEVICE_HEADER + DEVICE_SIZE * DEVICE_COUNT + (DEVICE_COUNT - 1) * DEVICE_PADDING) - - @pytest.fixture(name='ydb') def start_ydb_cluster(): @@ -125,7 +106,20 @@ def _wait_for_devices_to_be_cleared(client, expected_dirty_count=0): @pytest.fixture(name='disk_agent') -def start_disk_agent(ydb, nbs, agent_id, data_path): +def start_disk_agent(ydb, nbs, agent_id): + + data_path = get_unique_path_for_current_test( + output_path=yatest_common.output_path(), + sub_folder="data") + + data_path = os.path.join(data_path, "dev", "disk", "by-partlabel") + ensure_path_exists(data_path) + + with open(os.path.join(data_path, 'NVMENBS01'), 'wb') as f: + os.truncate( + f.fileno(), + DEVICE_HEADER + DEVICE_SIZE * DEVICE_COUNT + (DEVICE_COUNT - 1) * + DEVICE_PADDING) cfg = NbsConfigurator(ydb, 'disk-agent') cfg.generate_default_nbs_configs() @@ -171,5 +165,6 @@ def test_create_volume_with_default_ecnryption(nbs, disk_agent): assert len(vol0.Devices) == 2 assert vol0.EncryptionDesc.Mode == ENCRYPTION_DEFAULT_AES_XTS + # XXX # assert vol0.EncryptionDesc.EncryptionKey.KekId == KEK_ID - # assert len(vol0.EncryptionDesc.EncryptionKey.EncryptedDEK) > 0 + # assert len(vol0.EncryptionDesc.EncryptionKey.EncryptedDEK) == 32 From 325e8d7ddb995787f7e2028f189362f52e5860ea Mon Sep 17 00:00:00 2001 From: sharpeye Date: Fri, 29 Nov 2024 09:18:38 +0000 Subject: [PATCH 07/13] fix tests --- cloud/blockstore/libs/storage/testlib/test_env.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/cloud/blockstore/libs/storage/testlib/test_env.cpp b/cloud/blockstore/libs/storage/testlib/test_env.cpp index 32020824414..67b6ae8987d 100644 --- a/cloud/blockstore/libs/storage/testlib/test_env.cpp +++ b/cloud/blockstore/libs/storage/testlib/test_env.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include From d8a52f32f0693a3dd15b9ce9bbc5124f02beb2e0 Mon Sep 17 00:00:00 2001 From: sharpeye Date: Tue, 3 Dec 2024 17:43:53 +0000 Subject: [PATCH 08/13] tweak --- .../libs/storage/core/proto_helpers.cpp | 7 +++ .../storage/service/service_actor_create.cpp | 21 ++++--- .../volume/volume_actor_monitoring.cpp | 62 +++++++++++++------ .../tests/encryption_at_rest/test.py | 6 +- 4 files changed, 63 insertions(+), 33 deletions(-) diff --git a/cloud/blockstore/libs/storage/core/proto_helpers.cpp b/cloud/blockstore/libs/storage/core/proto_helpers.cpp index e0b4ef42aee..4f1c472d4f7 100644 --- a/cloud/blockstore/libs/storage/core/proto_helpers.cpp +++ b/cloud/blockstore/libs/storage/core/proto_helpers.cpp @@ -100,6 +100,13 @@ NProto::TEncryptionDesc ConvertToEncryptionDesc( NProto::TEncryptionDesc resultDesc; resultDesc.SetMode(mode); resultDesc.SetKeyHash(desc.GetKeyHash()); + + if (desc.HasEncryptedDataKey()) { + auto& key = *resultDesc.MutableEncryptionKey(); + key.SetKekId(desc.GetEncryptedDataKey().GetKekId()); + key.SetEncryptedDEK(desc.GetEncryptedDataKey().GetCiphertext()); + } + return resultDesc; } diff --git a/cloud/blockstore/libs/storage/service/service_actor_create.cpp b/cloud/blockstore/libs/storage/service/service_actor_create.cpp index c2d5fdc2658..3390b5c7577 100644 --- a/cloud/blockstore/libs/storage/service/service_actor_create.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor_create.cpp @@ -140,7 +140,7 @@ void TCreateVolumeActor::DescribeBaseVolume(const TActorContext& ctx) bool TCreateVolumeActor::ShouldCreateVolumeWithEncryptionAtRest() const { if (GetStorageMediaKind() == NProto::STORAGE_MEDIA_SSD_LOCAL) { - // Encryption at rest for local disks is not implemented. TODO: issue + // Encryption at rest for local disks is not implemented (#2598) return false; } @@ -162,18 +162,17 @@ void TCreateVolumeActor::CreateVolume(const TActorContext& ctx) TBlockStoreComponents::SERVICE, "Generate DEK for " << Request.GetDiskId().Quote()); - auto* actorSystem = TActivationContext::ActorSystem(); - auto selfId = ctx.SelfID; - KeyProvider->GenerateDataEncryptionKey(Request.GetDiskId()) .Subscribe( - [=](const auto& future) { - auto [key, error] = future.GetValue(); + [actorSystem = TActivationContext::ActorSystem(), + selfId = ctx.SelfID](const auto& future) + { + const auto& [key, error] = future.GetValue(); auto response = std::make_unique< TEvServicePrivate::TEvCreateEncryptionKeyResponse>( - std::move(error), - std::move(key)); + error, + key); actorSystem->Send(selfId, response.release()); }); @@ -333,8 +332,10 @@ void TCreateVolumeActor::HandleCreateEncryptionKeyResponse( auto& desc = encryptionDesc.emplace(); desc.SetMode(NProto::ENCRYPTION_DEFAULT_AES_XTS); - // XXX - // desc.MutableEncryptionKey()->CopyFrom(msg.KmsKey); + + auto& dek = *desc.MutableEncryptedDataKey(); + dek.SetKekId(msg.KmsKey.GetKekId()); + dek.SetCiphertext(msg.KmsKey.GetEncryptedDEK()); } CreateVolumeImpl(ctx, std::move(encryptionDesc)); diff --git a/cloud/blockstore/libs/storage/volume/volume_actor_monitoring.cpp b/cloud/blockstore/libs/storage/volume/volume_actor_monitoring.cpp index e610f63415b..2c921e33985 100644 --- a/cloud/blockstore/libs/storage/volume/volume_actor_monitoring.cpp +++ b/cloud/blockstore/libs/storage/volume/volume_actor_monitoring.cpp @@ -95,6 +95,47 @@ IOutputStream& operator <<( } } +IOutputStream& operator <<( + IOutputStream& out, + const NKikimrBlockStore::TEncryptionDesc& desc) +{ + const auto encryptionMode = + static_cast(desc.GetMode()); + + HTML(out) { + DIV() { out << NProto::EEncryptionMode_Name(encryptionMode); } + + switch (encryptionMode) { + case NProto::ENCRYPTION_AES_XTS: { + DIV() { + const auto& keyHash = desc.GetKeyHash(); + if (keyHash.empty()) { + out << "Binding to the encryption key has not yet " + "occurred."; + } else { + out << "Encryption key hash: " << keyHash; + } + } + break; + } + + case NProto::ENCRYPTION_DEFAULT_AES_XTS: { + const auto& key = desc.GetEncryptedDataKey(); + DIV() { out << "Kek Id: " << key.GetKekId().Quote(); } + DIV() { + out << "Encrypted DEK has " << key.GetCiphertext().size() + << " bytes length"; + } + break; + } + default: + break; + } + } + + return out; +} + //////////////////////////////////////////////////////////////////////////////// void OutputProgress( @@ -1161,26 +1202,7 @@ void TVolumeActor::RenderConfig(IOutputStream& out) const TABLER() { TABLED() { out << "Encryption"; } - TABLED() { - DIV() - { - auto encryptionMode = - static_cast( - volumeConfig.GetEncryptionDesc().GetMode()); - out << NProto::EEncryptionMode_Name(encryptionMode); - } - DIV() - { - const auto& keyHash = - volumeConfig.GetEncryptionDesc().GetKeyHash(); - if (keyHash.empty()) { - out << "Binding to the encryption key has not " - "yet occurred."; - } else { - out << "Encryption key hash: " << keyHash; - } - } - } + TABLED() { out << volumeConfig.GetEncryptionDesc(); } } TABLER() { diff --git a/cloud/blockstore/tests/encryption_at_rest/test.py b/cloud/blockstore/tests/encryption_at_rest/test.py index 8149f0e7325..91314182f3b 100644 --- a/cloud/blockstore/tests/encryption_at_rest/test.py +++ b/cloud/blockstore/tests/encryption_at_rest/test.py @@ -165,6 +165,6 @@ def test_create_volume_with_default_ecnryption(nbs, disk_agent): assert len(vol0.Devices) == 2 assert vol0.EncryptionDesc.Mode == ENCRYPTION_DEFAULT_AES_XTS - # XXX - # assert vol0.EncryptionDesc.EncryptionKey.KekId == KEK_ID - # assert len(vol0.EncryptionDesc.EncryptionKey.EncryptedDEK) == 32 + + assert vol0.EncryptionDesc.EncryptionKey.KekId == KEK_ID + assert len(vol0.EncryptionDesc.EncryptionKey.EncryptedDEK) == 512 From ea28a3d88840cd4349df7e7a23c7ecf622f3300d Mon Sep 17 00:00:00 2001 From: sharpeye Date: Wed, 4 Dec 2024 19:54:26 +0000 Subject: [PATCH 09/13] local-nonrepl: load-encryption-at-rest --- .../tests/loadtest/local-nonrepl/test.py | 36 +++++++++++-------- .../tests/loadtest/local-nonrepl/ya.make.inc | 2 ++ .../blockstore/tests/python/lib/nbs_runner.py | 16 +++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/cloud/blockstore/tests/loadtest/local-nonrepl/test.py b/cloud/blockstore/tests/loadtest/local-nonrepl/test.py index 84cc97b6b0b..7bfdaecf396 100644 --- a/cloud/blockstore/tests/loadtest/local-nonrepl/test.py +++ b/cloud/blockstore/tests/loadtest/local-nonrepl/test.py @@ -39,7 +39,7 @@ def enable_lwtrace(kikimr, query_file): update_cms_config(kikimr, 'DiagnosticsConfig', config, 'disk-agent') -class TestCase(object): +class _TestCase(object): def __init__( self, @@ -55,7 +55,8 @@ def __init__( agent_count=1, storage_pool_name=None, dump_block_digests=False, - reject_late_requests_at_disk_agent=False): + reject_late_requests_at_disk_agent=False, + encryption_at_rest=False): self.name = name self.config_path = config_path self.restart_interval = restart_interval @@ -69,29 +70,30 @@ def __init__( self.storage_pool_name = storage_pool_name self.dump_block_digests = dump_block_digests self.reject_late_requests_at_disk_agent = reject_late_requests_at_disk_agent + self.encryption_at_rest = encryption_at_rest TESTS = [ - TestCase( + _TestCase( "load", "cloud/blockstore/tests/loadtest/local-nonrepl/local-smallreqs.txt", ), - TestCase( + _TestCase( "restarts", "cloud/blockstore/tests/loadtest/local-nonrepl/local-nonrepl.txt", restart_interval=get_restart_interval(), ), - TestCase( + _TestCase( "late_requests", "cloud/blockstore/tests/loadtest/local-nonrepl/local-nonrepl.txt", reject_late_requests_at_disk_agent=True, ), - TestCase( + _TestCase( "io-errors", "cloud/blockstore/tests/loadtest/local-nonrepl/local-nonrepl.txt", lwtrace_query_path="cloud/blockstore/tests/loadtest/local-nonrepl/io-errors.tr", ), - TestCase( + _TestCase( "large-disk", "cloud/blockstore/tests/loadtest/local-nonrepl/local-large-disk.txt", use_memory_devices=True, @@ -100,39 +102,44 @@ def __init__( block_count_per_device=256 * 1024 * 1024, # 1TiB allocation_unit_size=1024, # 1TiB ), - TestCase( + _TestCase( "migration", "cloud/blockstore/tests/loadtest/local-nonrepl/local-migration.txt", # agent_count=2, dump_block_digests=True, ), - TestCase( + _TestCase( "migration-nbd", "cloud/blockstore/tests/loadtest/local-nonrepl/local-migration-nbd.txt", # agent_count=2, dump_block_digests=True, ), - TestCase( + _TestCase( "local-encryption", "cloud/blockstore/tests/loadtest/local-nonrepl/local-encryption.txt", ), - TestCase( + _TestCase( "remote-encryption", "cloud/blockstore/tests/loadtest/local-nonrepl/remote-encryption.txt", ), - TestCase( + _TestCase( "local-encryption-overlay", "cloud/blockstore/tests/loadtest/local-nonrepl/local-encryption-overlay.txt", ), - TestCase( + _TestCase( "local-nonrepl-overlay", "cloud/blockstore/tests/loadtest/local-nonrepl/local-nonrepl-overlay.txt", ), - TestCase( + _TestCase( "load-hdd", "cloud/blockstore/tests/loadtest/local-nonrepl/local-hdd.txt", storage_pool_name="rot", ), + _TestCase( + "load-encryption-at-rest", + "cloud/blockstore/tests/loadtest/local-nonrepl/local-smallreqs.txt", + encryption_at_rest=True, + ) ] @@ -241,6 +248,7 @@ def __run_test(test_case): storage.RejectLateRequestsAtDiskAgentEnabled = test_case.reject_late_requests_at_disk_agent storage.AssignIdToWriteAndZeroRequestsEnabled = test_case.reject_late_requests_at_disk_agent storage.NodeType = 'main' + storage.EncryptionAtRestForDiskRegistryBasedDisksEnabled = test_case.encryption_at_rest if test_case.dump_block_digests: storage.BlockDigestsEnabled = True diff --git a/cloud/blockstore/tests/loadtest/local-nonrepl/ya.make.inc b/cloud/blockstore/tests/loadtest/local-nonrepl/ya.make.inc index 962065915cc..7cfd9e530b7 100644 --- a/cloud/blockstore/tests/loadtest/local-nonrepl/ya.make.inc +++ b/cloud/blockstore/tests/loadtest/local-nonrepl/ya.make.inc @@ -44,3 +44,5 @@ REQUIREMENTS( FORK_SUBTESTS() SPLIT_FACTOR(2) + +INCLUDE(${ARCADIA_ROOT}/cloud/blockstore/tests/recipes/fake-root-kms/recipe.inc) diff --git a/cloud/blockstore/tests/python/lib/nbs_runner.py b/cloud/blockstore/tests/python/lib/nbs_runner.py index fdf36222dd9..4f2064c3288 100644 --- a/cloud/blockstore/tests/python/lib/nbs_runner.py +++ b/cloud/blockstore/tests/python/lib/nbs_runner.py @@ -6,6 +6,7 @@ from cloud.blockstore.config.diagnostics_pb2 import TDiagnosticsConfig from cloud.blockstore.config.disk_pb2 import TDiskRegistryProxyConfig +from cloud.blockstore.config.root_kms_pb2 import TRootKmsConfig from cloud.blockstore.config.storage_pb2 import TStorageServiceConfig from cloud.blockstore.config.server_pb2 import TServerAppConfig, TKikimrServiceConfig, TServerConfig, TLocation from cloud.storage.core.config.features_pb2 import TFeaturesConfig @@ -146,6 +147,18 @@ def __init__( if self.__server_app_config is None or self.__server_app_config.HasField('KikimrServiceConfig'): self.init_scheme() + self.__has_root_kms = False + root_kms_port = os.environ.get("FAKE_ROOT_KMS_PORT") + if root_kms_port is not None: + root_kms = TRootKmsConfig() + root_kms.Address = f'localhost:{root_kms_port}' + root_kms.KeyId = 'nbs' + root_kms.RootCertsFile = os.environ.get("FAKE_ROOT_KMS_CA") + root_kms.CertChainFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_CRT") + root_kms.PrivateKeyFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_KEY") + self.__proto_configs['root-kms.txt'] = root_kms + self.__has_root_kms = True + self.__access_service = None if enable_access_service: host = "localhost" @@ -582,6 +595,9 @@ def append_conf_file_arg(command, config_path, option_name, conf_file): if self.kms_config is not None: command += ["--kms-file", os.path.join(self.config_path(), "kms.txt")] + if self.__has_root_kms: + command += ["--root-kms-file", os.path.join(self.config_path(), "root-kms.txt")] + append_conf_file_arg(command, self.config_path(), "--location-file", "location.txt") From fe7943bc50a7add0965245974ade72b001fb06e4 Mon Sep 17 00:00:00 2001 From: sharpeye Date: Wed, 4 Dec 2024 20:36:18 +0000 Subject: [PATCH 10/13] don't create unencrypted disk on failure --- .../storage/service/service_actor_create.cpp | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/cloud/blockstore/libs/storage/service/service_actor_create.cpp b/cloud/blockstore/libs/storage/service/service_actor_create.cpp index 3390b5c7577..8a7945c1285 100644 --- a/cloud/blockstore/libs/storage/service/service_actor_create.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor_create.cpp @@ -321,23 +321,27 @@ void TCreateVolumeActor::HandleCreateEncryptionKeyResponse( LOG_ERROR_S( ctx, TBlockStoreComponents::SERVICE, - "Failed to generate encryption key: " - << FormatError(error) << ". Create an unencrypted volume."); - } else { - LOG_INFO_S( + "Failed to generate encryption key: " << FormatError(error)); + ReplyAndDie( ctx, - TBlockStoreComponents::SERVICE, - "Create volume " << Request.GetDiskId().Quote() - << " with default AES XTS encryption"); - - auto& desc = encryptionDesc.emplace(); - desc.SetMode(NProto::ENCRYPTION_DEFAULT_AES_XTS); + std::make_unique(error)); - auto& dek = *desc.MutableEncryptedDataKey(); - dek.SetKekId(msg.KmsKey.GetKekId()); - dek.SetCiphertext(msg.KmsKey.GetEncryptedDEK()); + return; } + LOG_INFO_S( + ctx, + TBlockStoreComponents::SERVICE, + "Create volume " << Request.GetDiskId().Quote() + << " with default AES XTS encryption"); + + auto& desc = encryptionDesc.emplace(); + desc.SetMode(NProto::ENCRYPTION_DEFAULT_AES_XTS); + + auto& dek = *desc.MutableEncryptedDataKey(); + dek.SetKekId(msg.KmsKey.GetKekId()); + dek.SetCiphertext(msg.KmsKey.GetEncryptedDEK()); + CreateVolumeImpl(ctx, std::move(encryptionDesc)); } From f9b68c1afc9d7efc86865be2ed3815f3f898730d Mon Sep 17 00:00:00 2001 From: sharpeye Date: Wed, 4 Dec 2024 20:36:43 +0000 Subject: [PATCH 11/13] tweak test: check raw data --- .../tests/encryption_at_rest/test.py | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/cloud/blockstore/tests/encryption_at_rest/test.py b/cloud/blockstore/tests/encryption_at_rest/test.py index 91314182f3b..e60cce92f75 100644 --- a/cloud/blockstore/tests/encryption_at_rest/test.py +++ b/cloud/blockstore/tests/encryption_at_rest/test.py @@ -9,7 +9,7 @@ from cloud.blockstore.public.api.protos.encryption_pb2 import \ ENCRYPTION_DEFAULT_AES_XTS -from cloud.blockstore.public.sdk.python.client import CreateClient +from cloud.blockstore.public.sdk.python.client import CreateClient, Session from cloud.blockstore.public.sdk.python.protos import TCmsActionRequest, \ TAction, STORAGE_MEDIA_SSD_NONREPLICATED from cloud.blockstore.config.root_kms_pb2 import TRootKmsConfig @@ -161,10 +161,34 @@ def test_create_volume_with_default_ecnryption(nbs, disk_agent): blocks_count=2*DEVICE_SIZE//4096, storage_media_kind=STORAGE_MEDIA_SSD_NONREPLICATED) - vol0 = client.stat_volume("vol0")["Volume"] + session = Session(client, "vol0", "") + vol0 = session.mount_volume()['Volume'] + + def read_first_block(): + with open(vol0.Devices[0].DeviceName, 'rb') as f: + f.seek(vol0.Devices[0].PhysicalOffset) + return f.read(4096) + + # check that the first block is clean + raw_data = read_first_block() + assert raw_data.count(0) == 4096 assert len(vol0.Devices) == 2 assert vol0.EncryptionDesc.Mode == ENCRYPTION_DEFAULT_AES_XTS assert vol0.EncryptionDesc.EncryptionKey.KekId == KEK_ID assert len(vol0.EncryptionDesc.EncryptionKey.EncryptedDEK) == 512 + + expected_data = os.urandom(4096) + + session.write_blocks(0, [expected_data]) + blocks = session.read_blocks(0, 1, checkpoint_id="") + assert len(blocks) == 1 + assert expected_data == blocks[0] + + # check that the first block is encrypted + raw_data = read_first_block() + assert raw_data.count(0) != 4096 + assert raw_data != expected_data + + session.unmount_volume() From 6d6be47a417d8dbe54ff5fe65ca0873872884580 Mon Sep 17 00:00:00 2001 From: sharpeye Date: Fri, 6 Dec 2024 14:34:43 +0000 Subject: [PATCH 12/13] cleanup --- .../storage/service/service_actor_create.cpp | 27 ++++++++----------- .../blockstore/tests/python/lib/nbs_runner.py | 4 +-- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/cloud/blockstore/libs/storage/service/service_actor_create.cpp b/cloud/blockstore/libs/storage/service/service_actor_create.cpp index 8a7945c1285..935eee2eaba 100644 --- a/cloud/blockstore/libs/storage/service/service_actor_create.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor_create.cpp @@ -18,8 +18,6 @@ #include #include -#include - namespace NCloud::NBlockStore::NStorage { using namespace NActors; @@ -58,7 +56,7 @@ class TCreateVolumeActor final void CreateVolume(const TActorContext& ctx); void CreateVolumeImpl( const TActorContext& ctx, - std::optional encryptionDesc); + NKikimrBlockStore::TEncryptionDesc encryptionDesc); void HandleDescribeVolumeResponse( const TEvSSProxy::TEvDescribeVolumeResponse::TPtr& ev, @@ -179,13 +177,12 @@ void TCreateVolumeActor::CreateVolume(const TActorContext& ctx) return; } - std::optional encryptionDesc; + NKikimrBlockStore::TEncryptionDesc encryptionDesc; const auto& encryptionSpec = Request.GetEncryptionSpec(); if (encryptionSpec.GetMode() != NProto::NO_ENCRYPTION) { - auto& desc = encryptionDesc.emplace(); - desc.SetMode(encryptionSpec.GetMode()); - desc.SetKeyHash(encryptionSpec.GetKeyHash()); + encryptionDesc.SetMode(encryptionSpec.GetMode()); + encryptionDesc.SetKeyHash(encryptionSpec.GetKeyHash()); } CreateVolumeImpl(ctx, std::move(encryptionDesc)); @@ -193,7 +190,7 @@ void TCreateVolumeActor::CreateVolume(const TActorContext& ctx) void TCreateVolumeActor::CreateVolumeImpl( const TActorContext& ctx, - std::optional encryptionDesc) + NKikimrBlockStore::TEncryptionDesc encryptionDesc) { NKikimrBlockStore::TVolumeConfig config; @@ -285,14 +282,14 @@ void TCreateVolumeActor::CreateVolumeImpl( } config.MutableAgentIds()->CopyFrom(Request.GetAgentIds()); - if (encryptionDesc) { + if (encryptionDesc.GetMode() != NProto::NO_ENCRYPTION) { LOG_DEBUG_S( ctx, TBlockStoreComponents::SERVICE, "Creating volume with an encryption: " - << NProto::EEncryptionMode_Name(encryptionDesc->GetMode())); + << NProto::EEncryptionMode_Name(encryptionDesc.GetMode())); - *config.MutableEncryptionDesc() = std::move(*encryptionDesc); + *config.MutableEncryptionDesc() = std::move(encryptionDesc); } auto request = std::make_unique( @@ -315,8 +312,6 @@ void TCreateVolumeActor::HandleCreateEncryptionKeyResponse( { const auto& msg = *ev->Get(); - std::optional encryptionDesc; - if (const auto& error = msg.GetError(); HasError(error)) { LOG_ERROR_S( ctx, @@ -335,10 +330,10 @@ void TCreateVolumeActor::HandleCreateEncryptionKeyResponse( "Create volume " << Request.GetDiskId().Quote() << " with default AES XTS encryption"); - auto& desc = encryptionDesc.emplace(); - desc.SetMode(NProto::ENCRYPTION_DEFAULT_AES_XTS); + NKikimrBlockStore::TEncryptionDesc encryptionDesc; + encryptionDesc.SetMode(NProto::ENCRYPTION_DEFAULT_AES_XTS); - auto& dek = *desc.MutableEncryptedDataKey(); + auto& dek = *encryptionDesc.MutableEncryptedDataKey(); dek.SetKekId(msg.KmsKey.GetKekId()); dek.SetCiphertext(msg.KmsKey.GetEncryptedDEK()); diff --git a/cloud/blockstore/tests/python/lib/nbs_runner.py b/cloud/blockstore/tests/python/lib/nbs_runner.py index 4f2064c3288..7db616e20d3 100644 --- a/cloud/blockstore/tests/python/lib/nbs_runner.py +++ b/cloud/blockstore/tests/python/lib/nbs_runner.py @@ -147,7 +147,6 @@ def __init__( if self.__server_app_config is None or self.__server_app_config.HasField('KikimrServiceConfig'): self.init_scheme() - self.__has_root_kms = False root_kms_port = os.environ.get("FAKE_ROOT_KMS_PORT") if root_kms_port is not None: root_kms = TRootKmsConfig() @@ -157,7 +156,6 @@ def __init__( root_kms.CertChainFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_CRT") root_kms.PrivateKeyFile = os.environ.get("FAKE_ROOT_KMS_CLIENT_KEY") self.__proto_configs['root-kms.txt'] = root_kms - self.__has_root_kms = True self.__access_service = None if enable_access_service: @@ -595,7 +593,7 @@ def append_conf_file_arg(command, config_path, option_name, conf_file): if self.kms_config is not None: command += ["--kms-file", os.path.join(self.config_path(), "kms.txt")] - if self.__has_root_kms: + if 'root-kms.txt' in self.__proto_configs: command += ["--root-kms-file", os.path.join(self.config_path(), "root-kms.txt")] append_conf_file_arg(command, self.config_path(), From ca8d64bf3ae11659d7e32322d2da16c283dd5e9b Mon Sep 17 00:00:00 2001 From: sharpeye Date: Fri, 6 Dec 2024 17:12:15 +0000 Subject: [PATCH 13/13] use IsDiskRegistryLocalMediaKind --- cloud/blockstore/libs/storage/service/service_actor_create.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloud/blockstore/libs/storage/service/service_actor_create.cpp b/cloud/blockstore/libs/storage/service/service_actor_create.cpp index 935eee2eaba..2aab623acf7 100644 --- a/cloud/blockstore/libs/storage/service/service_actor_create.cpp +++ b/cloud/blockstore/libs/storage/service/service_actor_create.cpp @@ -137,7 +137,7 @@ void TCreateVolumeActor::DescribeBaseVolume(const TActorContext& ctx) bool TCreateVolumeActor::ShouldCreateVolumeWithEncryptionAtRest() const { - if (GetStorageMediaKind() == NProto::STORAGE_MEDIA_SSD_LOCAL) { + if (IsDiskRegistryLocalMediaKind(GetStorageMediaKind())) { // Encryption at rest for local disks is not implemented (#2598) return false; }