Skip to content

Commit

Permalink
Fix issue #54 with stream being unintentionally closed before all par…
Browse files Browse the repository at this point in the history
…ts are uploaded.
  • Loading branch information
normj committed Aug 23, 2024
1 parent ad92c45 commit e0e0dd3
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 12 deletions.
6 changes: 3 additions & 3 deletions src/Amazon.Extensions.S3.Encryption.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<TargetFrameworks>net35;net45;netstandard2.0;netcoreapp3.1</TargetFrameworks>
<Version>2.1.1</Version>
<Version>2.1.2</Version>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<PackageId>Amazon.Extensions.S3.Encryption</PackageId>
<Title>Amazon S3 Encryption Client for .NET</Title>
Expand All @@ -15,8 +15,8 @@
<PackageIcon>icon.png</PackageIcon>
<RepositoryUrl>https://github.com/aws/amazon-s3-encryption-client-dotnet/</RepositoryUrl>
<Company>Amazon Web Services</Company>
<AssemblyVersion>2.1.1</AssemblyVersion>
<FileVersion>2.1.1</FileVersion>
<AssemblyVersion>2.1.2</AssemblyVersion>
<FileVersion>2.1.2</FileVersion>

<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>..\public.snk</AssemblyOriginatorKeyFile>
Expand Down
3 changes: 2 additions & 1 deletion src/Internal/SetupDecryptionHandlerV1.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using Amazon.Runtime.Internal.Util;
using Amazon.S3;
using ThirdParty.Json.LitJson;
using Amazon.Extensions.S3.Encryption.Util;

namespace Amazon.Extensions.S3.Encryption.Internal
{
Expand Down Expand Up @@ -168,7 +169,7 @@ protected override void UpdateMultipartUploadEncryptionContext(UploadPartRequest
{
object stream = null;

if (!((Amazon.Runtime.Internal.IAmazonWebServiceRequest) uploadPartRequest).RequestState.TryGetValue(AmazonS3EncryptionClient.S3CryptoStream, out stream))
if (!((Amazon.Runtime.Internal.IAmazonWebServiceRequest) uploadPartRequest).RequestState.TryGetValue(Constants.S3CryptoStreamRequestState, out stream))
throw new AmazonS3Exception("Cannot retrieve S3 crypto stream from request state, hence cannot get Initialization vector for next uploadPart ");

var encryptionStream = stream as AESEncryptionUploadPartStream;
Expand Down
2 changes: 1 addition & 1 deletion src/Internal/SetupDecryptionHandlerV2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ protected override void UpdateMultipartUploadEncryptionContext(UploadPartRequest
{
object stream = null;

if (!((IAmazonWebServiceRequest) uploadPartRequest).RequestState.TryGetValue(AmazonS3EncryptionClient.S3CryptoStream, out stream))
if (!((IAmazonWebServiceRequest) uploadPartRequest).RequestState.TryGetValue(Constants.S3CryptoStreamRequestState, out stream))
throw new AmazonS3Exception("Cannot retrieve S3 crypto stream from request state, hence cannot get Initialization vector for next uploadPart ");

var encryptionStream = stream as AESEncryptionUploadPartStream;
Expand Down
3 changes: 2 additions & 1 deletion src/Internal/SetupEncryptionHandlerV1.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

using Amazon.Runtime;
using Amazon.S3.Model;
using Amazon.Extensions.S3.Encryption.Util;

namespace Amazon.Extensions.S3.Encryption.Internal
{
Expand Down Expand Up @@ -153,7 +154,7 @@ protected override void GenerateEncryptedUploadPartRequest(UploadPartRequest req
request.InputStream = EncryptionUtils.EncryptRequestUsingInstruction(request.InputStream, instructions);
contextForEncryption.IsFinalPart = true;
}
((Amazon.Runtime.Internal.IAmazonWebServiceRequest)request).RequestState.Add(AmazonS3EncryptionClient.S3CryptoStream, request.InputStream);
((Amazon.Runtime.Internal.IAmazonWebServiceRequest)request).RequestState.Add(Constants.S3CryptoStreamRequestState, request.InputStream);
}
}
}
79 changes: 74 additions & 5 deletions src/Internal/SetupEncryptionHandlerV2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
* permissions and limitations under the License.
*/

using Amazon.Extensions.S3.Encryption.Util;
using Amazon.Extensions.S3.Encryption.Util;
using Amazon.Runtime;
using Amazon.S3.Model;
using Amazon.S3.Model;
using System;

namespace Amazon.Extensions.S3.Encryption.Internal
namespace Amazon.Extensions.S3.Encryption.Internal
{
/// <summary>
/// Custom pipeline handler to encrypt the data as it is being uploaded to S3 for AmazonS3EncryptionClientV2.
Expand All @@ -37,6 +38,64 @@ public SetupEncryptionHandlerV2(AmazonS3EncryptionClientBase encryptionClient) :
{
}

/// <inheritdoc/>
public override void InvokeSync(IExecutionContext executionContext)
{
try
{
base.InvokeSync(executionContext);
}
catch (Exception)
{
HandleException(executionContext);
throw;
}
}

#if AWS_ASYNC_API
/// <inheritdoc/>
public override async System.Threading.Tasks.Task<T> InvokeAsync<T>(IExecutionContext executionContext)
{
try
{
return await base.InvokeAsync<T>(executionContext);
}
catch (Exception)
{
HandleException(executionContext);
throw;
}
}
#endif

/// <summary>
/// If the crypto stream that is reused for each part has its disposed disabled then the SDK
/// did not close the stream after the exception occurred. This method is called after a exception
/// has ocurred and force the crypto stream to be closed.
/// </summary>
/// <param name="executionContext"></param>
private void HandleException(IExecutionContext executionContext)
{
var request = executionContext.RequestContext.OriginalRequest;
var uploadPartRequest = request as UploadPartRequest;
if (uploadPartRequest != null)
{
var contextForEncryption = this.EncryptionClient.CurrentMultiPartUploadKeys[uploadPartRequest.UploadId];
if (contextForEncryption == null)
return;

var aesGcmEncryptStream = contextForEncryption.CryptoStream as AesGcmEncryptStream;
if (aesGcmEncryptStream == null)
return;

if (aesGcmEncryptStream.DisableDispose)
{
aesGcmEncryptStream.DisableDispose = false;
aesGcmEncryptStream.Dispose();
}
}
}

/// <inheritdoc/>
protected override EncryptionInstructions GenerateInstructions(IExecutionContext executionContext)
{
Expand Down Expand Up @@ -149,7 +208,7 @@ protected override void GenerateEncryptedUploadPartRequest(UploadPartRequest req
UpdateRequestInputStream(request, contextForEncryption, instructions);
contextForEncryption.IsFinalPart = true;
}
((Amazon.Runtime.Internal.IAmazonWebServiceRequest)request).RequestState.Add(AmazonS3EncryptionClient.S3CryptoStream, request.InputStream);
((Amazon.Runtime.Internal.IAmazonWebServiceRequest)request).RequestState.Add(Constants.S3CryptoStreamRequestState, request.InputStream);

}

Expand All @@ -167,9 +226,19 @@ private static void UpdateRequestInputStream(UploadPartRequest request, UploadPa
// Clear the buffer filled for retry request
var aesGcmEncryptCachingStream = request.InputStream as AesGcmEncryptCachingStream;
if (aesGcmEncryptCachingStream != null)
{
{
aesGcmEncryptCachingStream.ClearReadBufferToPosition();
}

var aesGcmEncryptStream = request.InputStream as AesGcmEncryptStream;
if (aesGcmEncryptStream != null)
{
// The stream is reused across multi part uploads to maintain the encryption state.
// The SDK will attempt to close the stream after the part is upload but setting
// DisableDispose to true for anything besides the last part will make the
// disable a noop.
aesGcmEncryptStream.DisableDispose = !request.IsLastPart;
}
}
}
}
25 changes: 25 additions & 0 deletions src/Util/AesGcmEncryptStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -148,5 +148,30 @@ public override async System.Threading.Tasks.Task<int> ReadAsync(byte[] buffer,
}
}
#endif

/// <summary>
/// If set to true the Close and Dispose methods will be a noop. This is necessary in multipart
/// upload scenarios when we want the SDK to only dispose the stream on the last part.
/// </summary>
internal bool DisableDispose { get; set; }

#if !NETSTANDARD
/// <inheritdoc/>
public override void Close()
{
if (!DisableDispose)
{
base.Close();
}
}
#else
protected override void Dispose(bool disposing)
{
if (!DisableDispose)
{
base.Dispose(disposing);
}
}
#endif
}
}
28 changes: 28 additions & 0 deletions src/Util/Constants.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright (c) 2000 - 2020 The Legion of the Bouncy Castle Inc. (https://www.bouncycastle.org)
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of this
* software and associated documentation files (the "Software"), to deal in the
* Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
* the Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all copies
* or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
* PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
* FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
* OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
*/
namespace Amazon.Extensions.S3.Encryption.Util
{
internal class Constants
{
internal const string S3CryptoStreamRequestState = "S3-Crypto-Stream";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static void TestTransferUtility(IAmazonS3 s3EncryptionClient, string buck

public static void TestTransferUtility(IAmazonS3 s3EncryptionClient, IAmazonS3 s3DecryptionClient, string bucketName)
{
var directory = TransferUtilityTests.CreateTestDirectory(10 * TransferUtilityTests.KILO_SIZE);
var directory = TransferUtilityTests.CreateTestDirectory(30 * MegaByteSize);
var keyPrefix = directory.Name;
var directoryPath = directory.FullName;

Expand Down

0 comments on commit e0e0dd3

Please sign in to comment.