Skip to content

Commit

Permalink
Client Encryption : Removes retry during decryption failure. (Azure#2640
Browse files Browse the repository at this point in the history
)

Removes retry during decryption failure in DeserializeAndDecryptResponseAsync(). The retry logic around decryption was basically to address the problem with policy change and is handled by the caller in all cases except for ChangeFeedProcessor API set since we do not have Options to pass in the required headers. Will address this later with a better approach.
  • Loading branch information
kr-santosh authored Aug 3, 2021
1 parent c0bb20a commit b344afc
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ internal static async Task<Stream> DeserializeAndDecryptResponseAsync(
CosmosDiagnosticsContext diagnosticsContext = CosmosDiagnosticsContext.Create(null);
using (diagnosticsContext.CreateScope("EncryptionProcessor.DeserializeAndDecryptResponseAsync"))
{
(JObject decryptedDocument, DecryptionContext _) = await EncryptionProcessor.DecryptAsync(
await EncryptionProcessor.DecryptAsync(
document,
encryptor,
diagnosticsContext,
Expand Down
70 changes: 19 additions & 51 deletions Microsoft.Azure.Cosmos.Encryption/src/EncryptionContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -910,29 +910,10 @@ internal async Task<Stream> DeserializeAndDecryptResponseAsync(
continue;
}

try
{
JObject decryptedDocument = await EncryptionProcessor.DecryptAsync(
document,
encryptionSettings,
cancellationToken);
}

// we cannot rely currently on a specific exception, this is due to the fact that the run time issue can be variable,
// we can hit issue with either Json serialization say an item was not encrypted but the policy shows it as encrypted,
// or we could hit a MicrosoftDataEncryptionException from MDE lib etc.
catch (Exception)
{
// most likely the encryption policy has changed.
encryptionSettings = await this.GetOrUpdateEncryptionSettingsFromCacheAsync(
obsoleteEncryptionSettings: encryptionSettings,
cancellationToken: cancellationToken);

JObject decryptedDocument = await EncryptionProcessor.DecryptAsync(
document,
encryptionSettings,
cancellationToken);
}
await EncryptionProcessor.DecryptAsync(
document,
encryptionSettings,
cancellationToken);
}

// the contents get decrypted in place by DecryptAsync.
Expand Down Expand Up @@ -1297,33 +1278,12 @@ private async Task<List<T>> DecryptChangeFeedDocumentsAsync<T>(

foreach (JObject document in documents)
{
try
{
JObject decryptedDocument = await EncryptionProcessor.DecryptAsync(
document,
encryptionSettings,
cancellationToken);

decryptedItems.Add(decryptedDocument.ToObject<T>());
}

// we cannot rely currently on a specific exception, this is due to the fact that the run time issue can be variable,
// we can hit issue with either Json serialization say an item was not encrypted but the policy shows it as encrypted,
// or we could hit a MicrosoftDataEncryptionException from MDE lib etc.
catch (Exception)
{
// most likely the encryption policy has changed.
encryptionSettings = await this.GetOrUpdateEncryptionSettingsFromCacheAsync(
obsoleteEncryptionSettings: encryptionSettings,
cancellationToken: cancellationToken);

JObject decryptedDocument = await EncryptionProcessor.DecryptAsync(
document,
encryptionSettings,
cancellationToken);
JObject decryptedDocument = await EncryptionProcessor.DecryptAsync(
document,
encryptionSettings,
cancellationToken);

decryptedItems.Add(decryptedDocument.ToObject<T>());
}
decryptedItems.Add(decryptedDocument.ToObject<T>());
}

return decryptedItems;
Expand Down Expand Up @@ -1385,9 +1345,17 @@ await this.GetOrUpdateEncryptionSettingsFromCacheAsync(
isRetry: true);
}

Stream decryptedContent = await this.DeserializeAndDecryptResponseAsync(responseMessage.Content, encryptionSettings, cancellationToken);
if (responseMessage.IsSuccessStatusCode && responseMessage.Content != null)
{
Stream decryptedContent = await this.DeserializeAndDecryptResponseAsync(
responseMessage.Content,
encryptionSettings,
cancellationToken);

return new DecryptedResponseMessage(responseMessage, decryptedContent);
}

return new DecryptedResponseMessage(responseMessage, decryptedContent);
return responseMessage;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1524,7 +1524,9 @@ public async Task VerifyKekRevokeHandling()
ContainerProperties containerProperties = new ContainerProperties(Guid.NewGuid().ToString(), "/PK") { ClientEncryptionPolicy = clientEncryptionPolicyWithRevokedKek };

Container encryptionContainer = await database.CreateContainerAsync(containerProperties, 400);


TestDoc testDoc1 = await MdeEncryptionTests.MdeCreateItemAsync(encryptionContainer);

testEncryptionKeyStoreProvider.RevokeAccessSet = true;

// try creating it and it should fail as it has been revoked.
Expand All @@ -1537,6 +1539,19 @@ public async Task VerifyKekRevokeHandling()
{
}

// testing query read fail due to revoked access.
try
{
await MdeEncryptionTests.ValidateQueryResultsAsync(
encryptionContainer,
"SELECT * FROM c",
testDoc1);
Assert.Fail("Query should have failed, since property path /Sensitive_NestedObjectFormatL1 has been encrypted using Cek with revoked access. ");
}
catch (RequestFailedException)
{
}

// for unwrap to succeed
testEncryptionKeyStoreProvider.RevokeAccessSet = false;

Expand Down

0 comments on commit b344afc

Please sign in to comment.