Reuse of encrypted properties results in downstream data corruption

[LeonBouquiet]

Describe the bug

Description

We have the situation where the same DTO, including properties that are encrypted by NServiceBus.Encryption.MessageProperty, is sent more than once. The first message that is sent has properly encrypted values, however the second message that gets sent has its properties encrypted twice. Which is obviously not expected or wanted.

We're using convention-based encryption rather than the EncryptedString type, by the way.

The code with which we're running into this issue is the following:

if(...)
{
	if (compareResult.Contains(CompareResult.Core))
	{
		//1) We start with modifiedEsbMedewerker having only plaintext values.
		await _serviceBusAdapterProxy.PublishMedewerkerCoreUpdated(modifiedEsbMedewerker);
		
		//2) At this point the sensitive string properties on modifiedEsbMedewerker contain encrypted values.
	}
}
if (!compareResult.Contains(CompareResult.MedewerkerNummerMissing))
{
	//3) If we get here, the encrypted properties get encrypted again before sending the second event.
	await _serviceBusAdapterProxy.PublishMedewerkerUpdated(modifiedEsbMedewerker);
}	

Expected behavior

At point 2, I would expect all properties on the modifiedEsbMedewerker to still have their plaintext values.

Actual behavior

At point 2, the properties that should be encrypted instead contain their encrypted values, which means that I can't use this DTO further down the line, and that when I want to send it again (at point 3), the encrypted value gets encrypted again before sending (which looks really weird on the receiving side btw, just as if encryption failed or isn't configured properly).

Where this probably goes wrong is at https://github.com/Particular/NServiceBus.Encryption.MessageProperty/blob/master/src/MessageProperty/StringConversions.cs#L12, where the plaintext is overwritten with the encrypted value.

Versions

We're using:

• NServiceBus Version 9.2.8
• NServiceBus.Encryption.MessageProperty Version 5.0.1

Steps to reproduce

Basically as shown above:

1. Configure property encryption by convention
2. Send myDto with a property to be encrypted as part of an event
3. Send the same myDto again, the encrypted property is encrypted again before sending.

Relevant log output

Additional Information

Workarounds

We currently create a deep-copy of the DTO that gets passed to the Publish() method, so that one gets changed instead and the original DTO can be used again. From a performance point this isn't optimal however.

Backported To

• 5.0.2: Reuse of encrypted properties results in downstream data corruption #716

[andreasohlund]

@LeonBouquiet, thanks for reporting this. We'll look deeper into this and get back to you, since you mentioned the workaround I assume you are not blocked by this.

To get the full context: That "modifiedEsbMedewerker" DTO is then included in different events published by calling that _serviceBusAdapterProxy right?

[LeonBouquiet]

Hi @andreasohlund, thanks for your reply. No we are not blocked by this, it's just that the current behaviour when publishing more than one event with the same DTO is really confusing, and I'd like to save others the same bughunt :)

And correct, the same "modifiedEsbMedewerker" instance is used twice, in two different events. Behind each _serviceBusAdapterProxy call is a call to IUniformSession.Publish() which triggers the encryption.

[andreasohlund]

@LeonBouquiet we have a fix for this that will be released shortly #713