Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG][Storage] Client side encryption causes high CPU. #21594

Closed
kasobol-msft opened this issue Jun 4, 2021 · 3 comments
Closed

[BUG][Storage] Client side encryption causes high CPU. #21594

kasobol-msft opened this issue Jun 4, 2021 · 3 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Storage Storage Service (Queues, Blobs, Files)
Milestone

Comments

@kasobol-msft
Copy link
Contributor

On behalf of customer.

When client side encryption is used it wraps a stream here:

// if using clientside encryption, wrap the auto-retry stream in a decryptor
// we already return a nonseekable stream; returning a crypto stream is fine
if (UsingClientSideEncryption)
{
stream = await new BlobClientSideDecryptor(
new ClientSideDecryptor(ClientSideEncryption)).DecryptInternal(
stream,
response.Value.Details.Metadata,
requestedRange,
response.Value.Details.ContentRange,
async,
cancellationToken).ConfigureAwait(false);
}

The actual wrapping happens here:
return new CryptoStream(contentStream, aesProvider.CreateDecryptor(), CryptoStreamMode.Read);

The CryptoStream uses low block size leading to multiple unbuffered reads from network.
This in turn creates a lot of trace event emission from RetriableStream.

Desired solution is to inject buffering between crypto stream and network stream, or make crypto stream respect buffering settings from callers.

@kasobol-msft kasobol-msft added Storage Storage Service (Queues, Blobs, Files) Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. bug This issue requires a change to an existing behavior in the product in order to be resolved. labels Jun 4, 2021
@ghost ghost added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jun 4, 2021
@shalinirj
Copy link

We are running into the same issue with latest Storage SDK. Production workloads are impacted during the peak hours. Having to scale up/out is incurring additional business costs.

@kasobol-msft kasobol-msft self-assigned this Jun 5, 2021
@kasobol-msft
Copy link
Contributor Author

kasobol-msft commented Jun 7, 2021

Repro:

namespace ConsoleApp1
{
    using System;
    using System.Diagnostics.Tracing;
    using System.IO;
    using System.Threading;
    using System.Threading.Tasks;
    using Azure.Core;
    using Azure.Identity;
    using Azure.Security.KeyVault.Keys;
    using Azure.Security.KeyVault.Keys.Cryptography;
    using Azure.Storage;
    using Azure.Storage.Blobs;
    using Azure.Storage.Blobs.Models;
    using Azure.Storage.Blobs.Specialized;

    class Program
    {
        private const string StorageConnectionString = "REDACTED";


        static async Task Main(string[] args)
        {
            var cred = new ClientSecretCredential("REDACTED", "REDACTED", "REDACTED");
            var kvClient = new KeyClient(new Uri("https://kasobolkv.vault.azure.net/"), cred);
            var service = new BlobServiceClient(StorageConnectionString);

            using (var evtListener = new ThreadTransferSendEventListener())
            {
                await ReadWithClientSideEncryption(kvClient, service, cred);
                Console.WriteLine($"(ReadWithClientSideEncryption) Number of ThreadTransferSend events: {evtListener.counter}");
            }
            using (var evtListener = new ThreadTransferSendEventListener())
            {
                await ReadWithoutClientSideEncryption(service);
                Console.WriteLine($"(ReadWithoutClientSideEncryption) Number of ThreadTransferSend events: {evtListener.counter}");
            }
        }

        static async Task ReadWithClientSideEncryption(KeyClient kvClient, BlobServiceClient service, TokenCredential cred)
        {
            KeyVaultKey key = await kvClient.GetKeyAsync("testkey");
            var cryptoClient = new CryptographyClient(key.Id, cred);

            var container = service.GetBlobContainerClient("foo");
            var encryptionOptions = new ClientSideEncryptionOptions(ClientSideEncryptionVersion.V1_0);
            encryptionOptions.KeyEncryptionKey = cryptoClient;
            encryptionOptions.KeyWrapAlgorithm = "RSA-OAEP-256";
            var blob = container.GetBlobClient("bar").WithClientSideEncryptionOptions(encryptionOptions);

            byte[] data = new byte[1024 * 1024 * 10];

            // uncomment to upload data
            //new Random().NextBytes(data);
            //await blob.UploadAsync(BinaryData.FromBytes(data), new BlobUploadOptions());

            Stream stream = await blob.OpenReadAsync();

            await stream.ReadAsync(data);
        }

        static async Task ReadWithoutClientSideEncryption(BlobServiceClient service)
        {
            var container = service.GetBlobContainerClient("foo");
            var blob = container.GetBlobClient("baz");

            byte[] data = new byte[1024 * 1024 * 10];

            // uncomment to upload data
            //new Random().NextBytes(data);
            //await blob.UploadAsync(BinaryData.FromBytes(data), new BlobUploadOptions());
           
            Stream stream = await blob.OpenReadAsync();

            await stream.ReadAsync(data);
        }
    }

    class ThreadTransferSendEventListener : EventListener
    {
        public int counter = 0;

        protected override void OnEventSourceCreated(EventSource eventSource)
        {
            base.OnEventSourceCreated(eventSource);
            if (eventSource.Name.Contains("FrameworkEventSource"))
            {
                EnableEvents(eventSource, EventLevel.LogAlways);
            }
        }

        protected override void OnEventWritten(EventWrittenEventArgs eventData)
        {
            base.OnEventWritten(eventData);

            if (eventData.EventName.Contains("ThreadTransferSend"))
            {
                Interlocked.Increment(ref counter);
            }

        }
    }
}

image

image

  1. Network Streams used in DownloadStreamingInternal which powers functionalities like DownloadStreaming, Download, OpenRead are wrapped with ReadTimeoutStream.
  2. ReadTimeoutStream creates CancellationTokenSource with expiry on each read here.
  3. When using ClientSideEncryption the CryptoStream wraps around ReadTimeoutStream/NetworkStream sandwich. However it uses very tiny buffer (~16 bytes) leading to creating a lot of CancellationTokenSource.
  4. Creating CancellationTokenSource with expiry (i.e. CancelAfter) results in event published here.

Proposed solution is to inject layer of buffering between CryptoStream and ReadTimeoutStream/NetworkStream

@kasobol-msft
Copy link
Contributor Author

azure-sdk pushed a commit to azure-sdk/azure-sdk-for-net that referenced this issue Nov 18, 2022
lint error fix (Azure#21594)

Co-authored-by: Anandaraj Selvam <anselvam@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

3 participants